Am/test/multi bit noise check

[IceTDrinker]

See comments below on how to attack this PR

---

This change is 

[IceTDrinker]

This PR introduces a few traits, implements them at the shortint level to manage both a full multi bit PBS and a multi bit blind rotate (in case we need to finish a PBS after a multi bit PBS)

to manage the various supported cases of shortint I had to introduce yet another type in the hierarchy the DynModulusSwitchedLwe which can house the classic and multi bit mod switched ciphertexts given they don't work the same.

the multi bit variant needs a bit of special code to simulate the actual decryption happening during the blind rotate given how the mod switch is done

also introduced a generic bootstrap trait to be able to dispatch either classic or multi bit PBS and reuse the existing test harnesses

I'm available for questions

[IceTDrinker]

commits should be roughly readable one by one

[mayeul-zama]

Would be easier and less error prone to convert MultiBitModulusSwitchedCt into a StandardModulusSwitchedLweCiphertext
And then to decrypt it (by converting it to a LweCt I guess)

[IceTDrinker]

how would you do the conversion given the actual coefficient being subtracted needs to be added and then mod switched for a big part of them ?

recall here that the mod switch happens after coefficients are added together and so are keeping extra information vs first modswitching

[mayeul-zama]

Indeed, looks good as is!
Wonder if this decryption function shouldn't be somewhere in core

[IceTDrinker]

could be, we don't have these checks/tests in core, but the plan is to move some of those noise check functions to core IIRC, so could move some of those helper functions

[mayeul-zama]

Ok

[mayeul-zama]

@mayeul-zama reviewed 17 of 17 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @guillermo-oyarzun)

[IceTDrinker]

small bug in a "matches actual" I need to fix

[IceTDrinker]

@guillermo-oyarzun the thing to simplify decryption stuff you were asking about in your other PR

[mayeul-zama]

@mayeul-zama reviewed 2 of 4 files at r2, 20 of 21 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @agnesLeroy, @guillermo-oyarzun, and @IceTDrinker)

---

tfhe/src/shortint/server_key/tests/noise_distribution/utils/noise_simulation.rs line 1922 at r4 (raw file):

                        thread_count,
                        deterministic_execution: _,
                    } => match input {

Shouldn't thisreturn an error (KS32 and MultiBit not compatible)

---

tfhe/src/shortint/server_key/tests/noise_distribution/utils/noise_simulation.rs line 2014 at r4 (raw file):

                }
                Shortint128BootstrappingKey::MultiBit { .. } => self
                    .lwe_multi_bit_fft_128_blind_rotate(input, output, accumulator, side_resources),

Shouldn't this branch return an error (KS32 and MultiBit not compatible)

---

tfhe/src/core_crypto/commons/noise_formulas/noise_simulation/lwe_multi_bit_programmable_bootstrap.rs line 294 at r4 (raw file):

            grouping_factor,
            noise_distribution: _,
            modulus: _,

Can't the modulus be checked?

---

