-
Notifications
You must be signed in to change notification settings - Fork 131
[REVIEW] Rename small_hash_bitlen
of Single-CTA
#1338
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
base: branch-25.12
Are you sure you want to change the base?
Conversation
/ok to test 26d38a4 |
26d38a4
to
8c307e7
Compare
Hi @cjnolet, |
a137c74
to
df8d3d2
Compare
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? |
df8d3d2
to
266a6c3
Compare
Sure! I have updated the code and it is ready for review. |
Hi @cjnolet, could you please review this PR? |
/ok to test 266a6c3 |
/ok to test 31bea9d |
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
andmulti_cta_search::search
inherit fromsearch_plan_impl
, which in turn includes the member variablesmall_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 usinghash_bitlen
instead ofsmall_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:
small_hash_bitlen
touse_small_hash
in the single-cta algorithm, and mapssmall_hash_bitlen
to either 1 or 0.small_hash_bitlen
, is now determined by thehash_bitlen
parameter.small_hash_bitlen
, is now handled by theuse_small_hash
parameter.These changes do not affect program logic.