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

CAGRA: reduce argument count in select_and_run() kernel wrappers #227

Merged
merged 6 commits into from
Jul 23, 2024

Conversation

achirkin
Copy link
Contributor

A small change that reduces the number of arguments in one of the wrapper layers in the detail namespace of CAGRA. The goal is twofold:

  1. Simplify the overly long signature of selet_and_run (which has many instances)
  2. Give access to all search parameters for future upgrades of the search kernel

This is to simplify the integration (and review) of the persistent kernel (#215).
No performance or functional changes expected.

@achirkin achirkin requested a review from a team as a code owner July 17, 2024 18:39
@github-actions github-actions bot added the cpp label Jul 17, 2024
@achirkin achirkin mentioned this pull request Jul 18, 2024
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks Artem for the PR! The changes so far look good, but don't we need to change search_*_cta_kernel-ext.cuh files too?

@achirkin
Copy link
Contributor Author

Thanks for noticing this, Tamas! I've updated the *-ext.cuh files and filed a bug for these files not being used #242

Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks Artem for updating the PR and filing the issue. LGTM.

@tfeher tfeher added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Jul 23, 2024
@tfeher
Copy link
Contributor

tfeher commented Jul 23, 2024

/merge

@rapids-bot rapids-bot bot merged commit 9e6d311 into rapidsai:branch-24.08 Jul 23, 2024
58 checks passed
divyegala pushed a commit to divyegala/cuvs that referenced this pull request Jul 31, 2024
…idsai#227)

A small change that reduces the number of arguments in one of the wrapper layers in the detail namespace of CAGRA. The goal is twofold:
  1) Simplify the overly long signature of `selet_and_run` (which has many instances) 
  2) Give access to all search parameters for future upgrades of the search kernel

This is to simplify the integration (and review) of the persistent kernel (rapidsai#215).
No performance or functional changes expected.

Authors:
  - Artem M. Chirkin (https://github.com/achirkin)

Approvers:
  - Tamas Bela Feher (https://github.com/tfeher)

URL: rapidsai#227
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants