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.

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

Update threshold value after new result is added #1715

Merged
merged 1 commit into from
May 23, 2024

Conversation

heemin32
Copy link
Collaborator

@heemin32 heemin32 commented May 22, 2024

Description

Update threshold value after new result is added

Because the threshold is not updated after inserting a new value to the result list, new value got inserted in the result regardless of its size.

Issues Resolved

#1714

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed as per the DCO using --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.

@junqiu-lei
Copy link
Member

Could we create a diff for the patch update? Example: junqiu-lei/faiss@b36a287

@navneet1v
Copy link
Collaborator

@heemin32 can you add more details on what was the root cause of the issue?

@heemin32
Copy link
Collaborator Author

Could we create a diff for the patch update? Example: junqiu-lei/faiss@b36a287

https://github.com/facebookresearch/faiss/compare/0d1385959ddecabb2825957e48ff28ff0e8abf53..da77c9972f6e7313f956111129541677b33429cb

junqiu-lei
junqiu-lei previously approved these changes May 22, 2024
CHANGELOG.md Outdated
@@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
* Support script score when doc value is disabled and fix misusing DISI [#1696](https://github.com/opensearch-project/k-NN/pull/1696)
### Bug Fixes
* Block commas in model description [#1692](https://github.com/opensearch-project/k-NN/pull/1692)
* Fix a bug in multi-vector with faiss engine [#1715](https://github.com/opensearch-project/k-NN/pull/1715)
Copy link
Member

Choose a reason for hiding this comment

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

Lets come up with a better description. Also, "Fix a bug" is implied if entry is in "Bug Fixes" section

Copy link
Member

@jmazanec15 jmazanec15 left a comment

Choose a reason for hiding this comment

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

@heemin32 Does this fix need to be moved to upstream as well or is it just concerned with patches?

@heemin32
Copy link
Collaborator Author

heemin32 commented May 23, 2024

@heemin32 Does this fix need to be moved to upstream as well or is it just concerned with patches?

Already updated the upstream code as well.

@heemin32 heemin32 changed the title Fix a bug in multi-vector with faiss engine Update threshold value after new result is added May 23, 2024
@heemin32 heemin32 requested review from jmazanec15 and junqiu-lei May 23, 2024 14:56
@heemin32 heemin32 merged commit 9023604 into opensearch-project:main May 23, 2024
52 of 54 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 23, 2024
Signed-off-by: Heemin Kim <[email protected]>
(cherry picked from commit 9023604)
heemin32 added a commit that referenced this pull request May 23, 2024
Signed-off-by: Heemin Kim <[email protected]>
(cherry picked from commit 9023604)

Co-authored-by: Heemin Kim <[email protected]>
navneet1v added a commit that referenced this pull request Jun 1, 2024
* Fix flaky test in Faiss JNI range search (#1705)

Signed-off-by: Junqiu Lei <[email protected]>

* Support script score when doc value is disabled and fix misusing DISI (#1696)

* Revert "Revert 'Support script score when doc value is disabled' (#1662)"

This reverts commit bd2f403.

Signed-off-by: panguixin <[email protected]>

* fix misusing doc value

Signed-off-by: panguixin <[email protected]>

* add changelog

Signed-off-by: panguixin <[email protected]>

---------

Signed-off-by: panguixin <[email protected]>

* --- (#1712)

updated-dependencies:
- dependency-name: requests
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update threshold value after new result is added (#1715)

Signed-off-by: Heemin Kim <[email protected]>

* Use the Lucene Distance Calculation Function in Script Scoring for doing exact search (#1699)

* Use the Lucene Distance Calculation Function in Script Scoring for doing exact search

Signed-off-by: Ryan Bogan <[email protected]>

* Add Changelog entry

Signed-off-by: Ryan Bogan <[email protected]>

* Fix failing test

Signed-off-by: Ryan Bogan <[email protected]>

* fix test

Signed-off-by: Ryan Bogan <[email protected]>

* Fix test bug and remove unnecessary validation

Signed-off-by: Ryan Bogan <[email protected]>

* Remove cosineSimilOptimized

Signed-off-by: Ryan Bogan <[email protected]>

* Revert "Remove cosineSimilOptimized"

This reverts commit f872d83.

Signed-off-by: Ryan Bogan <[email protected]>

---------

Signed-off-by: Ryan Bogan <[email protected]>

* Add validation for pq m parameter before training starts (#1713)

* Add validation for pq code count before training starts

Signed-off-by: Ryan Bogan <[email protected]>

* Add integration test

Signed-off-by: Ryan Bogan <[email protected]>

* Add unit tests

Signed-off-by: Ryan Bogan <[email protected]>

* Clean up code

Signed-off-by: Ryan Bogan <[email protected]>

* Remove unnecessary lines

Signed-off-by: Ryan Bogan <[email protected]>

* Add changelog entry

Signed-off-by: Ryan Bogan <[email protected]>

* Change framework to add validation with data

Signed-off-by: Ryan Bogan <[email protected]>

* Remove unused error message

Signed-off-by: Ryan Bogan <[email protected]>

* Add unit tests

Signed-off-by: Ryan Bogan <[email protected]>

* Change space type check name for readability

Signed-off-by: Ryan Bogan <[email protected]>

* Add javadocs

Signed-off-by: Ryan Bogan <[email protected]>

* Modify validation error wording and add json structure to tests

Signed-off-by: Ryan Bogan <[email protected]>

* Change TrainingDataSpec to VectorSpaceInfo

Signed-off-by: Ryan Bogan <[email protected]>

* Add unit tests

Signed-off-by: Ryan Bogan <[email protected]>

---------

Signed-off-by: Ryan Bogan <[email protected]>

* Updating the BWC test config after 2.14 release (#1724)

Signed-off-by: Navneet Verma <[email protected]>

---------

Signed-off-by: Junqiu Lei <[email protected]>
Signed-off-by: panguixin <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Heemin Kim <[email protected]>
Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: Navneet Verma <[email protected]>
Co-authored-by: Junqiu Lei <[email protected]>
Co-authored-by: panguixin <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Heemin Kim <[email protected]>
Co-authored-by: Ryan Bogan <[email protected]>
Co-authored-by: Navneet Verma <[email protected]>
luyuncheng pushed a commit to luyuncheng/k-NN-1 that referenced this pull request Jul 7, 2024
luyuncheng pushed a commit to luyuncheng/k-NN-1 that referenced this pull request Jul 7, 2024
@heemin32 heemin32 deleted the multi branch July 18, 2024 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants