-
Notifications
You must be signed in to change notification settings - Fork 169
Support native Maximal Marginal Relevance #2868
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
Conversation
SpaceType methodSpaceType = getSpaceTypeFromMethodContext(knnMethodContext); | ||
SpaceType topLevelSpaceType = getSpaceTypeFromString(topLevelSpaceTypeString); | ||
|
||
// If we failed to find space type from both method context and top level |
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.
We cleaned up this logic in this PR but didn't clean up the comments which can cause confusion. So cleaned it up here since this PR also needs to resolve the space type.
bed7496
to
1bd1580
Compare
src/main/java/org/opensearch/knn/index/util/IndexMappingUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/index/util/IndexMappingUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/search/processor/mmr/MMRConstants.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/search/processor/mmr/MMROverSampleProcessor.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/search/processor/mmr/MMRKnnQueryTransformer.java
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/search/processor/mmr/MMRUtil.java
Outdated
Show resolved
Hide resolved
fbe5747
to
4ce8074
Compare
@navneet1v @Vikasht34 Could you help review this PR? |
I will start ...Please Make Sure all Integs tests are passing |
4b21b63
to
28d2f25
Compare
@Vikasht34 I have added some benchmark data. |
31f2342
to
bb342bd
Compare
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.
Looks good me , though one test is failing !! Please make sure to fix that.
src/main/java/org/opensearch/knn/search/processor/mmr/MMROverSampleProcessor.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/search/processor/mmr/MMRRerankProcessor.java
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/search/extension/MMRSearchExtBuilder.java
Show resolved
Hide resolved
Signed-off-by: Bo Zhang <[email protected]>
Signed-off-by: Bo Zhang <[email protected]>
Description
Support native Maximal Marginal Relevance. It relies on the system generated search pipeline to auto generate:
We also introduce the
MMRQueryTransformer
to support different transform logic for different query builder. Here we just implement the transformer for the knn query for now. And we will implement the transformer in neural plugin to handle the neural query properly.Checks will fail until this PR is merged in core.
example query with mmr:
Check benchmark
Related Issues
Resolves #2804
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.