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

fix(test): use fixed seed to improve reproducibility. #2557

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

LindaSummer
Copy link
Contributor

Issue

Fix: #2550

Unit test's random generation may make result unstable to expected result.

Proposed Changes

Use fixed seed for hnsw index unit test for improving reproducibility.

Comment on lines 95 to 98
HnswIndex(const SearchKey& search_key, HnswVectorFieldMetadata* vector, engine::Storage* storage,
std::random_device::result_type seed);
HnswIndex(const SearchKey& search_key, HnswVectorFieldMetadata* vector, engine::Storage* storage)
: HnswIndex(search_key, vector, storage, std::random_device()()) {}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can use a single constructor with a default argument?

HnswIndex(const SearchKey& search_key, 
          HnswVectorFieldMetadata* vector, 
          engine::Storage* storage, 
          std::random_device::result_type seed = std::random_device()())
{
    ...
}

Copy link
Contributor Author

@LindaSummer LindaSummer Sep 27, 2024

Choose a reason for hiding this comment

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

Hi @Beihao-Zhou ,

Thanks for your review suggestions!
I have updated related constructors. 😊

Best Regards,
Edward

@Beihao-Zhou
Copy link
Member

The rest LGTM :)

Copy link

sonarcloud bot commented Sep 27, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarCloud

@Beihao-Zhou Beihao-Zhou merged commit b8cf118 into apache:unstable Sep 27, 2024
31 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky test in HNSW index unit test
4 participants