test(core): add a many LUT test in core crypto #742
Conversation
|
@slab-ci cpu_fast_test |
|
using the MSB to manage the LUT location looks more interesting to avoid delta/encoding mixing (as discovered while checking this with @mayeul-zama) |
IceTDrinker
force-pushed
the
am/test/many-lut
branch
from
8954f15 to
eb2af7f
Compare
|
@slab-ci cpu_fast_test |
| ciphertext_modulus: CiphertextModulus<Scalar>, | ||
| delta: Scalar, | ||
| funcs: &[&dyn Fn(Scalar) -> Scalar], | ||
| ) -> (GlweCiphertextOwned<Scalar>, usize) { |
There was a problem hiding this comment.
I think it would be cleaner to assert!(funcs.len() * input_max_degree * mega_case_size < poly_size)
And then cut the LUT into funcs.len() chunks of size input_max_degree * mega_case_size (the end of the LUT may be filled with 0)
And then, fill each mega case i of each chunk j with f_j(i)
This way, fn_count does not need to be a power of 2
There was a problem hiding this comment.
Or we may choose the biggest possible input_max_degree and then in the shortint API, we return an error if the degree is bigger than this inferred max degree
There was a problem hiding this comment.
We do not require the power of 2 in that case it's just that we use power of 2 to organize the data which IMO is much less of a headache
There was a problem hiding this comment.
But it seems to me that it limits the maximum possible degree of the inputs of the many LUT
There was a problem hiding this comment.
I think the first implementation will be based on the power of 2 approach, and if we see we are missing some optimizations we'll likely update to be able to be more flexible wrt to the degree.
The only time we would have a limit is if we have a small degree and lots of functions then there may be a world where some functions will not be evaluated because of wasted space if we use the power of 2, I think it's an acceptable tradeoff at first.
Plus here for core we don't really care, it's a first implementation to check the principles, the question will be of more interest later when it goes up to shortint
There was a problem hiding this comment.
It seems to me that the non power of 2 approach is not more complex
Plus here for core we don't really care, it's a first implementation to check the principles, the question will be of more interest later when it goes up to shortint
Why merge it then? Shouldn't we only merge the shortint version in main once ready?
There was a problem hiding this comment.
well at least from a design perspective it means the LUT generation does not need the input ciphertext degree in the power of 2 approach as it's a pessimistic case, in the degree approach the lut needs to be tailor made for the ciphertext
There was a problem hiding this comment.
As described in my second comment, I believe we can take the biggest possible input_max_degree (which depend on fn_count and full_msg_mod) to build the LUT and we can use it for all compatible ciphertexts
| // N/(p/2) = size of each block, to correct noise from the input we introduce the | ||
| // notion of box, which manages redundancy to yield a denoised value | ||
| // for several noisy values around a true input value. | ||
| let box_size = polynomial_size.0 / message_modulus; |
There was a problem hiding this comment.
Does message_modulus include the carries?
I think it should, but the name, as used in shortint does not reflect that.
Does message_modulus have a differen meaning in core_crypto?
There was a problem hiding this comment.
yeah in core crypto it's most likely the full msg space here, it would be adapted for the port to shortint
IceTDrinker
force-pushed
the
am/test/many-lut
branch
from
eb2af7f to
0ae469b
Compare
|
@slab-ci cpu_fast_test |
IceTDrinker
force-pushed
the
am/test/many-lut
branch
from
0ae469b to
8f9f38c
Compare
|
@slab-ci gpu_test |
- it does not require any new primitive so was made into a test at the core crypto level - shortint will have a more user friendly API, using the MSBs for selecting the function means it should not require too much design as deltas are always the same
IceTDrinker
force-pushed
the
am/test/many-lut
branch
from
8f9f38c to
da79ca7
Compare
|
@slab-ci gpu_test |
|
Pull Request has been approved 🎉 |
PR content/description
Check-list:
* [ ] Docs have been added / updated (for bug fixes / features)* [ ] Relevant issues are marked as resolved/closed, related issues are linked in the description* [ ] Check for breaking changes (including serialization changes) and add them to commit message following the conventional commit specification