Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix LTR query feature with phrases (and two-phase) queries #125103

Merged
merged 6 commits into from
Mar 19, 2025
Merged

Fix LTR query feature with phrases (and two-phase) queries #125103

merged 6 commits into from
Mar 19, 2025

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Mar 18, 2025

LTR Query features should verify that docs match the two-phase iterator.

@jimczi
Fix LTR query feature with phrases (and two-phase) queries
d78ef8a 
Query features should verify that docs match the two-phase iterator.
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Mar 18, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@elasticsearchmachine
Copy link
Collaborator

Hi @jimczi, I've created a changelog YAML for you.

@benwtrent
Copy link
Member

I ran the updated test 1k times locally and still got failures:

  2> REPRODUCE WITH: ./gradlew ":x-pack:plugin:ml:test" --tests "org.elasticsearch.xpack.ml.inference.ltr.QueryFeatureExtractorTests.testQueryExtractor {seed=[F4010992C5A1B31:46197D24E648D6F8]}" -Dtests.seed=F4010992C5A1B31 -Dtests.locale=vai -Dtests.timezone=Australia/ACT -Druntime.java=23
  2> java.lang.AssertionError:
    Expected: map containing ["text_score"-><1.7135582F>]
         but: map was [<number_score=1.0>]
        at __randomizedtesting.SeedInfo.seed([F4010992C5A1B31:46197D24E648D6F8]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
        at org.elasticsearch.test.ESTestCase.assertThat(ESTestCase.java:2670)

another example:

  2> REPRODUCE WITH: ./gradlew ":x-pack:plugin:ml:test" --tests "org.elasticsearch.xpack.ml.inference.ltr.QueryFeatureExtractorTests.testQueryExtractor {seed=[F4010992C5A1B31:485AED4A1B4AFFA9]}" -Dtests.seed=F4010992C5A1B31 -Dtests.locale=vai -Dtests.timezone=Australia/ACT -Druntime.java=23
  2> java.lang.AssertionError:
    Expected: map containing ["number_score"-><1.0F>]
         but: map was []
        at __randomizedtesting.SeedInfo.seed([F4010992C5A1B31:485AED4A1B4AFFA9]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
        at org.elasticsearch.test.ESTestCase.assertThat(ESTestCase.java:2670)

One more:

  2> REPRODUCE WITH: ./gradlew ":x-pack:plugin:ml:test" --tests "org.elasticsearch.xpack.ml.inference.ltr.QueryFeatureExtractorTests.testQueryExtractor {seed=[F4010992C5A1B31:5512740B09618035]}" -Dtests.seed=F4010992C5A1B31 -Dtests.locale=vai -Dtests.timezone=Australia/ACT -Druntime.java=23
  2> java.lang.AssertionError:
    Expected: map containing ["text_score"-><1.7135582F>]
         but: map was [<phrase_score=2.468971>, <text_score=0.7554128>]
        at __randomizedtesting.SeedInfo.seed([F4010992C5A1B31:5512740B09618035]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
        at org.elasticsearch.test.ESTestCase.assertThat(ESTestCase.java:2670)

benwtrent
benwtrent approved these changes Mar 19, 2025
View reviewed changes
Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@jimczi jimczi added the auto-backport Automatically create backport pull requests when merged label Mar 19, 2025
@jimczi jimczi merged commit 7c5be12 into elastic:main Mar 19, 2025
17 checks passed
@jimczi jimczi deleted the ltr_two_pass branch March 19, 2025 14:21
@jimczi jimczi mentioned this pull request Mar 19, 2025
Merged
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
❌ 8.18 Commit could not be cherrypicked due to conflicts
❌ 8.x Commit could not be cherrypicked due to conflicts
✅ 9.0

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 125103

jimczi added a commit to jimczi/elasticsearch that referenced this pull request Mar 19, 2025
@jimczi
Fix LTR query feature with phrases (and two-phase) queries (elastic#1…
53fdc42 
…25103)

Query features should verify that docs match the two-phase iterator.
elasticsearchmachine pushed a commit that referenced this pull request Mar 19, 2025
@jimczi
Fix LTR query feature with phrases (and two-phase) queries (#125103) (#…
b3ce051 
…125223)

Query features should verify that docs match the two-phase iterator.
jimczi added a commit to jimczi/elasticsearch that referenced this pull request Mar 19, 2025
@jimczi
Fix LTR query feature with phrases (and two-phase) queries (elastic#1…
bb8f7de 
…25103)

Query features should verify that docs match the two-phase iterator.
@jimczi jimczi mentioned this pull request Mar 19, 2025
Merged
jimczi added a commit to jimczi/elasticsearch that referenced this pull request Mar 19, 2025
@jimczi
Fix LTR query feature with phrases (and two-phase) queries (elastic#1…
c90223e 
…25103)

Query features should verify that docs match the two-phase iterator.
@jimczi jimczi mentioned this pull request Mar 19, 2025
Merged
jimczi added a commit to jimczi/elasticsearch that referenced this pull request Mar 19, 2025
@jimczi
Leverage scorer supplier in QueryFeatureExtractor
c3a8271 
Follow up of elastic#125103 that leverages scorer supplier to create queries optimised to run on top docs only.
@jimczi jimczi mentioned this pull request Mar 19, 2025
Merged
elasticsearchmachine pushed a commit that referenced this pull request Mar 19, 2025
@jimczi
[8.x] Fix LTR query feature with phrases (and two-phase) queries (#12…
b0411d9 
…5257)

* Fix LTR query feature with phrases (and two-phase) queries (#125103)

Query features should verify that docs match the two-phase iterator.

* fix compilation after backport
elasticsearchmachine pushed a commit that referenced this pull request Mar 19, 2025
@jimczi
[8.18] Fix LTR query feature with phrases (and two-phase) queries (#1…
24c5031 
…25256)

* Fix LTR query feature with phrases (and two-phase) queries (#125103)

Query features should verify that docs match the two-phase iterator.

* fix compilation after backport
jimczi added a commit that referenced this pull request Mar 20, 2025
@jimczi
Leverage scorer supplier in QueryFeatureExtractor (#125259)
22be0d9 
Follow up of #125103 that leverages scorer supplier to create queries optimised to run on top docs only.
elasticsearchmachine pushed a commit that referenced this pull request Mar 20, 2025
@jimczi
Leverage scorer supplier in QueryFeatureExtractor (#125259) (#125295)
e4c0f5d 
Follow up of #125103 that leverages scorer supplier to create queries optimised to run on top docs only.
afoucret pushed a commit to afoucret/elasticsearch that referenced this pull request Mar 21, 2025
@jimczi @afoucret
Leverage scorer supplier in QueryFeatureExtractor (elastic#125259)
5c7cc4b 
Follow up of elastic#125103 that leverages scorer supplier to create queries optimised to run on top docs only.
smalyshev pushed a commit to smalyshev/elasticsearch that referenced this pull request Mar 21, 2025
@jimczi @smalyshev
Fix LTR query feature with phrases (and two-phase) queries (elastic#1…
0e7beec 
…25103)

Query features should verify that docs match the two-phase iterator.
smalyshev pushed a commit to smalyshev/elasticsearch that referenced this pull request Mar 21, 2025
@jimczi @smalyshev
Leverage scorer supplier in QueryFeatureExtractor (elastic#125259)
60a167a 
Follow up of elastic#125103 that leverages scorer supplier to create queries optimised to run on top docs only.
omricohenn pushed a commit to omricohenn/elasticsearch that referenced this pull request Mar 28, 2025
@jimczi @omricohenn
Fix LTR query feature with phrases (and two-phase) queries (elastic#1…
20cc52d 
…25103)

Query features should verify that docs match the two-phase iterator.
omricohenn pushed a commit to omricohenn/elasticsearch that referenced this pull request Mar 28, 2025
@jimczi @omricohenn
Leverage scorer supplier in QueryFeatureExtractor (elastic#125259)
31aa673 
Follow up of elastic#125103 that leverages scorer supplier to create queries optimised to run on top docs only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benwtrent benwtrent benwtrent approved these changes

Assignees
No one assigned
Labels
auto-backport Automatically create backport pull requests when merged >bug :Search Relevance/Ranking Scoring, rescoring, rank evaluation. Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v8.18.1 v8.19.0 v9.0.1 v9.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jimczi @elasticsearchmachine @benwtrent