feat(integer): add count_ones/zeros #1503
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tmontaigu
force-pushed
the
tm/count_ones
branch
2 times, most recently
from
df1bfcc to
1e2d237
Compare
IceTDrinker
reviewed
Sep 6, 2024
tfhe/src/integer/server_key/radix_parallel/tests_signed/test_count_zeros_ones.rs
Outdated
Show resolved
Hide resolved
Comment on lines
+315
to
+320
| // In 2_2, each block may have between 0 and 2 bits set. | ||
| // 2_2 also allows 5 additions maximum (noise wise) | ||
| // 2 * 5 = 10 which is less than the max value storable (15 = (2**4) -1) | ||
| // | ||
| // Since in 2_2 bivariate PBS is possible, we can actually group blocks by two. |
There was a problem hiding this comment.
probably not applicable but kind of makes me think of it for the eq and ne implems ? as we will get a slower version to be safe at the moment
| /// * ct must not have any carries | ||
| /// * The returned result has enough blocks to encrypt 32bits (e.g. 1_1 parameters -> 32 blocks, | ||
| /// 3_3 parameters -> 11 blocks == 33 bits) | ||
| fn count_bits_2_2<T>(&self, ct: &T, count_kind: BitCountKind) -> RadixCiphertext |
There was a problem hiding this comment.
perf gains of this variant ?
There was a problem hiding this comment.
As said in the commit its none on small sizes (< 64bits) and rather small on 64 bits (iirc naive 64 bits its ~135 ms, while non naive is ~115ms), where its really interesting is for really big sizes like 12800 bits where we go from 1.8s for naive, 1.1 sec for non-naive likely because the number of pbs reduced starts to be noticeable and there are less things to sum
tmontaigu
force-pushed
the
tm/count_ones
branch
from
1e2d237 to
649673b
Compare
The non naive version made for 2_2 parameters only bring slight (10-15%) for some small sizes like (64, 128, 256 bits) but reduces number of PBS. The place where it brings the best improvements it for very large numbers (e.g 6400 blocks 1.8s for naive, 1.1 sec for non-naive)
tmontaigu
force-pushed
the
tm/count_ones
branch
from
649673b to
62858b0
Compare
|
Changes since last time is just a rebase + small comment adressed + use of the |
IceTDrinker
approved these changes
Sep 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The non naive version made for 2_2 parameters
only bring slight (10-15%) for some small sizes like (64, 128, 256 bits) but reduces number of PBS. The place where it brings the best improvements it for very large numbers (e.g 6400 blocks 1.8s for naive, 1.1 sec for non-naive)
fixes https://github.com/zama-ai/tfhe-rs-internal/issues/638