tfhe/src/core_crypto/commons/noise_formulas/noise_simulation/lwe_multi_bit_programmable_bootstrap.rs line 373 at r4 (raw file):

        let br_additive_variance = match self.noise_distribution() {
            DynamicDistribution::Gaussian(_) => match grouping_factor.0 {
                2 => multi_bit_pbs_variance_132_bits_security_gaussian_gf_2_fft_mul(

Not sure the calling function makes sure we target 132 bits of security
Maybe we could have an assert

---

tfhe/src/shortint/atomic_pattern/mod.rs line 628 at r4 (raw file):

    }

    pub fn set_deterministic_execution(&mut self, do_it: bool) {

do_it name is weird to me
I suggest deterministic_execution

Applies to other places

---

tfhe/src/core_crypto/commons/noise_formulas/noise_simulation/lwe_programmable_bootstrap.rs line 283 at r4 (raw file):

        let br_additive_variance = match self.noise_distribution() {
            DynamicDistribution::Gaussian(_) => pbs_variance_132_bits_security_gaussian_fft_mul(

Fomula is the same for pbs 64 and 128 I guess (with the new matissa_size param)
Is that right?

---

tfhe/src/core_crypto/commons/noise_formulas/lwe_multi_bit_programmable_bootstrap.rs line 52 at r4 (raw file):

        * (0.0022
            * (2.0
                * if (1.0 * mantissa_size - core::f64::consts::LOG2_E * modulus.ln() >= 0.0) {

Why multiply by 1.0?
Same in other places
Same with -1.0 * mantissa_size instead of -mantissa_size

[mayeul-zama]

Indeed, looks good as is!
Wonder if this decryption function shouldn't be somewhere in core

[IceTDrinker]

Will be rebasing this, I think I answered all your questions @mayeul-zama

@IceTDrinker reviewed 1 file and made 9 comments.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @agnesLeroy, @guillermo-oyarzun, and @mayeul-zama).

---

tfhe/src/core_crypto/commons/noise_formulas/lwe_multi_bit_programmable_bootstrap.rs line 52 at r4 (raw file):

Previously, mayeul-zama wrote…

Why multiply by 1.0?
Same in other places
Same with -1.0 * mantissa_size instead of -mantissa_size

code is auto generated, I prefer not to touch the formulas so kept it as is

---

tfhe/src/core_crypto/commons/noise_formulas/noise_simulation/lwe_multi_bit_programmable_bootstrap.rs line 294 at r4 (raw file):

Previously, mayeul-zama wrote…

Can't the modulus be checked?

bsk is in the fourier domain and the modulus information is lost since it's treated as being on the "real" torus, i.e. we lose the discretization information which does not matter anymore in the fourier domain (seeing the torus as being the real un-descretized torus)

---

tfhe/src/core_crypto/commons/noise_formulas/noise_simulation/lwe_multi_bit_programmable_bootstrap.rs line 373 at r4 (raw file):

Previously, mayeul-zama wrote…

Not sure the calling function makes sure we target 132 bits of security
Maybe we could have an assert

that's a good point, today we don't have "dynamic" security noises, they are hard coded, so for now forcing everything to be 132 bits. Later on, the security level won't matter given hopefully we'll have dynamic noises, i.e. we can give the noises we used for encryption as parameters to the formula

---

tfhe/src/core_crypto/commons/noise_formulas/noise_simulation/lwe_programmable_bootstrap.rs line 283 at r4 (raw file):

Previously, mayeul-zama wrote…

Fomula is the same for pbs 64 and 128 I guess (with the new matissa_size param)
Is that right?

correct, I still need to check that for sure but the code was the same when I looked at it, just need a last confirmation

---

tfhe/src/shortint/atomic_pattern/mod.rs line 628 at r4 (raw file):

Previously, mayeul-zama wrote…

do_it name is weird to me
I suggest deterministic_execution

Applies to other places

do_it is a general pattern I saw in other code bases essentially:

set_something(do_it: bool)

if you don't like it I can rename

---

tfhe/src/shortint/server_key/tests/noise_distribution/utils/noise_simulation.rs line 1922 at r4 (raw file):

Previously, mayeul-zama wrote…

Shouldn't thisreturn an error (KS32 and MultiBit not compatible)

actually this is the PBS 128 and it is compatible, we don't have the limitation at that level

---

tfhe/src/shortint/server_key/tests/noise_distribution/utils/noise_simulation.rs line 2014 at r4 (raw file):

Previously, mayeul-zama wrote…

Shouldn't this branch return an error (KS32 and MultiBit not compatible)

same remark, this is the PBS 128, it takes the post KS ciphertext and does not care if it's 32 or 64 bits

the organization of the legacy ServerKey may not be the best at the moment, but it limits how many cases we have to handle so I think we kept it the way it is currently

[IceTDrinker]

could be, we don't have these checks/tests in core, but the plan is to move some of those noise check functions to core IIRC, so could move some of those helper functions

[mayeul-zama]

LGTM
Thanks!

pbs_variance_132_bits_security_gaussian_fft_mul to be verified

@mayeul-zama reviewed 8 files and all commit messages, made 6 comments, and resolved 3 discussions.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @agnesLeroy, @guillermo-oyarzun, @IceTDrinker, and @nsarlin-zama).

---

tfhe/src/core_crypto/commons/noise_formulas/noise_simulation/lwe_programmable_bootstrap.rs line 283 at r4 (raw file):

Previously, IceTDrinker wrote…

correct, I still need to check that for sure but the code was the same when I looked at it, just need a last confirmation

Keeping this comment alive to not forget it

---

tfhe/src/shortint/atomic_pattern/mod.rs line 628 at r4 (raw file):

Previously, IceTDrinker wrote…

do_it is a general pattern I saw in other code bases essentially:

set_something(do_it: bool)

if you don't like it I can rename

Ok, I never saw this pattern
Good for me

---

tfhe/src/shortint/server_key/tests/noise_distribution/utils/noise_simulation.rs line 1922 at r4 (raw file):

Previously, IceTDrinker wrote…

actually this is the PBS 128 and it is compatible, we don't have the limitation at that level

Ok

---

tfhe/src/shortint/server_key/tests/noise_distribution/utils/noise_simulation.rs line 2014 at r4 (raw file):

Previously, IceTDrinker wrote…

same remark, this is the PBS 128, it takes the post KS ciphertext and does not care if it's 32 or 64 bits

the organization of the legacy ServerKey may not be the best at the moment, but it limits how many cases we have to handle so I think we kept it the way it is currently

Ok

[mayeul-zama]

Ok