-
Notifications
You must be signed in to change notification settings - Fork 138
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
Support one_to_one in ML Inference Search Response Processor #2801
Support one_to_one in ML Inference Search Response Processor #2801
Conversation
We need to bump the version in the build.gradle file to let the CI runs.
|
} catch (Exception e) { | ||
if (ignoreFailure) { | ||
responseListener.onResponse(response); | ||
} else { | ||
responseListener.onFailure(e); | ||
responseListener.onFailure(new RuntimeException(e.getMessage())); |
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.
Why encapsulate the original exception to RuntimeException?
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.
to avoid 500x error
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.
Why are we getting 5x error in the first place?
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.
Suggest use OpenSearchStatusException to replace RuntimeException.
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.
I don't see 5xx exception, it's for precautions. changed to OpenSearchStatusException similar to
throw new OpenSearchStatusException("Failed to search index " + indexName, RestStatus.BAD_REQUEST); |
added in commit use OpenSearchStatusException in error handling
plugin/src/main/java/org/opensearch/ml/processor/MLInferenceSearchResponseProcessor.java
Show resolved
Hide resolved
Thanks for the explanation. IMO,
So we already released this one_to_one inference in 2.16? Where do we save this setting? |
plugin/src/main/java/org/opensearch/ml/processor/MLInferenceSearchResponseProcessor.java
Outdated
Show resolved
Hide resolved
plugin/src/test/java/org/opensearch/ml/processor/MLInferenceSearchResponseProcessorTests.java
Outdated
Show resolved
Hide resolved
aw ha, it should be 2.17, yeah this change is going to add to 2.17. just modified the description. |
in the ml search response processor, we already had it in 2.16 but it's not supported to turn on to true yet. |
can we change the settings name? or we need to stick with |
the name is added since 2.16, if we change the name in 2.17, it might case confusion. |
0d5c6bf
to
4926805
Compare
Signed-off-by: Mingshi Liu <[email protected]>
5b5d79f
to
f1aa03f
Compare
plugin/src/main/java/org/opensearch/ml/processor/MLInferenceSearchResponseProcessor.java
Show resolved
Hide resolved
plugin/src/main/java/org/opensearch/ml/processor/MLInferenceSearchResponseProcessor.java
Show resolved
Hide resolved
plugin/src/main/java/org/opensearch/ml/processor/MLInferenceSearchResponseProcessor.java
Outdated
Show resolved
Hide resolved
All tests passed |
Signed-off-by: Mingshi Liu <[email protected]>
* add one document to one prediction support Signed-off-by: Mingshi Liu <[email protected]> * rephrase javadoc Signed-off-by: Mingshi Liu <[email protected]> * use OpenSearchStatusException in error handling Signed-off-by: Mingshi Liu <[email protected]> * fix message Signed-off-by: Mingshi Liu <[email protected]> * add more tests Signed-off-by: Mingshi Liu <[email protected]> * handle different exceptions properly Signed-off-by: Mingshi Liu <[email protected]> --------- Signed-off-by: Mingshi Liu <[email protected]> (cherry picked from commit 2a33c65)
…2843) * add one document to one prediction support Signed-off-by: Mingshi Liu <[email protected]> * rephrase javadoc Signed-off-by: Mingshi Liu <[email protected]> * use OpenSearchStatusException in error handling Signed-off-by: Mingshi Liu <[email protected]> * fix message Signed-off-by: Mingshi Liu <[email protected]> * add more tests Signed-off-by: Mingshi Liu <[email protected]> * handle different exceptions properly Signed-off-by: Mingshi Liu <[email protected]> --------- Signed-off-by: Mingshi Liu <[email protected]> (cherry picked from commit 2a33c65) Co-authored-by: Mingshi Liu <[email protected]>
Description
Many document to one prediction is the default config in ML Inference Search Response Processor. This PR adds one document to one prediction support following up with #2688
in 2.16, ml inference search response processor support collect documents into a list of model input and make one prediction call.
for example,
the search response returns as following, and we would like to use the
text
field in the document to send to model input.one_to_one inference is false setting will combine the two document containing the field
text
into["this is document 1", "this is document 2"]
and this is one round of prediction.in 2.17, we support one_to_one inference is true setting, that will make two rounds of prediction, the first prediction, we send
"this is document 1"
and the second prediction we send"this is document 2"
.when users want to use a model that accept a list of model input, for example, using openai embedding model, we can set one_to_one is false,
when users want to use a model that accept a single string model input, for example, using bedrock embedding model, we can set one_to_one is true. The most common use case for one-to-one inference is rerank use case using xgboost, which usually take a single document and compare with the search string and return a single score.
Related Issues
#2173
#2444
#2839
#2879
Check List
--signoff
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.