Skip to content

Stabilize vector tests of cosine metric #21867

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jul 28, 2025
Merged

Stabilize vector tests of cosine metric #21867

merged 5 commits into from
Jul 28, 2025

Conversation

jbajic
Copy link
Contributor

@jbajic jbajic commented Jul 17, 2025 •
edited
Loading

Scope & Purpose

To stabilize cosine tests, we need to control the distances of the vectors we generate, or we might experience unstable tests since the similarity score of the top k vectors might be too close, which can mix the documents order.

  • 💩 Bugfix
  • 🍕 New feature
  • 🔥 Performance improvement
  • 🔨 Refactoring/simplification

Checklist

  • Tests
    • Regression tests
    • C++ Unit tests
    • integration tests
    • resilience tests
  • 📖 CHANGELOG entry made
  • 📚 documentation written (release notes, API changes, ...)
  • Backports
    • Backport for 3.12.0: (Please link PR)
    • Backport for 3.11: (Please link PR)
    • Backport for 3.10: (Please link PR)

Related Information

(Please reference tickets / specification / other PRs etc)

  • Docs PR:
  • Enterprise PR:
  • GitHub issue / Jira ticket:
  • Design document:

@cla-bot cla-bot bot added the cla-signed label Jul 17, 2025
@jbajic jbajic changed the title Clean vectors in setup Control vectors in cosine test setup Jul 17, 2025
@jbajic jbajic changed the title Control vectors in cosine test setup Stabilize vector tests of cosine metric Jul 17, 2025
@jbajic jbajic self-assigned this Jul 17, 2025
@jbajic jbajic marked this pull request as ready for review July 24, 2025 10:00
dothebart
dothebart approved these changes Jul 28, 2025
View reviewed changes
Copy link
Contributor

@dothebart dothebart left a comment

Choose a reason for hiding this comment

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

LGTM.
Should that effect be in the documentation as well?

@jbajic
Copy link
Contributor Author

jbajic commented Jul 28, 2025

LGTM. Should that effect be in the documentation as well?

If you refer to the changelog, then I don't think so, since this does not change any logic of changes to the product. And I documented the code enough I think, so the reason for doing this is clear

@KVS85 KVS85 added this to the devel milestone Jul 28, 2025
@KVS85 KVS85 merged commit aa4775b into devel Jul 28, 2025
7 checks passed
@KVS85 KVS85 deleted the bug-fix/stabilize-cosine-test branch July 28, 2025 10:33
@dothebart
Copy link
Contributor

I'm talking about user documentation. If simple tests may fail this, Users with too simple expectations may fail over this as well and search for a "bug" - whereas a simple sitenote in the documentation would be enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dothebart dothebart dothebart approved these changes

Assignees

@jbajic jbajic

Labels
3 - Testing cla-signed
Projects
None yet
Milestone
devel
Development

Successfully merging this pull request may close these issues.
3 participants
@jbajic @dothebart @KVS85