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

Improvements to raft-ann-bench scripts, docs, and benchmarking implementations. #1769

Merged
merged 69 commits into from
Aug 30, 2023

Conversation

cjnolet
Copy link
Member

@cjnolet cjnolet commented Aug 24, 2023

This is just fixing merge conflicts for #1661 to continue making progress on new self-contained Python packaging.

Closes #1762

@cjnolet cjnolet added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 24, 2023
@cjnolet cjnolet self-assigned this Aug 24, 2023
@cjnolet cjnolet requested review from a team as code owners August 24, 2023 18:44
@cjnolet
Copy link
Member Author

cjnolet commented Aug 29, 2023

/ok to test

@cjnolet
Copy link
Member Author

cjnolet commented Aug 29, 2023

/ok to test

Copy link
Contributor

@achirkin achirkin left a 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.

cpp/bench/ann/src/common/conf.hpp Outdated Show resolved Hide resolved
cpp/bench/ann/src/common/benchmark.cpp Outdated Show resolved Hide resolved
cpp/bench/ann/src/common/benchmark.cpp Outdated Show resolved Hide resolved
@cjnolet
Copy link
Member Author

cjnolet commented Aug 29, 2023

/ok to test

Copy link
Member

@dantegd dantegd left a 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

@cjnolet
Copy link
Member Author

cjnolet commented Aug 29, 2023

/ok to test

cpp/bench/ann/src/common/benchmark.cpp Outdated Show resolved Hide resolved
cpp/bench/ann/src/common/benchmark.cpp Outdated Show resolved Hide resolved
cpp/bench/ann/src/common/conf.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@achirkin achirkin left a 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!

@achirkin
Copy link
Contributor

/ok to test

@cjnolet
Copy link
Member Author

cjnolet commented Aug 30, 2023

/merge

@github-actions github-actions bot removed the CMake label Aug 30, 2023
@cjnolet
Copy link
Member Author

cjnolet commented Aug 30, 2023

/ok to test

@rapids-bot rapids-bot bot merged commit e8b97ff into rapidsai:branch-23.10 Aug 30, 2023
54 checks passed
cjnolet pushed a commit to cjnolet/raft that referenced this pull request Sep 5, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Vector Search
Projects
Development

Successfully merging this pull request may close these issues.

[FEA] RAFT benchmarks should allow memory type to be specified at runtime
6 participants