Skip to content

Add recall for test script - #190

Merged
sam-herman merged 4 commits into
opensearch-project:mainfrom
sam-herman:add-recall-for-test-script
Oct 28, 2025
Merged

Add recall for test script#190
sam-herman merged 4 commits into
opensearch-project:mainfrom
sam-herman:add-recall-for-test-script

Conversation

@sam-herman

Copy link
Copy Markdown
Collaborator

Description

This PR achieves a number of main changes in the scripts/create_and_test_large_index.py which is currently used as an ad-hoc dynamic performance benchmarking tool.

Main changes:

  1. Added dynamic recall measurement - the ground truth are calculated on the fly using a min heap, and doesn't require any additional storage of large datasets in memory during ingestion.
  2. Refactoring the monolithic scripts/create_and_test_large_index.py - into several modules (see documentation for the new package).
  3. Unit and integration testing for the recall measurement - since the recall measurement adds more complexity, new unit and integration tests were added to validate it's correctness and reliability.

Testing

Per the instructions in the readme file:

(.venv) sam.herman@Sam-Herman-FP9X7HJYJX scripts % python test_recall_integration.py \
  --host localhost:9200 \
  --dimension 256 \
  --num-vectors 5000 \
  --num-queries 10 \
  --k 20

...
...

=== Recall Summary ===
Average Recall@20: 0.8350
Min Recall@20: 0.7000
Max Recall@20: 1.0000
Std Dev: 0.0776

[Cleanup] Deleting test index...

======================================================================
✅ INTEGRATION TEST COMPLETED SUCCESSFULLY
======================================================================

Check List

  • New functionality includes testing.
  • [x ] New functionality has been documented.
  • API changes companion pull request created.
  • [x ] Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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.

Signed-off-by: Samuel Herman <sherman8915@gmail.com>
Signed-off-by: Samuel Herman <sherman8915@gmail.com>
Signed-off-by: Samuel Herman <sherman8915@gmail.com>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

pardon my python expertise, but what is the purpose of this file? I only see some comments (which can be in the readme), and version number.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not much of a python expert either, I think if you want the subdirectory to be picked up as a module you have to create the init file. But I can try removing it and see what happens.

Comment thread scripts/README.md Outdated

```
scripts/
├── create_and_test_large_index.py # Main testing script

@akash-shankaran akash-shankaran Oct 24, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe move all these files under a directory? /scripts/JvectorIndexAndSearch

I'll be adding more scripts here shortly, including Jake's Parquet loader so it'd make sense to have separate directories under scripts.

import matplotlib.pyplot as plt
import os
"""
Create and test large JVector indices in OpenSearch.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

consider renaming this file to reflect the scope.

It's doing more that testing large index at this point :)

dist = 1 - cosine_sim
else:
dist = 1.0
else:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

inner product is not supported?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Didn't add that yet because it requires normalization, which we're not doing.

@akash-shankaran akash-shankaran left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice change.

Maybe we should consider running this (at a small scale) as part of PR validations, until our other benchmarking solutions become robust.

@sam-herman

Copy link
Copy Markdown
Collaborator Author

Nice change.

Maybe we should consider running this (at a small scale) as part of PR validations, until our other benchmarking solutions become robust.

Good idea!

Signed-off-by: Samuel Herman <sherman8915@gmail.com>
@sam-herman
sam-herman force-pushed the add-recall-for-test-script branch from d540a1c to 65e7036 Compare October 28, 2025 21:03
@sam-herman
sam-herman merged commit e6fd81d into opensearch-project:main Oct 28, 2025
4 checks passed
@sam-herman
sam-herman deleted the add-recall-for-test-script branch October 28, 2025 22:40
opensearch-trigger-bot Bot pushed a commit that referenced this pull request Oct 28, 2025
* add dynamic recall measurement
* refactor for better maintainability, add tests

---------

Signed-off-by: Samuel Herman <sherman8915@gmail.com>
(cherry picked from commit e6fd81d)
sam-herman added a commit that referenced this pull request Oct 28, 2025
* add dynamic recall measurement
* refactor for better maintainability, add tests

---------


(cherry picked from commit e6fd81d)

Signed-off-by: Samuel Herman <sherman8915@gmail.com>
Co-authored-by: sam-herman <97131656+sam-herman@users.noreply.github.com>
akash-shankaran pushed a commit to akash-shankaran/opensearch-jvector that referenced this pull request Oct 30, 2025
* add dynamic recall measurement
* refactor for better maintainability, add tests

---------

Signed-off-by: Samuel Herman <sherman8915@gmail.com>
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.

3 participants