-
Notifications
You must be signed in to change notification settings - Fork 25.7k
New rescorer based on script #74274
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.
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
New rescorer based on script #74274
Conversation
|
💚 CLA has been signed |
|
/test |
|
Pinging @elastic/es-search (Team:Search) |
|
Curious if there is any plan to merge this in the near future? It would be incredibly helpful for my work! |
|
@javanna Any thoughts on possibly reviving this pr? |
|
I am +1 on reviving this, we just did not get to it until now, sadly. |
|
I have two questions about this PR:
{
"size": 50,
"query": {"match": {"title": {"query": "raymond chandler"}}},
"rescore": [
{
"window_size": 500,
"query": {
"rescore_query": {
"sltr": {
"params": {
"query_string": "raymond chandler"
},
"model": "lambdamart_v001",
"cache": true
}
},
"rescore_query_weight": 1,
"query_weight": 0
}
},
{
"window_size": 50,
"script": {
"lang": "painless",
"source": "source": "1.0 / (1.0 + Math.exp(0-_score))"
}
}
]
}My hope is that the resulting scores at the end of this will be whatever SLTR returns, pushed through a sigmoid. (This can't be done in a single step, for the reason discussed here: o19s/elasticsearch-learning-to-rank#121 (comment) )
|
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
|
@elasticmachine generate changelog |
|
|
||
| @Override | ||
| public TransportVersion getMinimalSupportedVersion() { | ||
| return TransportVersions.SCRIPT_RESCORER; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question for my knowledge. I'm only somewhat familiar with Transport Versions. What does this protect? Does this mean we can only support this syntax in the next release forward? And why is that in this case as in is it possible to have some new Builder where the minimal supported version is any version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. This ensures that node that de-serializes this request has the provided min version. If it doesn't, the request with a script rescorer fails (I think with IllegalArgumentException).
We could potentially add some logic, if the deserialization happens on an older node, convert it to query rescorer, but I think it makes the code more complicated for little added advantage: supporting mixed cluster scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense thanks!
john-wagster
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
@elasticmachine test this please |
|
@elasticmachine test this please |
|
@elasticmachine test this please |
|
@elasticmachine test this please |
|
@elasticmachine test this please |
PR elastic#74274 introduced a new rescorer based on script. This adds a documenation for this rescorer.
PR elastic#74274 introduced a new rescorer based on script. This adds a documentation for this rescorer.
Introduce a new rescorer based on script.
A script can use scores from the previous query/rescorer.
For example:
Closes #52338