You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/03/02 00:55:29 UTC
[GitHub] [lucene] gsmiller opened a new pull request #718: LUCENE-10444: Support alternate aggregation functions in association facets
gsmiller opened a new pull request #718:
URL: https://github.com/apache/lucene/pull/718
# Description
This change adds support for "max" aggregations (in addition to "sum") to association faceting. It does so in a way that is (somewhat) extensible for future aggregation functionality.
# Solution
Replaced the existing association faceting classes that were hardcoded to "sum" with new classes that allow the user to specify an aggregation function. Note that I will open a separate PR for a backport of this that remains backwards-compatible on 9x.
# Tests
Added new testing for new aggregation functionality.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/lucene/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Lucene maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [x] I have run `./gradlew check`.
- [x] I have added tests for my changes.
--
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 pull request #718: LUCENE-10444: Support alternate aggregation functions in association facets
Posted by GitBox <gi...@apache.org>.
gsmiller commented on pull request #718:
URL: https://github.com/apache/lucene/pull/718#issuecomment-1066998156
Even though I ran benchmarks on the backport version of this change (#719), I figured it would be good to run benchmarks here as well. Below compares this patch against `main` using `wikimedium10m`:
```
TaskQPS baseline StdDevQPS candidate StdDev Pct diff p-value
BrowseDayOfYearTaxoFacets 21.65 (22.8%) 20.61 (22.9%) -4.8% ( -41% - 52%) 0.507
BrowseDateTaxoFacets 21.61 (22.6%) 20.61 (22.8%) -4.6% ( -40% - 52%) 0.519
Prefix3 362.81 (10.5%) 349.00 (11.4%) -3.8% ( -23% - 20%) 0.273
BrowseRandomLabelTaxoFacets 17.76 (18.5%) 17.15 (20.0%) -3.4% ( -35% - 43%) 0.577
BrowseMonthTaxoFacets 27.32 (27.1%) 26.45 (29.2%) -3.2% ( -46% - 72%) 0.721
Wildcard 64.61 (5.6%) 63.58 (6.0%) -1.6% ( -12% - 10%) 0.381
OrNotHighMed 887.00 (3.3%) 874.66 (3.4%) -1.4% ( -7% - 5%) 0.191
LowTerm 2661.07 (2.9%) 2630.67 (3.2%) -1.1% ( -7% - 5%) 0.240
OrNotHighHigh 1523.01 (3.8%) 1506.01 (4.2%) -1.1% ( -8% - 7%) 0.379
AndHighMedDayTaxoFacets 124.82 (1.4%) 123.59 (1.5%) -1.0% ( -3% - 1%) 0.032
HighSpanNear 21.47 (4.9%) 21.27 (4.9%) -0.9% ( -10% - 9%) 0.558
MedPhrase 342.22 (2.8%) 339.35 (3.2%) -0.8% ( -6% - 5%) 0.373
HighPhrase 453.21 (2.2%) 449.61 (2.6%) -0.8% ( -5% - 4%) 0.291
MedSpanNear 74.03 (4.3%) 73.45 (4.2%) -0.8% ( -8% - 8%) 0.559
BrowseMonthSSDVFacets 13.79 (19.1%) 13.69 (19.7%) -0.7% ( -33% - 47%) 0.910
LowPhrase 85.89 (1.9%) 85.33 (2.0%) -0.7% ( -4% - 3%) 0.294
PKLookup 169.25 (3.2%) 168.19 (2.3%) -0.6% ( -5% - 5%) 0.481
OrNotHighLow 962.38 (3.0%) 956.47 (2.9%) -0.6% ( -6% - 5%) 0.511
BrowseDayOfYearSSDVFacets 12.23 (14.4%) 12.15 (14.5%) -0.6% ( -25% - 33%) 0.897
BrowseDateSSDVFacets 2.34 (6.1%) 2.32 (7.8%) -0.6% ( -13% - 14%) 0.802
OrHighMed 134.10 (5.0%) 133.49 (4.6%) -0.5% ( -9% - 9%) 0.766
Fuzzy1 91.09 (1.2%) 90.71 (1.8%) -0.4% ( -3% - 2%) 0.373
HighTerm 1690.39 (4.6%) 1684.21 (5.3%) -0.4% ( -9% - 10%) 0.816
OrHighNotHigh 1592.87 (2.4%) 1587.86 (3.7%) -0.3% ( -6% - 5%) 0.751
AndHighHighDayTaxoFacets 12.34 (2.4%) 12.31 (2.4%) -0.3% ( -4% - 4%) 0.696
AndHighLow 927.42 (3.2%) 924.95 (3.1%) -0.3% ( -6% - 6%) 0.790
MedSloppyPhrase 107.35 (2.5%) 107.06 (2.5%) -0.3% ( -5% - 4%) 0.736
IntNRQ 83.16 (1.2%) 82.95 (1.0%) -0.2% ( -2% - 1%) 0.469
MedTerm 1935.87 (4.3%) 1934.77 (4.8%) -0.1% ( -8% - 9%) 0.969
OrHighNotLow 1109.87 (4.1%) 1109.25 (4.7%) -0.1% ( -8% - 9%) 0.968
HighSloppyPhrase 28.62 (1.8%) 28.61 (2.6%) -0.0% ( -4% - 4%) 0.981
Respell 57.41 (1.1%) 57.40 (1.6%) -0.0% ( -2% - 2%) 0.968
LowSpanNear 193.79 (3.4%) 193.81 (3.8%) 0.0% ( -6% - 7%) 0.992
LowSloppyPhrase 30.88 (1.5%) 30.90 (1.8%) 0.1% ( -3% - 3%) 0.885
OrHighHigh 38.71 (4.3%) 38.74 (4.2%) 0.1% ( -8% - 8%) 0.954
OrHighLow 606.33 (2.8%) 606.93 (2.5%) 0.1% ( -5% - 5%) 0.907
OrHighNotMed 985.50 (4.3%) 986.95 (4.6%) 0.1% ( -8% - 9%) 0.917
Fuzzy2 38.38 (1.5%) 38.45 (1.8%) 0.2% ( -3% - 3%) 0.698
OrHighMedDayTaxoFacets 5.37 (4.8%) 5.39 (4.9%) 0.4% ( -8% - 10%) 0.801
MedTermDayTaxoFacets 27.22 (3.9%) 27.41 (4.6%) 0.7% ( -7% - 9%) 0.606
LowIntervalsOrdered 38.55 (3.8%) 38.89 (3.2%) 0.9% ( -5% - 8%) 0.415
BrowseRandomLabelSSDVFacets 9.42 (7.4%) 9.53 (9.7%) 1.1% ( -14% - 19%) 0.677
MedIntervalsOrdered 33.92 (4.9%) 34.32 (4.2%) 1.2% ( -7% - 10%) 0.412
AndHighMed 132.91 (5.1%) 135.21 (7.0%) 1.7% ( -9% - 14%) 0.375
HighIntervalsOrdered 25.36 (8.5%) 25.81 (7.0%) 1.7% ( -12% - 18%) 0.478
HighTermTitleBDVSort 82.25 (17.7%) 83.75 (12.8%) 1.8% ( -24% - 39%) 0.710
HighTermMonthSort 133.31 (13.6%) 135.96 (13.2%) 2.0% ( -21% - 33%) 0.638
AndHighHigh 65.89 (5.0%) 67.30 (7.3%) 2.1% ( -9% - 15%) 0.282
TermDTSort 69.38 (14.1%) 73.72 (12.3%) 6.3% ( -17% - 38%) 0.135
HighTermDayOfYearSort 43.49 (15.0%) 46.25 (18.9%) 6.3% ( -23% - 47%) 0.239
```
--
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] msokolov commented on pull request #718: LUCENE-10444: Support alternate aggregation functions in association facets
Posted by GitBox <gi...@apache.org>.
msokolov commented on pull request #718:
URL: https://github.com/apache/lucene/pull/718#issuecomment-1058501380
> It makes sense to me to add this capability. I wonder if the extra abstraction hurts us though in these tight loops summing up values in an array? If it does, we might want to provide a specialization for such loops as well?
Oh I missed the benchmarking you did - I guess it was on the backport PR. Looked like no significant change there, 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-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 a change in pull request #718: LUCENE-10444: Support alternate aggregation functions in association facets
Posted by GitBox <gi...@apache.org>.
gsmiller commented on a change in pull request #718:
URL: https://github.com/apache/lucene/pull/718#discussion_r837744334
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/FloatTaxonomyFacets.java
##########
@@ -49,22 +57,24 @@ void rollup() throws IOException {
if (ft.hierarchical && ft.multiValued == false) {
int dimRootOrd = taxoReader.getOrdinal(new FacetLabel(dim));
assert dimRootOrd > 0;
- values[dimRootOrd] += rollup(children[dimRootOrd]);
+ float newValue =
+ aggregationFunction.aggregate(values[dimRootOrd], rollup(children[dimRootOrd]));
Review comment:
Right. More work needed to support MIN. I'll leave as TODO as you suggest. Thanks!
--
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 a change in pull request #718: LUCENE-10444: Support alternate aggregation functions in association facets
Posted by GitBox <gi...@apache.org>.
gsmiller commented on a change in pull request #718:
URL: https://github.com/apache/lucene/pull/718#discussion_r826082690
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/FloatTaxonomyFacets.java
##########
@@ -130,16 +140,16 @@ public FacetResult getTopChildren(int topN, String dim, String... path) throws I
ord = siblings[ord];
}
- if (sumValues == 0) {
+ if (aggregatedValue == 0) {
return null;
}
if (dimConfig.multiValued) {
if (dimConfig.requireDimCount) {
- sumValues = values[dimOrd];
+ aggregatedValue = values[dimOrd];
} else {
// Our sum'd count is not correct, in general:
Review comment:
It's not necessarily a "count" though here right? It's an aggregated weight associated with the value.
--
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] msokolov commented on a change in pull request #718: LUCENE-10444: Support alternate aggregation functions in association facets
Posted by GitBox <gi...@apache.org>.
msokolov commented on a change in pull request #718:
URL: https://github.com/apache/lucene/pull/718#discussion_r819043542
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
##########
@@ -173,17 +185,17 @@ public FacetResult getTopChildren(int topN, String dim, String... path) throws I
if (sparseValues != null) {
for (IntIntCursor c : sparseValues) {
- int count = c.value;
+ int value = c.value;
int ord = c.key;
- if (parents[ord] == dimOrd && count > 0) {
- totValue += count;
+ if (parents[ord] == dimOrd && value > 0) {
+ aggregatedValue = aggregationFunction.aggregate(aggregatedValue, value);
childCount++;
- if (count > bottomValue) {
+ if (value > bottomValue) {
Review comment:
I guess we need to ensure that aggregation functions are nondecreasing? I mean `min` wouldn't work very well here
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/FloatTaxonomyFacets.java
##########
@@ -130,16 +140,16 @@ public FacetResult getTopChildren(int topN, String dim, String... path) throws I
ord = siblings[ord];
}
- if (sumValues == 0) {
+ if (aggregatedValue == 0) {
return null;
}
if (dimConfig.multiValued) {
if (dimConfig.requireDimCount) {
- sumValues = values[dimOrd];
+ aggregatedValue = values[dimOrd];
} else {
// Our sum'd count is not correct, in general:
Review comment:
our "aggregated" count?
--
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 a change in pull request #718: LUCENE-10444: Support alternate aggregation functions in association facets
Posted by GitBox <gi...@apache.org>.
gsmiller commented on a change in pull request #718:
URL: https://github.com/apache/lucene/pull/718#discussion_r838621313
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyFacetFloatAssociations.java
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.facet.taxonomy;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.lucene.facet.FacetField;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.FacetsCollector.MatchingDocs;
+import org.apache.lucene.facet.FacetsConfig;
+import org.apache.lucene.index.BinaryDocValues;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.index.SortedNumericDocValues;
+import org.apache.lucene.search.ConjunctionUtils;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.DoubleValues;
+import org.apache.lucene.search.DoubleValuesSource;
+import org.apache.lucene.util.BitUtil;
+import org.apache.lucene.util.BytesRef;
+
+/**
+ * Aggregates float values associated with facet fields. Supports two different approaches:
+ *
+ * <ol>
+ * <li>Fields can be indexed with {@link FloatAssociationFacetField}, associating weights with
+ * facet values at indexing time.
+ * <li>Fields can be indexed with {@link FacetField} and a {@link DoubleValuesSource} can
+ * dynamically supply a weight from each doc. With this approach, the document's weight gets
+ * contributed to each facet value associated with the doc.
+ * </ol>
+ *
+ * Aggregation logic is supplied by the provided {@link FloatAssociationFacetField}.
+ *
+ * @lucene.experimental
+ */
+public class TaxonomyFacetFloatAssociations extends FloatTaxonomyFacets {
+
+ /** Create {@code TaxonomyFacetFloatAssociations} against the default index field. */
+ public TaxonomyFacetFloatAssociations(
+ TaxonomyReader taxoReader,
+ FacetsConfig config,
+ FacetsCollector fc,
+ AssociationAggregationFunction aggregationFunction)
+ throws IOException {
+ this(FacetsConfig.DEFAULT_INDEX_FIELD_NAME, taxoReader, config, fc, aggregationFunction);
+ }
+
+ /**
+ * Create {@code TaxonomyFacetFloatAssociations} against the default index field. Sources values
+ * from the provided {@code valuesSource}.
+ */
+ public TaxonomyFacetFloatAssociations(
+ TaxonomyReader taxoReader,
+ FacetsConfig config,
+ FacetsCollector fc,
+ AssociationAggregationFunction aggregationFunction,
+ DoubleValuesSource valuesSource)
+ throws IOException {
+ this(
+ FacetsConfig.DEFAULT_INDEX_FIELD_NAME,
+ taxoReader,
+ config,
+ fc,
+ aggregationFunction,
+ valuesSource);
+ }
+
+ /** Create {@code TaxonomyFacetFloatAssociations} against the specified index field. */
+ public TaxonomyFacetFloatAssociations(
+ String indexFieldName,
+ TaxonomyReader taxoReader,
+ FacetsConfig config,
+ FacetsCollector fc,
+ AssociationAggregationFunction aggregationFunction)
+ throws IOException {
+ super(indexFieldName, taxoReader, aggregationFunction, config);
+ aggregateValues(aggregationFunction, fc.getMatchingDocs());
+ }
+
+ /**
+ * Create {@code TaxonomyFacetFloatAssociations} against the specified index field. Sources values
+ * from the provided {@code valuesSource}.
+ */
+ public TaxonomyFacetFloatAssociations(
+ String indexFieldName,
+ TaxonomyReader taxoReader,
+ FacetsConfig config,
+ FacetsCollector fc,
+ AssociationAggregationFunction aggregationFunction,
+ DoubleValuesSource valuesSource)
+ throws IOException {
+ super(indexFieldName, taxoReader, aggregationFunction, config);
+ aggregateValues(aggregationFunction, fc.getMatchingDocs(), fc.getKeepScores(), valuesSource);
+ }
+
+ private static DoubleValues scores(MatchingDocs hits) {
+ return new DoubleValues() {
+
+ int index = -1;
+
+ @Override
+ public double doubleValue() throws IOException {
+ return hits.scores[index];
+ }
+
+ @Override
+ public boolean advanceExact(int doc) throws IOException {
+ index++;
Review comment:
Oh wow. Great catch! I lifted this from the current implementation but this is an uncaught bug. I just reproduced it with a test. I'm going to open a separate issue and fix this bug first, then merge the fix into this PR. Thanks Mike!
--
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 a change in pull request #718: LUCENE-10444: Support alternate aggregation functions in association facets
Posted by GitBox <gi...@apache.org>.
gsmiller commented on a change in pull request #718:
URL: https://github.com/apache/lucene/pull/718#discussion_r826085655
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
##########
@@ -173,17 +185,17 @@ public FacetResult getTopChildren(int topN, String dim, String... path) throws I
if (sparseValues != null) {
for (IntIntCursor c : sparseValues) {
- int count = c.value;
+ int value = c.value;
int ord = c.key;
- if (parents[ord] == dimOrd && count > 0) {
- totValue += count;
+ if (parents[ord] == dimOrd && value > 0) {
+ aggregatedValue = aggregationFunction.aggregate(aggregatedValue, value);
childCount++;
- if (count > bottomValue) {
+ if (value > bottomValue) {
Review comment:
That's right. There are a number of things actually preventing us from cleanly adding something like `min`. I had it originally but as I started looking at all the changes it would require, I backed off for the time being (especially since I don't have a concrete use-case in mind). One interesting challenge is that these facets implementations all assume the weights are positive values. There are a lot of `> 0` checks floating around the various implementations to check whether-or-not a value had any "weight" associated with it. This makes sense when using counts, but it's weird when generally associated weights with the values. So `min` started to feel a little weird and I just left it out for 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
[GitHub] [lucene] mikemccand commented on a change in pull request #718: LUCENE-10444: Support alternate aggregation functions in association facets
Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #718:
URL: https://github.com/apache/lucene/pull/718#discussion_r837510411
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/FloatTaxonomyFacets.java
##########
@@ -49,22 +57,24 @@ void rollup() throws IOException {
if (ft.hierarchical && ft.multiValued == false) {
int dimRootOrd = taxoReader.getOrdinal(new FacetLabel(dim));
assert dimRootOrd > 0;
- values[dimRootOrd] += rollup(children[dimRootOrd]);
+ float newValue =
+ aggregationFunction.aggregate(values[dimRootOrd], rollup(children[dimRootOrd]));
Review comment:
Are we assuming here that using `0` always as an initial value is OK for any aggregation function? What if we wanted `MIN` aggregator?
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/IntTaxonomyFacets.java
##########
@@ -173,17 +185,17 @@ public FacetResult getTopChildren(int topN, String dim, String... path) throws I
if (sparseValues != null) {
for (IntIntCursor c : sparseValues) {
- int count = c.value;
+ int value = c.value;
int ord = c.key;
- if (parents[ord] == dimOrd && count > 0) {
- totValue += count;
+ if (parents[ord] == dimOrd && value > 0) {
+ aggregatedValue = aggregationFunction.aggregate(aggregatedValue, value);
childCount++;
- if (count > bottomValue) {
+ if (value > bottomValue) {
Review comment:
Ahh, OK, so then disregard my comment above -- we should not try to tackle `min` here!
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/taxonomy/TaxonomyFacetFloatAssociations.java
##########
@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.facet.taxonomy;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.lucene.facet.FacetField;
+import org.apache.lucene.facet.FacetsCollector;
+import org.apache.lucene.facet.FacetsCollector.MatchingDocs;
+import org.apache.lucene.facet.FacetsConfig;
+import org.apache.lucene.index.BinaryDocValues;
+import org.apache.lucene.index.DocValues;
+import org.apache.lucene.index.SortedNumericDocValues;
+import org.apache.lucene.search.ConjunctionUtils;
+import org.apache.lucene.search.DocIdSetIterator;
+import org.apache.lucene.search.DoubleValues;
+import org.apache.lucene.search.DoubleValuesSource;
+import org.apache.lucene.util.BitUtil;
+import org.apache.lucene.util.BytesRef;
+
+/**
+ * Aggregates float values associated with facet fields. Supports two different approaches:
+ *
+ * <ol>
+ * <li>Fields can be indexed with {@link FloatAssociationFacetField}, associating weights with
+ * facet values at indexing time.
+ * <li>Fields can be indexed with {@link FacetField} and a {@link DoubleValuesSource} can
+ * dynamically supply a weight from each doc. With this approach, the document's weight gets
+ * contributed to each facet value associated with the doc.
+ * </ol>
+ *
+ * Aggregation logic is supplied by the provided {@link FloatAssociationFacetField}.
+ *
+ * @lucene.experimental
+ */
+public class TaxonomyFacetFloatAssociations extends FloatTaxonomyFacets {
+
+ /** Create {@code TaxonomyFacetFloatAssociations} against the default index field. */
+ public TaxonomyFacetFloatAssociations(
+ TaxonomyReader taxoReader,
+ FacetsConfig config,
+ FacetsCollector fc,
+ AssociationAggregationFunction aggregationFunction)
+ throws IOException {
+ this(FacetsConfig.DEFAULT_INDEX_FIELD_NAME, taxoReader, config, fc, aggregationFunction);
+ }
+
+ /**
+ * Create {@code TaxonomyFacetFloatAssociations} against the default index field. Sources values
+ * from the provided {@code valuesSource}.
+ */
+ public TaxonomyFacetFloatAssociations(
+ TaxonomyReader taxoReader,
+ FacetsConfig config,
+ FacetsCollector fc,
+ AssociationAggregationFunction aggregationFunction,
+ DoubleValuesSource valuesSource)
+ throws IOException {
+ this(
+ FacetsConfig.DEFAULT_INDEX_FIELD_NAME,
+ taxoReader,
+ config,
+ fc,
+ aggregationFunction,
+ valuesSource);
+ }
+
+ /** Create {@code TaxonomyFacetFloatAssociations} against the specified index field. */
+ public TaxonomyFacetFloatAssociations(
+ String indexFieldName,
+ TaxonomyReader taxoReader,
+ FacetsConfig config,
+ FacetsCollector fc,
+ AssociationAggregationFunction aggregationFunction)
+ throws IOException {
+ super(indexFieldName, taxoReader, aggregationFunction, config);
+ aggregateValues(aggregationFunction, fc.getMatchingDocs());
+ }
+
+ /**
+ * Create {@code TaxonomyFacetFloatAssociations} against the specified index field. Sources values
+ * from the provided {@code valuesSource}.
+ */
+ public TaxonomyFacetFloatAssociations(
+ String indexFieldName,
+ TaxonomyReader taxoReader,
+ FacetsConfig config,
+ FacetsCollector fc,
+ AssociationAggregationFunction aggregationFunction,
+ DoubleValuesSource valuesSource)
+ throws IOException {
+ super(indexFieldName, taxoReader, aggregationFunction, config);
+ aggregateValues(aggregationFunction, fc.getMatchingDocs(), fc.getKeepScores(), valuesSource);
+ }
+
+ private static DoubleValues scores(MatchingDocs hits) {
+ return new DoubleValues() {
+
+ int index = -1;
+
+ @Override
+ public double doubleValue() throws IOException {
+ return hits.scores[index];
+ }
+
+ @Override
+ public boolean advanceExact(int doc) throws IOException {
+ index++;
Review comment:
Hmm, instead of `index++` shouldn't we do `index = doc`?
##########
File path: lucene/facet/src/test/org/apache/lucene/facet/taxonomy/TestTaxonomyFacetAssociations.java
##########
@@ -76,6 +93,49 @@ public static void beforeClass() throws Exception {
writer.addDocument(config.build(taxoWriter, doc));
}
+ // Also index random content for more random testing:
Review comment:
Yay, thanks!
--
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