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.

Sign up for GitHub

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

Jump to bottom

Add CAGRA gbench #1496

Merged
merged 6 commits into from
Jul 18, 2023
Merged

Add CAGRA gbench #1496

merged 6 commits into from
Jul 18, 2023

Conversation

tfeher
Copy link
Contributor

@tfeher tfeher commented May 8, 2023 •
edited
Loading

This PR adds synthetic benchmarks for CAGRA.

The kNN graph is generated randomly, otherwise most of the time would be spent in building the index.

@tfeher tfeher added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 8, 2023
@tfeher tfeher self-assigned this May 8, 2023
@tfeher tfeher requested review from a team as code owners May 8, 2023 19:21
@tfeher
Copy link
Contributor Author

tfeher commented May 8, 2023

Currently we benchmark only with:

@enp1s0 please let me know if you have a suggestion for extending the benchmark parameter range.

cjnolet
cjnolet reviewed May 15, 2023
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

Looks great. Just a couple minor questions

cpp/bench/prims/neighbors/knn.cuh Outdated Show resolved Hide resolved
cpp/bench/prims/neighbors/knn.cuh Outdated Show resolved Hide resolved
cjnolet
cjnolet reviewed May 15, 2023
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

Looks great. Just a couple minor questions

cjnolet
cjnolet reviewed May 15, 2023
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

Looks great. Just a couple minor questions

@cjnolet cjnolet changed the base branch from branch-23.06 to branch-23.08 June 6, 2023 21:01
@cjnolet cjnolet mentioned this pull request Jun 22, 2023
Closed
9 tasks
@tfeher tfeher force-pushed the cagra_gbench branch 2 times, most recently from aa3a239 to 706c3db Compare July 5, 2023 07:52
@tfeher
Copy link
Contributor Author

tfeher commented Jul 5, 2023

@cjnolet I have moved a benchmark to a separate file, to allow us to focus on CAGRA relevant param combinations.

From my side, the PR is ready to go. Tagging @enp1s0 for any comments or suggestion.

@tfeher tfeher mentioned this pull request Jul 5, 2023
Closed
cjnolet
cjnolet requested changes Jul 6, 2023
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

Looks good. Just one very minor detail about the bench namespace.

cpp/bench/prims/neighbors/cagra_bench.cuh Outdated Show resolved Hide resolved
@tfeher
Copy link
Contributor Author

tfeher commented Jul 7, 2023

Thanks @cjnolet for the review, I have updated the PR!

@cjnolet
Copy link
Member

cjnolet commented Jul 18, 2023

/merge

@rapids-bot rapids-bot bot merged commit 1824e42 into rapidsai:branch-23.08 Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjnolet cjnolet cjnolet approved these changes

Assignees

@tfeher tfeher

Labels
CMake cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Vector Search
Projects
VS/ML/DM Primitives Release Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tfeher @cjnolet