[ https://issues.apache.org/jira/browse/SOLR-15869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17464427#comment-17464427 ]
Alessandro Benedetti commented on SOLR-15869: --------------------------------------------- Hi [~cpoerschke] I see your point and I reviewed the test addition. But I have a consideration: Shouldn't we first address "Whether or not the block of code in question could/should be removed I'd like to leave out of the scope" and then proceed with the test (in case we keep it) ? I try always to follow [https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it|https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it] and in this case I guess, if we opt for "removing" the code, "we are not going to need the test" :) Let me know, and if you want to create a separate ticket for the "keep" or "not keep" discussion I'll move there for my considerations. Keep [~4nn4r] and [~diegoceccarelli] in the loop, as I discussed this with them in the past! > expand LTRRescorer.rescore test coverage > ---------------------------------------- > > Key: SOLR-15869 > URL: https://issues.apache.org/jira/browse/SOLR-15869 > Project: Solr > Issue Type: Test > Components: contrib - LTR > Reporter: Christine Poerschke > Assignee: Christine Poerschke > Priority: Minor > Time Spent: 10m > Remaining Estimate: 0h > > On the dev mailing list > https://lists.apache.org/thread/113d1yzty5ryvyt2o9msfytldv41qpgq thread > [~4nn4r] shared about the discovery of the > {{org.apache.solr.ltr.LTRRescorer#scoreSingleHit}} code block and how > {{hitUpto >= topN}} never arises. I agree that the condition currently never > evaluates to true due to how the rescore method is called: > * The Solr {{ReRankCollector}} caps the number of documents that are passed > to the {{rescore}} method: > https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.11.1/solr/core/src/java/org/apache/solr/search/ReRankCollector.java#L112-L119 > * The Lucene {{Rescorer}} API however describes topN as the number of hits to > return i.e. it is possible for firstPassTopDocs to contain more than topN > documents: > https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.11.1/lucene/core/src/java/org/apache/lucene/search/Rescorer.java#L51 > This ticket here proposes to expand {{LTRRescorer.rescore}} test coverage to > include the "more than topN documents are passed to be rescored" scenario. > (Whether or not the block of code in question could/should be removed I'd > like to leave out of the scope of this current ticket here since on a high > level not supporting {{topN != firstPassTopDocs.scoreDocs.length}} in > {{LTRRescorer}} could simplify its code but on a practical level (at least > theoretically) backwards compatibility would also need consideration and it's > possible that some custom {{ReRankCollector}} (which we don't know of) does > for some reason not cap the number of documents passed in the way > {{ReRankCollector}} does.) -- This message was sent by Atlassian Jira (v8.20.1#820001) --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org