You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "stefanvodita (via GitHub)" <gi...@apache.org> on 2023/09/23 12:51:27 UTC
[GitHub] [lucene] stefanvodita opened a new issue, #12585: Is it correct for facets to assume positive aggregation values?
stefanvodita opened a new issue, #12585:
URL: https://github.com/apache/lucene/issues/12585
### Description
In `IntTaxonomyFacets` and `FloatTaxonomyFacets` when we `getTopChildrenForPath` we maintain a priority queue of aggregation values and corresponding ordinals.
If an aggregation value is not positive, we discard it ([code](https://github.com/apache/lucene/blob/8b84f6c0965dda01514c196126ed10e57cb6d01a/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/FloatTaxonomyFacets.java#L219)). If it is not larger than the lowest value in the queue (initially set to 0), we also discard it ([code](https://github.com/apache/lucene/blob/8b84f6c0965dda01514c196126ed10e57cb6d01a/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/FloatTaxonomyFacets.java#L222)).
It looks like the facets assume aggregation results should be positive, but I think we could have negatives or zeroes. I wrote a [unit test to demo this](https://github.com/apache/lucene/commit/d1455fd9447011d1209f7f965afd6c1aa03ad5c1).
The current behavior makes sense for counts. Counts are always non-negative and we probably don't care about counts of 0. But does it make sense for other aggregations?
### Version and environment details
_No response_
--
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-unsubscribe@lucene.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [I] Is it correct for facets to assume positive aggregation values? [lucene]
Posted by "stefanvodita (via GitHub)" <gi...@apache.org>.
stefanvodita commented on issue #12585:
URL: https://github.com/apache/lucene/issues/12585#issuecomment-1821880175
I thought some more about this issue and it really seems like a bug that I can have a non-positive aggregation value, but I can't return it in top children.
If we were to address it, I can think of three approaches:
1. Choose a magic value that communicates to us that an ordinal was not encountered. This magic value is 0 right now. Maybe `Integer.MIN_VALUE` and `Float.MAX_VALUE` might be better choices, allowing for more valid aggregation values.
2. Store a boolean for each ordinal. This would produce correct results, but cost more memory.
3. Use a map to store the arrays. `IntTaxonomyFacets` already does this in some cases and I [think](https://github.com/apache/lucene/issues/12576#issuecomment-1783545880) this might be appropriate in enough cases that we can always use a map.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [I] Is it correct for facets to assume positive aggregation values? [lucene]
Posted by "stefanvodita (via GitHub)" <gi...@apache.org>.
stefanvodita closed issue #12585: Is it correct for facets to assume positive aggregation values?
URL: https://github.com/apache/lucene/issues/12585
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] stefanvodita commented on issue #12585: Is it correct for facets to assume positive aggregation values?
Posted by "stefanvodita (via GitHub)" <gi...@apache.org>.
stefanvodita commented on issue #12585:
URL: https://github.com/apache/lucene/issues/12585#issuecomment-1741471016
> not try to modify the faceting module as-is, but rather spin up a new "aggregations" module
I'm definitely leaning that way too right now. @Shradha26 and I were considering this (#12553). I initially thought faceting can fill most of the gaps we were identifying. What got me to change my mind was seeing how much extra work there is to bring in a new feature when it has to be reimplemented for multiple faceting classes. If we made facets generic, that would already be a significant rework, so maybe we might as well start fresh? There's other things that are difficult to do with facets - arbitrary aggregation groups come to mind, but genericity is the one that convinced me that it's a better strategy to have a dedicated aggregation engine. I'm sure there are counter-arguments as well or maybe I'm overstating the impact of genericity.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [I] Is it correct for facets to assume positive aggregation values? [lucene]
Posted by "gsmiller (via GitHub)" <gi...@apache.org>.
gsmiller commented on issue #12585:
URL: https://github.com/apache/lucene/issues/12585#issuecomment-1832904037
I think I'd be curious if there are many real-world use-cases out there that have a non-positive association between each document and facet label? I would guess it's pretty uncommon. I actually can't really contrive an example either. @stefanvodita have you encountered real-world use-cases that would benefit from this? Apologies if you describe it somewhere and I'm missing it.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [I] Is it correct for facets to assume positive aggregation values? [lucene]
Posted by "stefanvodita (via GitHub)" <gi...@apache.org>.
stefanvodita commented on issue #12585:
URL: https://github.com/apache/lucene/issues/12585#issuecomment-1833870747
I have written a [unit test to showcase the problem](https://github.com/apache/lucene/commit/d1455fd9447011d1209f7f965afd6c1aa03ad5c1), but I haven't seen it in the wild.
Non-positive associations make sense when we're dealing with data that can naturally be non-positive. Let's say we're managing a school's report cards. Each student has a corresponding document. They take multiple-choice tests, which penalize wrong answers to discourage randomly filling in the test. Each test the students take during the year is represented by a facet label and each student has their score associated with the label. Not all scores would be positive and aggregations over those scores may or may not be positive. Maybe that's too much speculation on my part though.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on issue #12585: Is it correct for facets to assume positive aggregation values?
Posted by "gsmiller (via GitHub)" <gi...@apache.org>.
gsmiller commented on issue #12585:
URL: https://github.com/apache/lucene/issues/12585#issuecomment-1741276611
Yeah, this is a good callout. I ran into this when adding more flexibility to association faceting a while back (making note that supporting, e.g., "min" would require rethinking these assumptions).
My opinion is that the current assumption makes sense for the faceting support _currently available_, but I know there's conversation going on more generally about improving (rethinking?) aggregation capabilities in Lucene. My preference on this would be to, 1) leave this current behavior in place unless there is a use-case that's immediately blocked by it, but 2) include it in some broader rethinking of Lucene aggregation capabilities. As a side-note on that, I wonder if a successful approach to moving forward with some new aggregation thinking would be to _not_ try to modify the faceting module as-is, but rather spin up a new "aggregations" module under "sandbox" to start sketching out ideas. I think it will be difficult to retrofit more flexible aggregation capabilities into the faceting API that exists today, but maybe I'm wrong? OK, I'm off in the weeds now...
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org