Skip to content

Conversation

QDXG-CXK
Copy link

In the implementation of the single-cta algorithm in CAGRA, the naming of the variable small_hash_bitlen is inappropriate.
Both single_cta_search::search and multi_cta_search::search inherit from search_plan_impl , which in turn includes the member variable small_hash_bitlen . However, while in the multi-cta algorithm this parameter represents the bit length of the local hash, in the single-cta algorithm it is mainly used to determine whether the small hash method is being used. Even when the small hash method is used in single-cta, the hash length is still passed using hash_bitlen instead of small_hash_bitlen , which leads to potential confusion. Since it is used as a boolean flag in this context, use_small_hash is a more appropriate name.

This PR introduces the following changes:

  1. Renames small_hash_bitlen to use_small_hash in the single-cta algorithm, and maps small_hash_bitlen to either 1 or 0.
  2. The shared memory layout, which was previously determined by small_hash_bitlen , is now determined by the hash_bitlen parameter.
  3. The check for whether the small hash method is used, which was previously done via small_hash_bitlen , is now handled by the use_small_hash parameter.

These changes do not affect program logic.

Copy link

copy-pr-bot bot commented Sep 18, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cjnolet cjnolet added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Sep 18, 2025
@cjnolet
Copy link
Member

cjnolet commented Sep 18, 2025

/ok to test 26d38a4

@QDXG-CXK
Copy link
Author

Hi @cjnolet,
I have formated with clang-format, but am I responsible to change the copyright for below reports of pre-commit run?

I got:
image

@cjnolet
Copy link
Member

cjnolet commented Sep 19, 2025

Hi @QDXG-CXK,

Pre-commit will update the file for you, but you still need to explicitly add it to your git commit and push. Does this answer your question?

@QDXG-CXK
Copy link
Author

Hi @QDXG-CXK,

Pre-commit will update the file for you, but you still need to explicitly add it to your git commit and push. Does this answer your question?

Sure! I have updated the code and it is ready for review.
Thank you very much.

@QDXG-CXK
Copy link
Author

Hi @cjnolet, could you please review this PR?

@cjnolet
Copy link
Member

cjnolet commented Sep 23, 2025

/ok to test 266a6c3

@cjnolet
Copy link
Member

cjnolet commented Oct 1, 2025

/ok to test 31bea9d

@cjnolet cjnolet changed the base branch from branch-25.10 to branch-25.12 October 2, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

2 participants