BlendedInfixSuggester bad score calculus for certain suggestion weights [LUCENE-8343]

asfimport commented 6 years ago

Currently the BlendedInfixSuggester return a (long) score to rank the suggestions. This score is calculated as a multiplication between :

long Weight : the suggestion weight, coming from a document field, it can be any long value ( including 1, 0,.. )

double Coefficient : 0<=x<=1, calculated based on the position match, earlier the better

The resulting score is a long, which means that at the moment, any weight<10 can bring inconsistencies.

Edge cases Weight =1 Score = 1( if we have a match at the beginning of the suggestion) or 0 ( for any other match)

Weight =0 Score = 0 ( independently of the position match coefficient)

Migrated from LUCENE-8343 by Alessandro Benedetti (@alessandrobenedetti), 1 vote, resolved Sep 11 2018 Attachments: LUCENE-8343.patch (versions: 6) Linked issues:

11193
- 9394

Pull requests: https://github.com/apache/lucene-solr/pull/391, https://github.com/apache/lucene-solr/pull/393

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

Patch + edge cases tests is ready for review

asfimport commented 6 years ago

Lucene/Solr QA (migrated from JIRA)

✔ +1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
master Compile Tests
+1	compile	0m 51s	master passed
Patch Compile Tests
+1	compile	0m 47s	the patch passed
+1	javac	0m 47s	the patch passed
+1	Release audit (RAT)	0m 47s	the patch passed
+1	Check forbidden APIs	0m 47s	the patch passed
+1	Validate source patterns	0m 47s	the patch passed
Other Tests
+1	unit	1m 41s	suggest in the patch passed.
	6m 20s

Subsystem	Report/Notes
JIRA Issue	LUCENE-8343
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12926078/LUCENE-8343.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene1-us-west 3.13.0-88-generic #135-Ubuntu SMP Wed Jun 8 21:10:42 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / 3dc4fa1
ant	version: Apache Ant(TM) version 1.9.3 compiled on April 8 2014
Default Java	1.8.0_172
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/22/testReport/
modules	C: lucene/suggest U: lucene/suggest
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/22/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

asfimport commented 6 years ago

Cassandra Targett (@ctargett) (migrated from JIRA)

@alessandrobenedetti - you mentioned on solr-user mailing list that the patch here had Ref Guide changes in it, but I don't see them?

asfimport commented 6 years ago

Adrien Grand (@jpountz) (migrated from JIRA)

I'm not very familiar with this suggester so I might be missing obvious things, but I don't understand why turning weight=0 into 1 is right, or why it's better to multiply the coefficient by 10. It might even introduce overflows?

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

Hi @ctargett,

I did the change and pushed, so they were just in the Jira associated Pull Request : GitHub Pull Request #391 I just uploaded the patch as well. You can take a look now ( I think the Github Pull Request is easier to read, but feel free to use the patch at your convenience)|

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

Hi @jpountz,

thanks for your time, I can give you a quick explanation here:

The (positional) coefficient should be a double 0<=x<=1 calculated with 3 possible formulas from the position of the first matching query term in the suggestion ( linear doesn't respect that constraint and can go negative for postion which are farer than 10 positions from the beginning ) :

position_linear: (1 – 0.10*position): Matches to the start will be given a higher score (Default)
position_reciprocal: 1/(1+position): Matches to the start will be given a score which decay faster than linear
position_exponential_reciprocal: 1/pow(1+position,exponent): Matches to the start will be given a score which decay faster than reciprocal

To answer your questions :

1) "turning weight=0 into 1" , so this is an interesting one : You don't want all your weights to be 0 for the BlendedInfixSuggester because you would just flat to 0 the positional score of the suggestion, which is the only reason to use the Blended Infix ( if you are not interested in the positional score for the suggestion, you should use the parent suggester : AnalyzingInfixSuggester) If you don't configure the weight field ( which is not and shouldn't be mandatory) all your weights go to 0s (org.apache.lucene.search.suggest.DocumentDictionary.DocumentInputIterator#getWeight ) and your BlendedInfixSuggester doesn't blend anything anymore scoring each suggestion a constant 0. That was the reason to move the weight 0 to the smallest bigger value ( which in a long data type is 1) . With that fix you limit the ability of a user to move certain suggestions to 0 weight ( they can just drop them to 1 weight) , but you gain a good bug fix for the missing weight field scenario.

2) So the chosen of 10 was completely arbitrary to get at least 10 possible ranked outcomes out of the positional coefficient. You may end up in overflows if :

the weight is already big enough. You are right maybe we can apply that scaling factor only if the weight is small.

The overflow according to my analysis can not come from the coefficient, because the edge cases for linear are : 1 - where input position is 0 ~~2.1474836370000002E8~~ where input position is Integer.MAX_VALUE ( which is not going to be achievable as Strings full length are maxed by that value)

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

In the meantime I attached the patch with the overflow edge case fixed and a better handling of the weight just when it is too small.

Happy to discuss the implications of "turning weight=0 into 1" with the community!

asfimport commented 6 years ago

Adrien Grand (@jpountz) (migrated from JIRA)

Thanks for the explanations. I 'm not familiar enough with suggesters to move this forward, but this patch still feels hacky to me. It looks like it is working around index-time issues and the fact that the weight is a long rather than a double.

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

Hi Adrien, I theoretically agree with you. The reason I structured the patch this way is because what I noticed so far in my contributions is that a contribution is much more likely to be reviewed and accepted if it fixes a bug with the minimal impact as possible and involving less classes as possible.

The problem here is indeed related the data type of :

the suggestion score ( should be double)
and weght ( should be Long as 0 must be considered different from null)

I would be more than happy to contribute that, but my feeling is that a patch that span over a lot of different classes and areas, would be ignored with the final result of the bug(s) to remain there. Happy if you( the community in general) contradict me and I will proceed with the data types change approach :)

asfimport commented 6 years ago

Adrien Grand (@jpountz) (migrated from JIRA)

@mikemccand What do you think would be the right fix?

asfimport commented 6 years ago

Lucene/Solr QA (migrated from JIRA)

✔ +1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
master Compile Tests
+1	compile	0m 52s	master passed
Patch Compile Tests
+1	compile	0m 47s	the patch passed
+1	javac	0m 47s	the patch passed
+1	Release audit (RAT)	0m 51s	the patch passed
+1	Check forbidden APIs	0m 47s	the patch passed
+1	Validate source patterns	0m 47s	the patch passed
+1	Validate ref guide	0m 47s	the patch passed
Other Tests
+1	unit	1m 36s	suggest in the patch passed.
	10m 2s

Subsystem	Report/Notes
JIRA Issue	LUCENE-8343
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12926941/LUCENE-8343.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns validaterefguide
uname	Linux lucene1-us-west 3.13.0-88-generic #135-Ubuntu SMP Wed Jun 8 21:10:42 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / 36b7cdd
ant	version: Apache Ant(TM) version 1.9.3 compiled on April 8 2014
Default Java	1.8.0_172
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/30/testReport/
modules	C: lucene/suggest solr/solr-ref-guide U: .
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/30/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

First of all, thanks again Adrien for your time. I have done the work for the data type migration approach here : https://github.com/apache/lucene-solr/pull/398 The patch is affecting many more files as expected, but the strictly BlendedInfixSuggester fix is much more elegant.

The drawbacks are that :

much more attention is needed to review the new patch
it should be pretty safe, but introducing nulls around, I never feel fully comfortable unless I am super confident of my tests

in case this approach is preferred and someone from the community commit to take care of the review process, I am more than happy to spend more effort in this and make it producton ready!

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

I agree the proposed fix seems somewhat hacky; could we maybe catch these "invalid" weight values during indexing and throw an exception, asking the user to pick more appropriate weights?

And also +1 to discussing switching to double instead of long for our suggesters; maybe open a spinoff issue to discuss?

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

Hi @mikemccand, thank you for your feedback! I definitely agree the path with data type migration is the best route to solve the bug(s) elegantly. I already attached a Pull Request : https://github.com/apache/lucene-solr/pull/398

Which :

move the weight long to Long ( preserving null values with difference to 0 values)
move the suggestion score to double ( preserving the precision)

It is ready for review and after a first feedback I can work more on that to make it production ready!

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

Woops sorry @alessandrobenedetti; I missed the PR above. I'll have a look.

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

I like the change in the PR; I left small comments there, that we should document that having a weight is optional so returning null from weight means there is no weight.

Maybe we should do it only for 8.x since it's a substantial API change?

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

Hi @mikemccand, thanks for your review ! I followed your suggestions and I updated the Pull Request ( fixing a recent merge conflict). Feel free to check the additional comments in there.

I agree to bring this to 8.x . When we are close to an acceptable status let me know and I will go on with refinements and double checks to be production ready.

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

Any update on this ? Can I help anyway with this to move forward ?

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

Hi @alessandrobenedetti, sorry for the delay. The PR looks good to me; do all tests (including Solr's) pass? If there are no objections, I'll push this in a few days.

Thanks @alessandrobenedetti!

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

Hi @mikemccand, I just merged the branch with the latest master, no conflicts : https://github.com/apache/lucene-solr/pull/398

I am pushing now the patch here in the way all tests will be executed by Jenkins automatically. If this doesn't work in the next day or so, I will proceed doing the tests locally as well.

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

Thank you @alessandrobenedetti!

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

Hi @alessandrobenedetti, it looks like there are some ant precommit failures with the patch:

[forbidden-apis] Forbidden method invocation: java.lang.Long#<init>(long) [Constructors for wrapper classes of Java primitives should be avoided in fav\

or of the public static methods available or autoboxing]

[forbidden-apis]   in org.apache.lucene.search.suggest.fst.FSTCompletionTest (FSTCompletionTest.java:34)

[forbidden-apis] Forbidden method invocation: java.lang.Long#<init>(long) [Constructors for wrapper classes of Java primitives should be avoided in fav\

or of the public static methods available or autoboxing]

[forbidden-apis]   in org.apache.lucene.search.suggest.Input (Input.java:57)

[forbidden-apis] Forbidden method invocation: java.lang.Long#<init>(long) [Constructors for wrapper classes of Java primitives should be avoided in fav\

or of the public static methods available or autoboxing]

[forbidden-apis]   in org.apache.lucene.search.suggest.PersistenceTest (PersistenceTest.java:82)

[forbidden-apis] Forbidden method invocation: java.lang.Long#<init>(long) [Constructors for wrapper classes of Java primitives should be avoided in fav\

or of the public static methods available or autoboxing]

[forbidden-apis]   in org.apache.lucene.search.suggest.analyzing.FuzzySuggesterTest (FuzzySuggesterTest.java:971)

[forbidden-apis] Forbidden method invocation: java.lang.Long#<init>(long) [Constructors for wrapper classes of Java primitives should be avoided in fav\

or of the public static methods available or autoboxing]

[forbidden-apis]   in org.apache.lucene.search.suggest.FileDictionaryTest (FileDictionaryTest.java:88)

[forbidden-apis] Forbidden method invocation: java.lang.Long#<init>(long) [Constructors for wrapper classes of Java primitives should be avoided in fav\

or of the public static methods available or autoboxing]

[forbidden-apis]   in org.apache.lucene.search.suggest.FileDictionaryTest (FileDictionaryTest.java:110)

[forbidden-apis] Forbidden method invocation: java.lang.Long#<init>(long) [Constructors for wrapper classes of Java primitives should be avoided in fav\

or of the public static methods available or autoboxing]

[forbidden-apis]   in org.apache.lucene.search.suggest.DocumentValueSourceDictionaryTest (DocumentValueSourceDictionaryTest.java:508)

[forbidden-apis] Forbidden method invocation: java.lang.Long#<init>(long) [Constructors for wrapper classes of Java primitives should be avoided in fav\

or of the public static methods available or autoboxing]

[forbidden-apis]   in org.apache.lucene.search.suggest.DocumentValueSourceDictionaryTest (DocumentValueSourceDictionaryTest.java:538)

[forbidden-apis] Scanned 229 class file(s) for forbidden API invocations (in 0.06s), 8 error(s).

which was a recent change to Lucene. Can you please fix these? Thanks.

