-
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 raft-ann-bench scripts, docs, and benchmarking implementations. #1769
Improvements to raft-ann-bench scripts, docs, and benchmarking implementations. #1769
Conversation
…in the next commit
/ok to test |
/ok to test |
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.
A couple more merge artifacts/typos.
/ok to test |
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.
Just reviewed as well as tested the changes and things look great
/ok to test |
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.
cpp/bench changes LGTM!
/ok to test |
/merge |
/ok to test |
Builds on top of rapidsai#1769 - [x] Removes `libraft-ann-bench` C++ based package - [x] Creates `raft-ann-bench` packages that includes C++ tests as well as Python scripts - [x] `raft-ann-bench` package includes all tests for CPU and GPU - [x] `raft-ann-bench-cpu` package that does not depend on CUDA or RAFT GPU code - [x] Update docs - [x] Test artifacts and scripts in CI - [x] Minor code cleaning Some changes include: - Use `RAPIDS_DATASET_ROOT_DIR` env variable to indicate location of datasets (optional) consistent with other repos: https://docs.rapids.ai/maintainers/datasets/ - CPU and GPU packages are built in the existing GPU build GHA. Only the CUDA 12 jobs build the CPU packages. - Small change for invocation of scripts, for example: `python bench/ann/run.py --dataset deep-image-96-inner` is now `python -m raft-ann-bench.run --dataset deep-image-96-inner`, but still scripts meant to be invoked from the command line. Future improvements: - Remove use of popen python scripts from python scripts. - Improve printing and logging - Allow functions of package to be called from python scripts. Closes rapidsai#1744 Authors: - Dante Gama Dessavre (https://github.com/dantegd) - Artem M. Chirkin (https://github.com/achirkin) - Divye Gala (https://github.com/divyegala) - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Corey J. Nolet (https://github.com/cjnolet) - Divye Gala (https://github.com/divyegala) - AJ Schmidt (https://github.com/ajschmidt8) - Artem M. Chirkin (https://github.com/achirkin) URL: rapidsai#1773
This is just fixing merge conflicts for #1661 to continue making progress on new self-contained Python packaging.
Closes #1762