-
Notifications
You must be signed in to change notification settings - Fork 197
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
Improvements to ANN Benchmark Python scripts and docs #1734
Improvements to ANN Benchmark Python scripts and docs #1734
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple minor things but these are huge improvements. Thanks!
if args.configuration: | ||
conf_filepath = args.configuration | ||
else: | ||
conf_filepath = os.path.join(scripts_path, "conf", f"{args.dataset}.json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we enforce the constraint here that either the dataset or the configuration argument needs to be specified (one of them is required but both can't be specified).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since dataset has a default, that enforces that one of them is specified. Should I remove the default and throw an error instead if both are unspecified?
@@ -34,6 +34,7 @@ dependencies: | |||
- nccl>=2.9.9 | |||
- ninja | |||
- nlohmann_json>=3.11.2 | |||
- pyyaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Matplotlib doesn't seem to be getting installed as a dependency of the libraft-ann-bench
conda package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's part of the bench-ann environment file. It should not be a dependency of the libraft-ann-bench conda package because that package is just the executables
Co-authored-by: Corey J. Nolet <[email protected]>
/merge |
No description provided.