asfimport commented 6 years ago

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 15 new or modified test files.
master Compile Tests
+1	compile	3m 17s	master passed
Patch Compile Tests
+1	compile	2m 53s	the patch passed
-1	javac	1m 20s	lucene_suggest generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1)
+1	Release audit (RAT)	1m 44s	the patch passed
-1	Check forbidden APIs	1m 20s	Check forbidden APIs check-forbidden-apis failed
-1	Validate source patterns	1m 20s	Check forbidden APIs check-forbidden-apis failed
-1	Validate ref guide	1m 20s	Check forbidden APIs check-forbidden-apis failed
Other Tests
+1	unit	1m 19s	suggest in the patch passed.
-1	unit	78m 10s	core in the patch failed.
	89m 45s

Reason	Tests
Failed junit tests	solr.handler.component.SuggestComponentTest
solr.handler.component.DistributedSuggestComponentTest
solr.spelling.suggest.TestBlendedInfixSuggestions

Subsystem	Report/Notes
JIRA Issue	LUCENE-8343
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12932058/LUCENE-8343.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns validaterefguide
uname	Linux lucene2-us-west.apache.org 4.4.0-112-generic #135-Ubuntu SMP Fri Jan 19 11:48:36 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / d443ed0
ant	version: Apache Ant(TM) version 1.9.6 compiled on July 8 2015
Default Java	1.8.0_172
javac	https://builds.apache.org/job/PreCommit-LUCENE-Build/48/artifact/out/diff-compile-javac-lucene_suggest.txt
Check forbidden APIs	https://builds.apache.org/job/PreCommit-LUCENE-Build/48/artifact/out/patch-check-forbidden-apis-root.txt
Validate source patterns	https://builds.apache.org/job/PreCommit-LUCENE-Build/48/artifact/out/patch-check-forbidden-apis-root.txt
Validate ref guide	https://builds.apache.org/job/PreCommit-LUCENE-Build/48/artifact/out/patch-check-forbidden-apis-root.txt
unit	https://builds.apache.org/job/PreCommit-LUCENE-Build/48/artifact/out/patch-unit-solr_core.txt
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/48/testReport/
modules	C: lucene/suggest solr/core solr/solr-ref-guide U: .
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/48/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

Hi @mikemccand, I just updated the Pull Request and patch.

I have checked ant precommit ant it seems fine to me. I have also executed some of the tests ( the one that are related with the Suggesters). There were some Solr tests failing. So I addressed that as well. let me know, happy to take care of anything that is missing. i will monitor the Jira issue and check when the robot returns the checks and tests.

asfimport commented 6 years ago

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 17 new or modified test files.
master Compile Tests
+1	compile	1m 20s	master passed
Patch Compile Tests
+1	compile	1m 14s	the patch passed
-1	javac	0m 20s	lucene_suggest generated 1 new + 5 unchanged - 0 fixed = 6 total (was 5)
+1	Release audit (RAT)	0m 27s	the patch passed
+1	Check forbidden APIs	0m 20s	the patch passed
+1	Validate source patterns	0m 20s	the patch passed
+1	Validate ref guide	0m 20s	the patch passed
Other Tests
+1	unit	1m 0s	suggest in the patch passed.
-1	unit	39m 56s	core in the patch failed.
	46m 56s

Reason	Tests
Failed junit tests	solr.handler.component.DistributedSuggestComponentTest

Subsystem	Report/Notes
JIRA Issue	LUCENE-8343
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12933585/LUCENE-8343.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns validaterefguide
uname	Linux lucene1-us-west 4.4.0-130-generic #156\~14.04.1-Ubuntu SMP Thu Jun 14 13:51:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / e264d03
ant	version: Apache Ant(TM) version 1.9.3 compiled on July 24 2018
Default Java	1.8.0_172
javac	https://builds.apache.org/job/PreCommit-LUCENE-Build/57/artifact/out/diff-compile-javac-lucene_suggest.txt
unit	https://builds.apache.org/job/PreCommit-LUCENE-Build/57/artifact/out/patch-unit-solr_core.txt
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/57/testReport/
modules	C: lucene/suggest solr/core solr/solr-ref-guide U: .
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/57/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

Thank you @alessandrobenedetti I'll have a look.

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

Thanks @alessandrobenedetti – the latest patch looks good to me, and passes and precommit and Lucene tests, but I'm hitting a few Solr failures:

   [junit4] Tests with failures [seed: 49604CF03EE2D834]:

   [junit4]   - org.apache.solr.client.solrj.response.TestSuggesterResponse.testSuggesterResponseObject

   [junit4]   - org.apache.solr.client.solrj.response.TestSuggesterResponse.testEmptySuggesterResponse

   [junit4]   - org.apache.solr.client.solrj.response.TestSuggesterResponse.testSuggesterResponseTerms

This seems to be the root cause:

   [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=TestSuggesterResponse -Dtests.method=testSuggesterResponseObject -Dtests.seed=49604CF\

03EE2D834 -Dtests.badapples=true -Dtests.locale=is-IS -Dtests.timezone=Africa/Tunis -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1

   [junit4] ERROR   0.03s J2 | TestSuggesterResponse.testSuggesterResponseObject <<<

   [junit4]    > Throwable #1: org.apache.solr.client.solrj.SolrServerException: org.apache.solr.client.solrj.SolrServerException: java.lang.Null\

PointerException

   [junit4]    >        at __randomizedtesting.SeedInfo.seed([49604CF03EE2D834:9C197DBE8EBE03B0]:0)

   [junit4]    >        at org.apache.solr.client.solrj.embedded.EmbeddedSolrServer.request(EmbeddedSolrServer.java:233)

   [junit4]    >        at org.apache.solr.client.solrj.SolrRequest.process(SolrRequest.java:194)

   [junit4]    >        at org.apache.solr.client.solrj.SolrRequest.process(SolrRequest.java:211)

   [junit4]    >        at org.apache.solr.client.solrj.response.TestSuggesterResponse.testSuggesterResponseObject(TestSuggesterResponse.java:54)

   [junit4]    >        at java.lang.Thread.run(Thread.java:745)

   [junit4]    > Caused by: org.apache.solr.client.solrj.SolrServerException: java.lang.NullPointerException

   [junit4]    >        at org.apache.solr.client.solrj.embedded.EmbeddedSolrServer.checkForExceptions(EmbeddedSolrServer.java:301)

   [junit4]    >        at org.apache.solr.client.solrj.embedded.EmbeddedSolrServer.request(EmbeddedSolrServer.java:192)

   [junit4]    >        ... 42 more

   [junit4]    > Caused by: java.lang.NullPointerException

   [junit4]    >        at org.apache.lucene.search.suggest.analyzing.AnalyzingSuggester.build(AnalyzingSuggester.java:466)

   [junit4]    >        at org.apache.lucene.search.suggest.Lookup.build(Lookup.java:194)

   [junit4]    >        at org.apache.solr.spelling.suggest.SolrSuggester.build(SolrSuggester.java:181)

   [junit4]    >        at org.apache.solr.handler.component.SuggestComponent.prepare(SuggestComponent.java:185)

   [junit4]    >        at org.apache.solr.handler.component.SearchHandler.handleRequestBody(SearchHandler.java:272)

   [junit4]    >        at org.apache.solr.handler.RequestHandlerBase.handleRequest(RequestHandlerBase.java:199)

   [junit4]    >        at org.apache.solr.core.SolrCore.execute(SolrCore.java:2541)

   [junit4]    >        at org.apache.solr.client.solrj.embedded.EmbeddedSolrServer.request(EmbeddedSolrServer.java:191)

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

@alessandrobenedetti if you can resolve the solr test failures then we can push forward here :) Thanks!

asfimport commented 6 years ago

Alessandro Benedetti (@alessandrobenedetti) (migrated from JIRA)

Hi @mikemccand, sorry for the immense delay, but I have been busy/ on holidays and I missed the August comment ! I just updated the PR and a new patch is attached . The changes added are :

AnalysingSuggester weight encoded to 0 in the FST when weight is null ( this should be ok as the AnalysingSuggester weight the suggestion with no positional information, so in this case null or 0 weight should have the same semantic)
SolrJ suggestion weight moved to double from long

let me know! Happy to work more on it if necessary!

asfimport commented 6 years ago

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
Prechecks
+1	test4tests	0m 0s	The patch appears to include 18 new or modified test files.
master Compile Tests
+1	compile	1m 37s	master passed
Patch Compile Tests
+1	compile	1m 25s	the patch passed
-1	javac	0m 19s	lucene_suggest generated 1 new + 5 unchanged - 0 fixed = 6 total (was 5)
+1	Release audit (RAT)	0m 30s	the patch passed
+1	Check forbidden APIs	0m 19s	the patch passed
+1	Validate source patterns	0m 19s	the patch passed
+1	Validate ref guide	0m 19s	the patch passed
Other Tests
+1	unit	0m 51s	suggest in the patch passed.
-1	unit	49m 59s	core in the patch failed.
+1	unit	2m 47s	solrj in the patch passed.
	60m 0s

Reason	Tests
Failed junit tests	solr.handler.component.DistributedSuggestComponentTest

Subsystem	Report/Notes
JIRA Issue	LUCENE-8343
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/12939155/LUCENE-8343.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns validaterefguide
uname	Linux lucene1-us-west 4.4.0-130-generic #156\~14.04.1-Ubuntu SMP Thu Jun 14 13:51:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / 623cdf2
ant	version: Apache Ant(TM) version 1.9.3 compiled on July 24 2018
Default Java	1.8.0_172
javac	https://builds.apache.org/job/PreCommit-LUCENE-Build/92/artifact/out/diff-compile-javac-lucene_suggest.txt
unit	https://builds.apache.org/job/PreCommit-LUCENE-Build/92/artifact/out/patch-unit-solr_core.txt
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/92/testReport/
modules	C: lucene/suggest solr/core solr/solrj solr/solr-ref-guide U: .
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/92/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

Thanks @alessandrobenedetti; I'll have a look!

asfimport commented 6 years ago

ASF subversion and git services (migrated from JIRA)

Commit e83e8ee1a42388606fffd10330ed1aeec9518098 in lucene-solr's branch refs/heads/master from @alessandrobenedetti https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e83e8ee

[LUCENE-8343] introduced weight 0 check and positional coefficient scaling + tests

asfimport commented 6 years ago

ASF subversion and git services (migrated from JIRA)

Commit 17cfa634798f96539c2535dca2e9a8f2cc0bff45 in lucene-solr's branch refs/heads/master from @alessandrobenedetti https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=17cfa63

[LUCENE-8343] documentation fix

asfimport commented 6 years ago

ASF subversion and git services (migrated from JIRA)

Commit cef9a2283e30a297b3add8e73ee6dba9492dcc57 in lucene-solr's branch refs/heads/master from @alessandrobenedetti https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=cef9a22

Merge remote-tracking branch 'upstream/master' into LUCENE-8343

asfimport commented 6 years ago

ASF subversion and git services (migrated from JIRA)

Commit 2b636e8c3adb879f0cd2cff45824e226d747b5f0 in lucene-solr's branch refs/heads/master from @alessandrobenedetti https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2b636e8

[LUCENE-8343] minor documentation fixes

asfimport commented 6 years ago

ASF subversion and git services (migrated from JIRA)

Commit e0232f104509f28126d9ce060663f87508366338 in lucene-solr's branch refs/heads/master from @alessandrobenedetti https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=e0232f1

[LUCENE-8343] weight long overflow fix + test

asfimport commented 6 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1a83a146684f20f431ba646fecb7db311e1e0afa in lucene-solr's branch refs/heads/master from @alessandrobenedetti https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1a83a14

Merge remote-tracking branch 'upstream/master' into LUCENE-8343

asfimport commented 6 years ago

ASF subversion and git services (migrated from JIRA)

Commit 398074d0f878d4ba262245e35fa1ea285e52b791 in lucene-solr's branch refs/heads/master from Mike McCandless https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=398074d

LUCENE-8343: change suggesters to use Long instead of long weight during indexing, and double instead of long score at suggest time

asfimport commented 6 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

Thanks @alessandrobenedetti; I just pushed. I also added an entry in MIGRATE.txt describing the change.

asfimport commented 6 years ago

ASF subversion and git services (migrated from JIRA)

Commit d6143867df97d61f6d4eaafcce21b2319b5de602 in lucene-solr's branch refs/heads/master from Mike McCandless https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d614386

LUCENE-8343: add CHANGES entry

apache / lucene

BlendedInfixSuggester bad score calculus for certain suggestion weights [LUCENE-8343] #9390

11193

9394