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 2020/11/25 15:03:33 UTC
[GitHub] [lucene-solr] cammiemw opened a new pull request #2097: LUCENE-9537
cammiemw opened a new pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Lucene or Solr:
* https://issues.apache.org/jira/projects/LUCENE
* https://issues.apache.org/jira/projects/SOLR
You will need to create an account in Jira in order to create an issue.
The title of the PR should reference the Jira issue number in the form:
* LUCENE-####: <short description of problem or changes>
* SOLR-####: <short description of problem or changes>
LUCENE and SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
This pull request implements logic from our academic search engine Indri: http://www.lemurproject.org/indri.php. The functionality that is implemented is a smoothing score for search terms or subqueries that are not present in the document being scored. The smoothing score acts like an idf so that documents that do not have terms or subqueries that are more frequent in the index are not penalized as much as documents that do not have less frequent terms or subqueries. Additionally, Indri's dirichelet smoothing similarity has been added.
# Solution
The smoothingScore method has been added to the Scorable interface and implemented in the abstract class Scorer. The classes IndriAndQuery, IndriAndWeight, and IndriAndScorer have been added to call the smoothingScore method on documents where the search term or subquery are not present. The class IndriDirichletSimilarity has been added for implementing Indri's equation for the Language Model with Dirichlet smoothing.
# Tests
TestIndriAndQuery and TestIndriDirichletSmoothing have been added. I am happy to expand upon these tests and implement more tests.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/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.
- [ ] I have given Solr 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 `master` branch.
- [x] I have run `./gradlew check`.
- [x] I have added tests for my changes.
- [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
----------------------------------------------------------------
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.
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-solr] mikemccand commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-767646296
> I have added an entry in CHANGES.txt under API changes.
Thank you!
I ran top-level `./gradlew test` with this change but hit this Solr test failure, caused by this change, and hopefully easy to fix?:
```
org.apache.solr.ltr.feature.TestFeature > test suite's output saved to /l/trunk/solr/contrib/ltr/build/test-results/test/outputs/OUTPUT-org.apache.solr.ltr.feature.TestFeature.txt, copied below:
> java.lang.AssertionError: class org.apache.solr.ltr.feature.Feature$FeatureWeight$FilterFeatureScorer needs to override 'public float org.apache.lucene.search.Scorer.smoothingScore(int) throws java.io.IOException'
> at __randomizedtesting.SeedInfo.seed([933C5904DAF6843E:848500E5F410D49E]:0)
> at org.junit.Assert.fail(Assert.java:89)
> at org.apache.solr.ltr.feature.TestFeature.testFilterFeatureScorerOverridesScorerMethods(TestFeature.java:46)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64)
> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:564)
> at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1754)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:942)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:978)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:992)
> at org.apache.lucene.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:44)
> at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
> at org.apache.lucene.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:45)
> at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
> at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
> at org.junit.rules.RunRules.evaluate(RunRules.java:20)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:370)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:819)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:470)
> at com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:951)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:836)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:887)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:898)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule$1.evaluate(SystemPropertiesRestoreRule.java:57)
> at org.junit.rules.RunRules.evaluate(RunRules.java:20)
> at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at org.apache.lucene.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:38)
> at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
> at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at org.apache.lucene.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:51)
> at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
> at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
> at org.apache.lucene.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:47)
> at org.junit.rules.RunRules.evaluate(RunRules.java:20)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:370)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl.lambda$forkTimeoutingTask$0(ThreadLeakControl.java:826)
> at java.base/java.lang.Thread.run(Thread.java:832)
2> NOTE: reproduce with: gradlew test --tests TestFeature.testFilterFeatureScorerOverridesScorerMethods -Dtests.seed=933C5904DAF6843E -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=bo-IN -Dtests.timezone=Etc/GMT -Dte\
sts.asserts=true -Dtests.file.encoding=UTF-8
2> NOTE: test params are: codec=Asserting(Lucene90): {}, docValues:{}, maxPointsInLeafNode=1006, maxMBSortInHeap=6.621471242227271, sim=Asserting(RandomSimilarity(queryNorm=true): {}), locale=bo-IN, timezone=Etc/GMT
2> NOTE: Linux 5.9.8-arch1-1 amd64/Oracle Corporation 15.0.1 (64-bit)/cpus=128,threads=1,free=249561968,total=270532608
2> NOTE: All tests run in this JVM: [TestFeature]
```
----------------------------------------------------------------
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.
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-solr] cammiemw commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r555482275
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriQuery.java
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.search;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * A Basic abstract query that all IndriQueries can extend to implement
+ * toString, equals, getClauses, and iterator.
+ *
+ */
+public abstract class IndriQuery extends Query
Review comment:
Currently, IndriQuery does not take advantage of Block MAX Weak and optimization. We iterate through all documents that that have a posting for at least one of the search terms. I would be interested in expanding the smoothing score functionality to more parts of lucene in the future.
----------------------------------------------------------------
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.
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-solr] muse-dev[bot] commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
muse-dev[bot] commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r555505138
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndWeight.java
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.search;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.lucene.index.LeafReaderContext;
+
+/**
+ * The Weight for IndriAndQuery, used to normalize, score and explain these
+ * queries.
+ */
+public class IndriAndWeight extends Weight {
+
+ private final IndriAndQuery query;
+ private final ArrayList<Weight> weights;
+ private final ScoreMode scoreMode;
+ private final float boost;
+
+ public IndriAndWeight(IndriAndQuery query, IndexSearcher searcher,
+ ScoreMode scoreMode, float boost) throws IOException {
+ super(query);
+ this.query = query;
+ this.boost = boost;
+ this.scoreMode = scoreMode;
+ weights = new ArrayList<>();
+ for (BooleanClause c : query) {
+ Weight w = searcher.createWeight(c.getQuery(), scoreMode, 1.0f);
+ weights.add(w);
+ }
+ }
+
+ private Scorer getScorer(LeafReaderContext context) throws IOException {
+ List<Scorer> subScorers = new ArrayList<>();
+
+ for (Weight w : weights) {
+ Scorer scorer = w.scorer(context);
+ if (scorer != null) {
+ subScorers.add(scorer);
+ }
+ }
+
+ if (subScorers.isEmpty()) {
+ return null;
+ }
+ Scorer scorer = subScorers.get(0);
+ if (subScorers.size() > 1) {
+ scorer = new IndriAndScorer(this, subScorers, scoreMode, boost);
+ }
+ return scorer;
+ }
+
+ @Override
+ public Scorer scorer(LeafReaderContext context) throws IOException {
+ return getScorer(context);
+ }
+
+ @Override
+ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException {
+ Scorer scorer = getScorer(context);
+ if (scorer != null) {
+ BulkScorer bulkScorer = new DefaultBulkScorer(scorer);
+ return bulkScorer;
+ }
+ return null;
+ }
+
+ @Override
+ public boolean isCacheable(LeafReaderContext ctx) {
+ for (Weight w : weights) {
+ if (w.isCacheable(ctx) == false) return false;
+ }
+ return true;
+ }
+
+ @Override
+ public Explanation explain(LeafReaderContext context, int doc)
+ throws IOException {
+ List<Explanation> subs = new ArrayList<>();
+ boolean fail = false;
+ Iterator<BooleanClause> cIter = query.iterator();
+ for (Iterator<Weight> wIter = weights.iterator(); wIter.hasNext();) {
+ Weight w = wIter.next();
+ BooleanClause c = cIter.next();
+ Explanation e = w.explain(context, doc);
+ if (e.isMatch()) {
+ subs.add(e);
+ } else if (c.isRequired()) {
+ subs.add(Explanation.noMatch(
+ "no match on required clause (" + c.getQuery().toString() + ")",
+ e));
+ fail = true;
+ }
+ }
+ if (fail) {
+ return Explanation.noMatch(
+ "Failure to meet condition(s) of required/prohibited clause(s)",
+ subs);
+ } else {
+ Scorer scorer = scorer(context);
+ int advanced = scorer.iterator().advance(doc);
Review comment:
*NULLPTR_DEREFERENCE:* accessing memory that is the null pointer on line 117 indirectly during the call to `IndriAndWeight.scorer(...)`.
----------------------------------------------------------------
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.
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-solr] mikemccand merged pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
mikemccand merged pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097
----------------------------------------------------------------
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.
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-solr] cammiemw commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r555482893
##########
File path: lucene/core/src/java/org/apache/lucene/search/ScoreAndDoc.java
##########
@@ -32,4 +33,9 @@ public int docID() {
public float score() {
return score;
}
+
+ @Override
+ public float smoothingScore(int docId) throws IOException {
Review comment:
I agree that this makes more sense ;-) I have added the default implementation of smoothingScore in Scorable and reverted my changes to add the smoothingScore method to all the unnecessary extending classes.
----------------------------------------------------------------
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.
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-solr] mikemccand commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-768299026
Hello @cammiemw, I ran top-level `./gradlew test` on the latest diff here, yet still see the Solr `TestFeature` failure:
```
org.apache.solr.ltr.feature.TestFeature > test suite's output saved to /l/trunk/solr/contrib/ltr/build/test-results/test/outputs/OUTPUT-org.apache.solr.ltr.feature.TestFeature.txt, copied belo\
w:
> java.lang.AssertionError: class org.apache.solr.ltr.feature.Feature$FeatureWeight$FilterFeatureScorer needs to override 'public float org.apache.lucene.search.Scorer.smoothingScore(in\
t) throws java.io.IOException'
> at __randomizedtesting.SeedInfo.seed([E634884D6EF7B762:F18DD1AC4011E7C2]:0)
> at org.junit.Assert.fail(Assert.java:89)
> at org.apache.solr.ltr.feature.TestFeature.testFilterFeatureScorerOverridesScorerMethods(TestFeature.java:46)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64)
> at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:564)
> at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1754)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:942)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:978)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:992)
> at org.apache.lucene.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:44)
> at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
> at org.apache.lucene.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:45)
> at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
> at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
> at org.junit.rules.RunRules.evaluate(RunRules.java:20)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:370)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:819)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:470)
> at com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:951)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:836)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:887)
> at com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:898)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule$1.evaluate(SystemPropertiesRestoreRule.java:57)
> at org.junit.rules.RunRules.evaluate(RunRules.java:20)
> at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at org.apache.lucene.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:38)
> at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
> at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at org.apache.lucene.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:51)
> at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
> at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
> at org.apache.lucene.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:47)
> at org.junit.rules.RunRules.evaluate(RunRules.java:20)
> at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:370)
> at com.carrotsearch.randomizedtesting.ThreadLeakControl.lambda$forkTimeoutingTask$0(ThreadLeakControl.java:826)
> at java.base/java.lang.Thread.run(Thread.java:832)
2> NOTE: reproduce with: gradlew test --tests TestFeature.testFilterFeatureScorerOverridesScorerMethods -Dtests.seed=E634884D6EF7B762 -Dtests.slow=true -Dtests.badapples=true -Dtests.locale=\
om-KE -Dtests.timezone=Pacific/Auckland -Dtests.asserts=true -Dtests.file.encoding=UTF-8
2> NOTE: test params are: codec=Asserting(Lucene90): {}, docValues:{}, maxPointsInLeafNode=111, maxMBSortInHeap=6.800048371274569, sim=Asserting(RandomSimilarity(queryNorm=true): {}), locale\
=om-KE, timezone=Pacific/Auckland
2> NOTE: Linux 5.9.8-arch1-1 amd64/Oracle Corporation 15.0.1 (64-bit)/cpus=128,threads=1,free=250395088,total=270532608
2> NOTE: All tests run in this JVM: [TestFeature]
```
----------------------------------------------------------------
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.
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-solr] cammiemw commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r532905011
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndQuery.java
##########
@@ -0,0 +1,22 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+import java.util.List;
+
+/** A Query that matches documents matching combinations of
+ * {@link TermQuery}s or other IndriAndQuerys.
+ */
+public class IndriAndQuery extends IndriQuery {
Review comment:
I agree the naming is confusing. I have taken the naming schema as well as the logic from the original Indri search engine implementation. The issue with renaming it is that there is already IndriOrQuery, which I have created and hope to be able to add at a future time. I will continue to think about whether there is a better name for the IndriAndQuery 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.
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-solr] cammiemw commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r532905390
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndScorer.java
##########
@@ -0,0 +1,59 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+import java.util.List;
+
+/** Combines scores of subscorers. If a subscorer does not contain
+ * the docId, a smoothing score is calculated for that
+ * document/subscorer combination.
+ */
+public class IndriAndScorer extends IndriDisjunctionScorer {
+
+ protected IndriAndScorer(Weight weight, List<Scorer> subScorers,
+ ScoreMode scoreMode, float boost) throws IOException {
+ super(weight, subScorers, scoreMode, boost);
+ }
+
+ @Override
+ public float score(List<Scorer> subScorers) throws IOException {
+ int docId = this.docID();
+ return scoreDoc(subScorers, docId);
+ }
+
+ @Override
+ public float smoothingScore(List<Scorer> subScorers, int docId)
+ throws IOException {
+ return scoreDoc(subScorers, docId);
+ }
+
+ private float scoreDoc(List<Scorer> subScorers, int docId)
+ throws IOException {
+ double score = 0;
+ double boostSum = 0.0;
+ for (Scorer scorer : subScorers) {
+ if (scorer instanceof IndriScorer) {
+ IndriScorer indriScorer = (IndriScorer) scorer;
+ int scorerDocId = indriScorer.docID();
+ //If the query exists in the document, score the document
+ //Otherwise, compute a smoothing score, which acts like an idf
+ //for subqueries/terms
+ if (docId == scorerDocId) {
Review comment:
Done :-)
----------------------------------------------------------------
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.
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-solr] cammiemw commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r532927287
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndScorer.java
##########
@@ -0,0 +1,59 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+import java.util.List;
+
+/** Combines scores of subscorers. If a subscorer does not contain
+ * the docId, a smoothing score is calculated for that
+ * document/subscorer combination.
+ */
+public class IndriAndScorer extends IndriDisjunctionScorer {
Review comment:
Yes, I do hope to be able to add additional Indri query types that will extend the IndriDisjuctionScorer.
----------------------------------------------------------------
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.
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-solr] cammiemw commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-759133439
Hi @mikemccand! I think the issues are fixed in my latest commit. Let me know if there is anything else that you think I need. 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.
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-solr] mikemccand commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r555792974
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndWeight.java
##########
@@ -0,0 +1,124 @@
+/*
+ * 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.search;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.lucene.index.LeafReaderContext;
+
+/**
+ * The Weight for IndriAndQuery, used to normalize, score and explain these
+ * queries.
+ */
+public class IndriAndWeight extends Weight {
+
+ private final IndriAndQuery query;
+ private final ArrayList<Weight> weights;
+ private final ScoreMode scoreMode;
+ private final float boost;
+
+ public IndriAndWeight(IndriAndQuery query, IndexSearcher searcher,
+ ScoreMode scoreMode, float boost) throws IOException {
+ super(query);
+ this.query = query;
+ this.boost = boost;
+ this.scoreMode = scoreMode;
+ weights = new ArrayList<>();
+ for (BooleanClause c : query) {
+ Weight w = searcher.createWeight(c.getQuery(), scoreMode, 1.0f);
+ weights.add(w);
+ }
+ }
+
+ private Scorer getScorer(LeafReaderContext context) throws IOException {
+ List<Scorer> subScorers = new ArrayList<>();
+
+ for (Weight w : weights) {
+ Scorer scorer = w.scorer(context);
+ if (scorer != null) {
+ subScorers.add(scorer);
+ }
+ }
+
+ if (subScorers.isEmpty()) {
+ return null;
+ }
+ Scorer scorer = subScorers.get(0);
+ if (subScorers.size() > 1) {
+ scorer = new IndriAndScorer(this, subScorers, scoreMode, boost);
+ }
+ return scorer;
+ }
+
+ @Override
+ public Scorer scorer(LeafReaderContext context) throws IOException {
+ return getScorer(context);
+ }
+
+ @Override
+ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException {
+ Scorer scorer = getScorer(context);
+ if (scorer != null) {
+ BulkScorer bulkScorer = new DefaultBulkScorer(scorer);
+ return bulkScorer;
+ }
+ return null;
+ }
+
+ @Override
+ public boolean isCacheable(LeafReaderContext ctx) {
+ for (Weight w : weights) {
+ if (w.isCacheable(ctx) == false) return false;
+ }
+ return true;
+ }
+
+ @Override
+ public Explanation explain(LeafReaderContext context, int doc)
+ throws IOException {
+ List<Explanation> subs = new ArrayList<>();
+ boolean fail = false;
+ Iterator<BooleanClause> cIter = query.iterator();
+ for (Iterator<Weight> wIter = weights.iterator(); wIter.hasNext();) {
+ Weight w = wIter.next();
+ BooleanClause c = cIter.next();
+ Explanation e = w.explain(context, doc);
+ if (e.isMatch()) {
+ subs.add(e);
+ } else if (c.isRequired()) {
+ subs.add(Explanation.noMatch(
+ "no match on required clause (" + c.getQuery().toString() + ")",
+ e));
+ fail = true;
+ }
+ }
+ if (fail) {
+ return Explanation.noMatch(
+ "Failure to meet condition(s) of required/prohibited clause(s)",
+ subs);
+ } else {
+ Scorer scorer = scorer(context);
+ int advanced = scorer.iterator().advance(doc);
Review comment:
Hmm, can `scorer` ever be null? Other Lucene queries can/do return `null` scorers to indicate that there are no matches for this query in this segment, and callers need to check for that. But maybe in this context it never happens?
----------------------------------------------------------------
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.
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-solr] mikemccand commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-758675804
Thank you @cammiemw!
Looks like there are some small style problems with your PR -- see the above `Gradle precommit` for details. We now enforce consistent styling, and I think you can run `gradle tidy` to re-style automatically.
----------------------------------------------------------------
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.
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-solr] cammiemw edited a comment on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw edited a comment on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-736443177
Thanks for your comments and quick response time @mikemccand! I think I have addressed and replied to each one.
An alternative to touching so many low-level lucene classes would be to create a separate interface with only the smoothingScore method. Then the only Lucene class that I would need to change would be TermScorer to implement that interface and add the smoothing score method. This might be less risky although perhaps not as clean.
As I mentioned in some of my comments, I am hoping that can be a base for suggesting to add more Indri content. I have several more query types and an Indri parser implemented already, but I thought it might make sense to start with a small piece of functionality so I could hopefully learn the code base and process.
----------------------------------------------------------------
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.
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-solr] cammiemw commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r555482893
##########
File path: lucene/core/src/java/org/apache/lucene/search/ScoreAndDoc.java
##########
@@ -32,4 +33,9 @@ public int docID() {
public float score() {
return score;
}
+
+ @Override
+ public float smoothingScore(int docId) throws IOException {
Review comment:
I agree that this makes more sense :-) I have added the default implementation of smoothingScore in Scorable and reverted my changes to add the smoothingScore method to all the unnecessary extending classes.
----------------------------------------------------------------
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.
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-solr] cammiemw commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-770074489
Hi @mikemccand! The smoothingScore method requires a docId because the most important implementation of smoothingScore is in TermScorer, which uses the postingsEnum to get the docId. However, in the case of the smoothing score, there will be no posting for the term in the document. Let me know if you think a different way of getting the docId would work better.
Thanks so much for all your help and time throughout this process! I am very excited about this change and hope that I can continue to add more.
----------------------------------------------------------------
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.
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-solr] mikemccand commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-770011220
Thanks @cammiemw, that test now passes again!
I ran top-level tests multiple times and each time, a solr test failed for what looked like (not certain) unrelated to this great change. On the 4th try, all solr tests finally passed.
And `./gradlew precommit` passes as well.
I will push soon! Thank you for persisting @cammiemw and sorry for the slow iterations here. I am excited that we are cross-fertilizing these two impactful open-source search engines!
----------------------------------------------------------------
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.
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-solr] cammiemw commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-764873350
Thanks @mikemccand! I have added an entry in CHANGES.txt under API changes.
I am hoping to be able to add more of the Indri search engine functionality that build upon these smoothingScore changes in the near future! Once this PR is merged, I would love to start working on adding a few more Indri operators such as the Indri OR, Indri Weighted Sum, and Indri Weighted And, and add either an Indri Query Parser or add to an existing lucene query parser so that it is easier to use these operators. Do you think it makes sense to open a new jira ticket for these changes once this PR is merged? About how long does the process or merging a PR take?
Thank you!
----------------------------------------------------------------
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.
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-solr] cammiemw commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r532907950
##########
File path: lucene/core/src/java/org/apache/lucene/search/Scorable.java
##########
@@ -30,6 +30,13 @@
* Returns the score of the current document matching the query.
*/
public abstract float score() throws IOException;
+
+ /**
+ * Returns the smoothing score of the current document matching the query.
+ * This score is used when the query/term does not appear in the document.
+ * This can return 0 or a smoothing score.
Review comment:
I have added more detail and a paper the describes the motivation of the smoothing score. The description of how the smoothing score is used is at the bottom of page 11 in the paper. It is important to note that most of the explanation has to do with when the score is a product. Even though the IndriAndScorer does not use a product, the smoothing score is still helpful for acting like an idf. Additionally, there are many more Indri operators that I would like to add that do use a product.
----------------------------------------------------------------
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.
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-solr] cammiemw commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-764873350
Thanks @mikemccand! I have added an entry in CHANGES.txt under API changes.
I am hoping to be able to add more of the Indri search engine functionality that build upon these smoothingScore changes in the near future! Once this PR is merged, I would love to start working on adding a few more Indri operators such as the Indri OR, Indri Weighted Sum, and Indri Weighted And, and add either an Indri Query Parser or add to an existing lucene query parser so that it is easier to use these operators. Do you think it makes sense to open a new jira ticket for these changes once this PR is merged? About how long does the process or merging a PR take?
Thank you!
----------------------------------------------------------------
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.
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-solr] mikemccand commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r532220806
##########
File path: lucene/core/src/java/org/apache/lucene/search/CachingCollector.java
##########
@@ -59,6 +59,11 @@
@Override
public final float score() { return score; }
+ @Override
+ public float smoothingScore(int docId) throws IOException {
+ return 0;
Review comment:
Hmm, should we also cache the `smoothingScore` for this hit?
Or, if we will keep it at returning `0`, couldn't we remove this impl and inherit the default from `Scorable`?
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndScorer.java
##########
@@ -0,0 +1,59 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+import java.util.List;
+
+/** Combines scores of subscorers. If a subscorer does not contain
+ * the docId, a smoothing score is calculated for that
+ * document/subscorer combination.
+ */
+public class IndriAndScorer extends IndriDisjunctionScorer {
Review comment:
Do we expect to have other scorers that need to extend the `IndriDisjunctionScorer`? Here we have only one such subclass, so I wonder if we could just fold all of this into a single `IndriDisjunctionScorer` class? Or, are you laying bedrock for future additional query types?
##########
File path: lucene/core/src/java/org/apache/lucene/search/FilterScorable.java
##########
@@ -46,6 +46,11 @@ public float score() throws IOException {
return in.score();
}
+ @Override
+ public float smoothingScore(int docId) throws IOException {
+ return 0;
Review comment:
Hmm should this be `return in.smoothingScore(docId)` instead?
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndQuery.java
##########
@@ -0,0 +1,22 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+import java.util.List;
+
+/** A Query that matches documents matching combinations of
+ * {@link TermQuery}s or other IndriAndQuerys.
+ */
+public class IndriAndQuery extends IndriQuery {
+
+ public IndriAndQuery(List<BooleanClause> clauses) {
Review comment:
In general, `BooleanClause` can hold any Lucene `Query` implementation, but it looks like we are only supporting `TermQuery` and other `IndriAndQuery` clauses (just from reading the javadoc)? If so, should we check/enforce this?
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndQuery.java
##########
@@ -0,0 +1,22 @@
+package org.apache.lucene.search;
Review comment:
Could you add the standard Apache copyright header here and in all the new classes?
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndScorer.java
##########
@@ -0,0 +1,59 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+import java.util.List;
+
+/** Combines scores of subscorers. If a subscorer does not contain
+ * the docId, a smoothing score is calculated for that
+ * document/subscorer combination.
+ */
+public class IndriAndScorer extends IndriDisjunctionScorer {
+
+ protected IndriAndScorer(Weight weight, List<Scorer> subScorers,
+ ScoreMode scoreMode, float boost) throws IOException {
+ super(weight, subScorers, scoreMode, boost);
+ }
+
+ @Override
+ public float score(List<Scorer> subScorers) throws IOException {
+ int docId = this.docID();
+ return scoreDoc(subScorers, docId);
+ }
+
+ @Override
+ public float smoothingScore(List<Scorer> subScorers, int docId)
+ throws IOException {
+ return scoreDoc(subScorers, docId);
+ }
+
+ private float scoreDoc(List<Scorer> subScorers, int docId)
+ throws IOException {
+ double score = 0;
+ double boostSum = 0.0;
+ for (Scorer scorer : subScorers) {
+ if (scorer instanceof IndriScorer) {
+ IndriScorer indriScorer = (IndriScorer) scorer;
+ int scorerDocId = indriScorer.docID();
+ //If the query exists in the document, score the document
+ //Otherwise, compute a smoothing score, which acts like an idf
+ //for subqueries/terms
+ if (docId == scorerDocId) {
Review comment:
Can you factor out the 2nd two statements under each of the `true` and `false` clauses here? I.e., only the first line needs to be conditional?
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriDisjunctionScorer.java
##########
@@ -0,0 +1,65 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+import java.util.List;
+
+/**
+ * The Indri implemenation of a disjunction scorer which stores the subscorers
+ * for for the child queries. The score and smoothingScore methods use the list
Review comment:
Remove extra `for`?
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriScorer.java
##########
@@ -0,0 +1,37 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+
+/**
+ * The Indri parent scorer that stores the boost so that
+ * IndriScorers can use the boost outside of the term.
+ *
+ */
+abstract public class IndriScorer extends Scorer {
+
+ private float boost;
Review comment:
Hmm is this (to store `boost`) the only reason to have a separate `IndriScorer`? If I remember right, Lucene used to apply `boost` similarly (every scorer kept track of it) but at one point we moved all boosting to a dedicated `BoostQuery`.
##########
File path: lucene/core/src/java/org/apache/lucene/search/Scorable.java
##########
@@ -30,6 +30,13 @@
* Returns the score of the current document matching the query.
*/
public abstract float score() throws IOException;
+
+ /**
+ * Returns the smoothing score of the current document matching the query.
+ * This score is used when the query/term does not appear in the document.
+ * This can return 0 or a smoothing score.
Review comment:
Maybe link to Indri paper that describes/motivates this?
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndQuery.java
##########
@@ -0,0 +1,22 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+import java.util.List;
+
+/** A Query that matches documents matching combinations of
+ * {@link TermQuery}s or other IndriAndQuerys.
+ */
+public class IndriAndQuery extends IndriQuery {
Review comment:
Normally in Lucene `AND` implies `MUST`, i.e. required clauses.
But this query is actually disjunctive, right? Documents will match even if they are missing some of the terms. Should we name it `IndriOrQuery` maybe? Or, `IndriBooleanQuery`?
##########
File path: lucene/core/src/java/org/apache/lucene/search/similarities/IndriDirichletSimilarity.java
##########
@@ -0,0 +1,108 @@
+/*
+ * ===============================================================================================
+ * Copyright (c) 2019 Carnegie Mellon University and University of Massachusetts. All Rights
+ * Reserved.
+ *
+ * Use of the Lemur Toolkit for Language Modeling and Information Retrieval is subject to the terms
+ * of the software license set forth in the LICENSE file included with this software, and also
+ * available at http://www.lemurproject.org/license.html
+ *
+ * ================================================================================================
+ */
+package org.apache.lucene.search.similarities;
+
+import java.util.List;
+import java.util.Locale;
+
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.similarities.BasicStats;
+import org.apache.lucene.search.similarities.LMSimilarity;
+
+/**
+ * Bayesian smoothing using Dirichlet priors as implemented in the Indri Search
+ * engine (http://www.lemurproject.org/indri.php). Indri Dirichelet Smoothing!
+ * tf_E + mu*P(t|D) P(t|E)= ------------------------ documentLength + documentMu
+ * mu*P(t|C) + tf_D where P(t|D)= --------------------- doclen + mu
+ */
Review comment:
Maybe, add a few words giving some intuition about what `mu` does? It looks like it roughly lets you tune how important document length is in the scoring?
Also, the formatting of the above equations looks like it got garbled? You will need to use html/javadoc markup to make the formatting survive future developers viewing in browser...
----------------------------------------------------------------
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.
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-solr] cammiemw commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r532933758
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriScorer.java
##########
@@ -0,0 +1,37 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+
+/**
+ * The Indri parent scorer that stores the boost so that
+ * IndriScorers can use the boost outside of the term.
+ *
+ */
+abstract public class IndriScorer extends Scorer {
+
+ private float boost;
Review comment:
I did this because I apply the boost in the scorer rather than in the similarity (such as in LMDirichletSimilarity), and I divide by the sum of the boosts. I originally did this to exactly match the Indri scores; however, this is not a huge priority. I don't have much issue with dropping this if it doesn't fit in the Lucene workflow well.
----------------------------------------------------------------
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.
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-solr] mikemccand commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-770069329
Thanks @cammiemw!
One thing I noticed after I pushed was the new `Scorable.smothingScore(int docId)` method: why does this take the `int docId` parameter? Shouldn't it take no parameters, like `score()`, and always return smoothing score for the "current" `docId` instead?
----------------------------------------------------------------
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.
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-solr] cammiemw commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r532905253
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndQuery.java
##########
@@ -0,0 +1,22 @@
+package org.apache.lucene.search;
Review comment:
Done :-)
----------------------------------------------------------------
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.
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-solr] cammiemw commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r532908661
##########
File path: lucene/core/src/java/org/apache/lucene/search/similarities/IndriDirichletSimilarity.java
##########
@@ -0,0 +1,108 @@
+/*
+ * ===============================================================================================
+ * Copyright (c) 2019 Carnegie Mellon University and University of Massachusetts. All Rights
+ * Reserved.
+ *
+ * Use of the Lemur Toolkit for Language Modeling and Information Retrieval is subject to the terms
+ * of the software license set forth in the LICENSE file included with this software, and also
+ * available at http://www.lemurproject.org/license.html
+ *
+ * ================================================================================================
+ */
+package org.apache.lucene.search.similarities;
+
+import java.util.List;
+import java.util.Locale;
+
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.similarities.BasicStats;
+import org.apache.lucene.search.similarities.LMSimilarity;
+
+/**
+ * Bayesian smoothing using Dirichlet priors as implemented in the Indri Search
+ * engine (http://www.lemurproject.org/indri.php). Indri Dirichelet Smoothing!
+ * tf_E + mu*P(t|D) P(t|E)= ------------------------ documentLength + documentMu
+ * mu*P(t|C) + tf_D where P(t|D)= --------------------- doclen + mu
+ */
Review comment:
I tried adding more formatting and a description of mu. Let me know if you would like to see anything different or more. 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.
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-solr] cpoerschke commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r792916357
##########
File path: solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFeature.java
##########
@@ -33,6 +33,9 @@ public void testFilterFeatureScorerOverridesScorerMethods() {
// the FilterFeatureScorer may simply inherit Scorer's default implementation
if (scorerClassMethod.getName().equals("twoPhaseIterator")) continue;
+ // the FilterFeatureScorer may simply inherit Scorer's default implementation
+ if (scorerClassMethod.getName().equals("smoothingScore")) continue;
+
Review comment:
This surprised me. Created https://issues.apache.org/jira/browse/SOLR-15958 and (draft) https://github.com/apache/solr/pull/567 to consider further.
--
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-solr] cammiemw commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-768751214
Hi @mikemccand, apologies for the failed test! Unfortunately, I cannot replicate the error in my environments and gradle test as well as that specific test come back successful. I am going to keep digging into what is different in my local. For right now, I looked into the code and could see what was causing the error, so I pushed the change that I think fixes it. If that fails for you, I will need to get my local environment working so that I get the same results that you do. Thanks so much for your time!
----------------------------------------------------------------
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.
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-solr] mikemccand commented on a change in pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#discussion_r542492206
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriQuery.java
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.search;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * A Basic abstract query that all IndriQueries can extend to implement
+ * toString, equals, getClauses, and iterator.
+ *
+ */
+public abstract class IndriQuery extends Query
Review comment:
Random question: will `IndriQuery` take advantage of Block MAX Weak And optimization? The added `smoothingScore` must alter the optimization logic to find the min block score to skip to?
I think it's OK to defer BMW to followon improvements, as long as it is not kicking in incorrectly here.
##########
File path: lucene/core/src/java/org/apache/lucene/search/ScoreAndDoc.java
##########
@@ -32,4 +33,9 @@ public int docID() {
public float score() {
return score;
}
+
+ @Override
+ public float smoothingScore(int docId) throws IOException {
Review comment:
Hmm, why not add a default method in the `Scorable` interface to `return 0`? Then we don't have to add this default method in all these subclasses?
##########
File path: lucene/core/src/java/org/apache/lucene/search/Scorable.java
##########
@@ -30,6 +30,13 @@
* Returns the score of the current document matching the query.
*/
public abstract float score() throws IOException;
+
+ /**
+ * Returns the smoothing score of the current document matching the query.
+ * This score is used when the query/term does not appear in the document.
+ * This can return 0 or a smoothing score.
Review comment:
Great, thanks.
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndQuery.java
##########
@@ -0,0 +1,22 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+import java.util.List;
+
+/** A Query that matches documents matching combinations of
+ * {@link TermQuery}s or other IndriAndQuerys.
+ */
+public class IndriAndQuery extends IndriQuery {
Review comment:
OK let's keep the (somewhat confusing) naming for now. Naming is the hardest part!!
##########
File path: lucene/core/src/java/org/apache/lucene/search/IndriAndQuery.java
##########
@@ -0,0 +1,22 @@
+package org.apache.lucene.search;
+
+import java.io.IOException;
+import java.util.List;
+
+/** A Query that matches documents matching combinations of
+ * {@link TermQuery}s or other IndriAndQuerys.
+ */
+public class IndriAndQuery extends IndriQuery {
+
+ public IndriAndQuery(List<BooleanClause> clauses) {
Review comment:
Hello, any comment on this one?
----------------------------------------------------------------
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.
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-solr] cammiemw commented on pull request #2097: LUCENE-9537
Posted by GitBox <gi...@apache.org>.
cammiemw commented on pull request #2097:
URL: https://github.com/apache/lucene-solr/pull/2097#issuecomment-736443177
An alternative to touching so many low-level lucene classes would be to create a separate interface with only the smoothingScore method. Then the only Lucene class that I would need to change would be TermScorer to implement that interface and add the smoothing score method. This might be less risky although perhaps not as clean.
As I mentioned in some of my comments, I am hoping that can be a base for suggesting to add more Indri content. I have several more query types and an Indri parser implemented already, but I thought it might make sense to start with a small piece of functionality so I could hopefully learn the code base and process.
----------------------------------------------------------------
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.
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