opensearch-project / neural-search

Plugin that adds dense neural retrieval into the OpenSearch ecosytem
Apache License 2.0
57 stars 58 forks source link

Support k-NN radial search parameters in neural search #697

Closed junqiu-lei closed 2 months ago

junqiu-lei commented 2 months ago

Description

Since radial search is supported in k-NN plugin recently, this PR allows neural search to have the ability to use radial search parameters, so finally it can be allowed to use one of k, min_score, max_distance for vector search.

Issues Resolved

Part of https://github.com/opensearch-project/k-NN/issues/814

Benchmark in k-NN plugin

https://github.com/opensearch-project/k-NN/issues/814#issuecomment-2060195145

Check List

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.

navneet1v commented 2 months ago

@junqiu-lei please attach the link of the benchmarks you have done on k-NN for radial search in this PR description.

junqiu-lei commented 2 months ago

@junqiu-lei please attach the link of the benchmarks you have done on k-NN for radial search in this PR description.

Yes, added in PR description.

vibrantvarun commented 2 months ago

@junqiu-lei where will you be adding bwc tests? In K-nn or neural search?

vibrantvarun commented 2 months ago

I think you can add one bwc test in the PR to ensure this does not break earlier versions.

navneet1v commented 2 months ago

I think you can add one bwc test in the PR to ensure this does not break earlier versions.

@junqiu-lei can we add a BWC test here.

junqiu-lei commented 2 months ago

bwc test added @navneet1v @vibrantvarun

vibrantvarun commented 2 months ago

Looks good to me , Will wait @navneet1v comments to get resolved before approving.

vibrantvarun commented 2 months ago

Hey @junqiu-lei BWC tests are failing because of your changes. I think you have to add a check to skip running versions bwc for this tests for particular versions. https://github.com/opensearch-project/neural-search/blob/f5a47a6cc829aed86f7db7296d7365675b31f04d/qa/restart-upgrade/build.gradle#L136

opensearch-trigger-bot[bot] commented 2 months ago

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-697-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 68cdbc90c4fabd87ec8fecca4bdd753a55994107
# Push it to GitHub
git push --set-upstream origin backport/backport-697-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-697-to-2.x.