shaie commented on PR #841: URL: https://github.com/apache/lucene/pull/841#issuecomment-1149554008
> `facetset` for the exact implementation and `hyperrectangle` for the range implementation If I understand correctly, @gsmiller's proposal was more about efficiency, right? So if you have a large number of points that you'd like to index, an R-Tree might be a better data structure for these range matches, than the current linear scan we do in `RangeFSM`. But it doesn't mean we shouldn't have `RangeFSM` for a small dimensionality / small number of FacetSet per doc cases right? They might even perform better? IMO, and to echo Greg's "progress not perfection", I feel it's better if we introduce the `HyperRectangle` impls (1) when we have a good use case to demonstrate the need for it (it just helps to reason about it) and (2) when we're actually ready to implement it "more efficiently". That is, in this PR I feel like we can do w/ `RangeFSM` and later (if and when we'll have a better alternative for large dims) document and reference the alternative? > For the second question, I think we should keep this as a `long[]` based API as we know we want to make the KD tree and R tree optimizations in the future If we think that KD-Trees can be more efficient for `FacetSetMatcher` impls and we're sure that we'll get there _soon_, then yeah, we can move to `long[]` based API. Another alternative I thought of is keep the `byte[]` API, but introduce a helper method on `FSM` which converts the `byte[]` to `long[]` for whoever needs it. To re-iterate what I previously wrote about API back-compat, I feel that putting `@lucene.experimental` tag on the classes is enough to have us not worry about changing it. Especially if the API hasn't been released yet, and definitely if we intend to follow with a KD-Tree impl very soon. A year from now (just an example) it's a different story, but for now I don't think we need to finalize the API for good. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
