Conversation
0cf14af to
0fbb809
Compare
- for now no dedicated types have been created for the the extended bootstrap, meaning an extended BSK is merely seen as a BSK/ExtensionFactor couple
0fbb809 to
a5753ce
Compare
mayeul-zama
left a comment
There was a problem hiding this comment.
@mayeul-zama reviewed 11 files and all commit messages, and made 9 comments.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on IceTDrinker, soonum, and SouchonTheo).
tfhe/src/core_crypto/experimental/algorithms/lwe_extended_programmable_bootstrapping.rs line 77 at r1 (raw file):
SplitLutCont: ContainerMut<Element = Scalar>, { for (idx, &coeff) in lut.as_ref().iter().enumerate() {
Could add asserts on sizes
tfhe/src/core_crypto/experimental/algorithms/lwe_extended_programmable_bootstrapping.rs line 126 at r1 (raw file):
{ // extension factor == 1 means classic bootstrap which is already optimized if extension_factor.0 == 1 {
Maybe we could simply panic here
tfhe/src/core_crypto/experimental/algorithms/lwe_extended_programmable_bootstrapping.rs line 88 at r1 (raw file):
/// N' = 2^nu * N /// new_lut_idx = (ai + old_lut_idx) % 2^nu /// (2^nu + (ai % 2N') - 1 - new_lut_idx)/2^nu seems to work for the multiplication by X^ai
Hard to understand comment
Could detail a bit more
tfhe/src/core_crypto/experimental/algorithms/lwe_extended_programmable_bootstrapping.rs line 154 at r1 (raw file):
); // TODO ? use only a split accumulator and an assign primitive ?
Do we want to keep this TODO?
tfhe/src/core_crypto/experimental/algorithms/lwe_extended_programmable_bootstrapping.rs line 265 at r1 (raw file):
std::thread::scope(|s| { let thread_processing = |id: usize, stack: &mut PodStack| {
Would be easier to read if split into smaller functions or (less ideally) closures
tfhe/src/core_crypto/experimental/algorithms/lwe_extended_programmable_bootstrapping.rs line 295 at r1 (raw file):
extension_factor, ct_dst_idx, );
A comment to explain how ct_dst_idx and small_monomial_degree are used to "simulate" rotation on the big LUT by doing small rotations and swaps would really help understand here
tfhe/src/core_crypto/experimental/commons/parameters.rs line 48 at r1 (raw file):
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] pub struct LweBootstrapExtensionFactor(pub usize);
Could add a comment to explain what it is
tfhe/src/core_crypto/experimental/algorithms/test/lwe_extended_programmable_bootstrapping.rs line 119 at r1 (raw file):
// Verify our formulas work the way we expect by comparing to the reference not using // the trick assert_eq!(ref_small_rotated_luts, small_luts_rotated_with_trick);
Putting computation of ref_small_rotated_luts and small_luts_rotated_with_trick in separate functions with extended_lut as input would help follow
tfhe/src/core_crypto/experimental/algorithms/test/lwe_extended_programmable_bootstrapping.rs line 245 at r1 (raw file):
let mut buffers = ComputationBuffers::new(); // TODO: have req for main thread and for workers ?
Keep TODO?
|
I think we should return the full extended LUT |
Brings the extended PBS to the experimental module of core_crypto
No entities for now to make the initial integration easier, it was deemed ok since the new type would only combine a BSK and an extension factor, not worth all the code for something experimental that can be used as is if we want in the shortint dyn atomic pattern