feat(core): add common mask

[mayeul-zama]

This change is 

[IceTDrinker]

First pass on some nits I saw, will do a more complete review when I get the chance

@IceTDrinker reviewed 14 files and all commit messages, and made 13 comments.
Reviewable status: 14 of 44 files reviewed, 12 unresolved discussions (waiting on mayeul-zama, soonum, and SouchonTheo).

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 93 at r2 (raw file):

    }
}
pub type GlweBodyOwned<Scalar> = GlweBody<Vec<Scalar>>;

perhaps those could be put in the experimental section ? so we don't have to write all the docstrings fully e.g.

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 128 at r2 (raw file):

    fn create_from(from: &[T], meta: Self::Metadata) -> GlweBodyListView<'_, T> {
        let GlweBodyListCreationMetadata {
            ciphertext_modulus,

sorry but this triggers my OCD 😅 having the fields inverted

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 131 at r2 (raw file):

            polynomial_size,
        } = meta;
        GlweBodyList {

should use from_container for data size validation

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 145 at r2 (raw file):

    fn create_from(from: &mut [T], meta: Self::Metadata) -> GlweBodyListMutView<'_, T> {
        let GlweBodyListCreationMetadata {
            ciphertext_modulus,

same here

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 148 at r2 (raw file):

            polynomial_size,
        } = meta;
        GlweBodyList {

should use from_container for data size validation

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 162 at r2 (raw file):

        ciphertext_modulus: CiphertextModulus<Scalar>,
    ) -> Self {
        Self {

from_container should have a verification that the data has proper size, in that case I believe the container should have exactly polynomila_size coeffs

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 169 at r2 (raw file):

    }

    pub fn data(self) -> C {

I believe the pattern in core is into_container

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 174 at r2 (raw file):

    pub fn lwe_body_count(&self) -> LweBodyCount {
        LweBodyCount(self.data.container_len() / self.polynomial_size.0)

that cannot be right, for two reasons, we have a GlweBodyList, so the return new type looks wrong

here the glwe body is a polynomial so returning how many polynomials/GLWE bodies are in the list right ?

---

tfhe/src/core_crypto/entities/lwe_ciphertext.rs line 153 at r1 (raw file):

    }

    pub fn data(self) -> C {

into_container ?

---

tfhe/src/core_crypto/entities/lwe_ciphertext.rs line 266 at r1 (raw file):

    }

    pub fn data(self) -> C {

same ?

---

tfhe/src/core_crypto/experimental/algorithms/mod.rs line 1 at r2 (raw file):

mod common_mask;

make it pub like the others ?

---

tfhe/src/core_crypto/experimental/entities/mod.rs line 1 at r2 (raw file):

mod common_mask;

pub ?

[IceTDrinker]

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 148 at r2 (raw file):

Previously, IceTDrinker wrote…

should use from_container for data size validation

actually ignore this comment, it was likely for performance in that case since those are called by iterators

[IceTDrinker]

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 131 at r2 (raw file):

Previously, IceTDrinker wrote…

should use from_container for data size validation

actually ignore this comment, it was likely for performance in that case since those are called by iterators

[IceTDrinker]

Another batch

@IceTDrinker reviewed 5 files and made 17 comments.
Reviewable status: 18 of 44 files reviewed, 26 unresolved discussions (waiting on mayeul-zama, soonum, and SouchonTheo).

---

tfhe/src/core_crypto/experimental/algorithms/common_mask/cm_lwe_programmable_bootstrapping/mod.rs line 57 at r2 (raw file):

    let accumulator_scalar = (0..cm_dimension.0)
        .flat_map(|_| accumulator_scalar.iter())

apparently we can apply different LUTs on different bodies, not sure if relevant here, but maybe we could have a TODO to have different functions ?

check the many lut stuff maybe for the way to manage the function pointers

---

tfhe/src/core_crypto/experimental/entities/common_mask/cm_lwe_bootstrap_key.rs line 10 at r2 (raw file):

#[derive(Clone, Copy, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
// Versionize
//#[versionize(CmLweBootstrapKeyVersions)]

Versioning to remove ?

---

tfhe/src/core_crypto/experimental/entities/common_mask/cm_lwe_ciphertext.rs line 3 at r2 (raw file):

//! Module containing the definition of the [`CmLweCiphertext`].

use crate::conformance::ParameterSetConformant;

is that useful for experimental primitives ?

---

tfhe/src/core_crypto/experimental/entities/common_mask/cm_lwe_ciphertext.rs line 34 at r2 (raw file):

// But these functions should not be used in other contexts, hence the `#[cfg(test)]`
#[cfg(test)]
#[allow(dead_code)]

do we use this for experimental tests ?

---

tfhe/src/core_crypto/experimental/entities/common_mask/cm_lwe_ciphertext.rs line 164 at r2 (raw file):

            && self.ciphertext_modulus() == lwe_ct_parameters.ct_modulus
    }
}

I think the conformance can be removed, in addition I believe it is wrong since there are no checks on the cm dimension

---

tfhe/src/core_crypto/experimental/fft64/common_mask/cm_bootstrap.rs line 279 at r2 (raw file):

impl FourierCmLweBootstrapKeyView<'_> {
    // CastInto required for PBS modulus switch which returns a usize
    pub fn cm_blind_rotate_assign<InputScalar, OutputScalar>(

the fft impl stuff is a legacy structure for the code, is there a way to put the relevant code in hte cm_bootstrap algo module and avoid having the methods implemented directly on the FourierCmBsk ?

---

tfhe/src/core_crypto/experimental/fft64/common_mask/cm_bootstrap.rs line 413 at r2 (raw file):

        // }

        // println!();

old comments to remove above

---

tfhe/src/core_crypto/experimental/fft64/common_mask/cm_bootstrap.rs line 430 at r2 (raw file):

        // }

        // println!();

old comments to remove above

---

tfhe/src/core_crypto/experimental/fft64/common_mask/cm_ggsw.rs line 496 at r2 (raw file):

#[cfg_attr(feature = "__profiling", inline(never))]
pub fn cm_add_external_product_assign<Scalar>(

similarly, maybe a module for the ggsw ext prod algos ?

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 75 at r2 (raw file):

        let cm_dimension = cm_param.cm_dimension;

        let total_number = cm_dimension.0;

total number of what ?

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 83 at r2 (raw file):

        let lwe_noise_distribution = DynamicDistribution::<u64>::new_gaussian_from_std_dev(
            StandardDev(cm_param.lwe_std_dev),

cm_params don't store the new type directly ?

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 86 at r2 (raw file):

        );

        let ciphertext_modulus = CiphertextModulus::<u64>::new_native();

same here, could be stored ?

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 87 at r2 (raw file):

        let ciphertext_modulus = CiphertextModulus::<u64>::new_native();
        let encoding_with_padding = 1 << 63;

there should be a function that does that automatically based on the modulus I think ?

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 126 at r2 (raw file):

        let plaintexts =
            PlaintextList::from_container((0..cm_dimension.0).map(|_| 0).collect_vec());

vec![0; cm_dimension.0]

instead of the collect ?

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 138 at r2 (raw file):

        let mut ct_after_ks = CmLweCiphertext::new(
            0u64,
            fbsk.input_lwe_dimension(),

ksk.output_lwe_size().to_lwe_dimension() ?

just because it's written as ct_after_ks, doesn't mean we have to do it

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 150 at r2 (raw file):

        );

        let max_nb_zeros_n = (cm_param.max_nb_zeros_n + 1.) as usize;

suspicious

why would max_nb_zeros be a float when converted to parameters ?

also this op is a ceil per se

[mayeul-zama]

@mayeul-zama made 22 comments and resolved 14 discussions.
Reviewable status: 5 of 51 files reviewed, 12 unresolved discussions (waiting on IceTDrinker, soonum, and SouchonTheo).

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 93 at r2 (raw file):

Previously, IceTDrinker wrote…

perhaps those could be put in the experimental section ? so we don't have to write all the docstrings fully e.g.

I think it's simpler to keep it here
The similar lwe code does not have comments anyway

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 128 at r2 (raw file):

Previously, IceTDrinker wrote…

sorry but this triggers my OCD 😅 having the fields inverted

Done

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 145 at r2 (raw file):

Previously, IceTDrinker wrote…

same here

Done

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 162 at r2 (raw file):

Previously, IceTDrinker wrote…

from_container should have a verification that the data has proper size, in that case I believe the container should have exactly polynomila_size coeffs

Done

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 169 at r2 (raw file):

Previously, IceTDrinker wrote…

I believe the pattern in core is into_container

Done

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 174 at r2 (raw file):

Previously, IceTDrinker wrote…

that cannot be right, for two reasons, we have a GlweBodyList, so the return new type looks wrong

here the glwe body is a polynomial so returning how many polynomials/GLWE bodies are in the list right ?

Removed

---

tfhe/src/core_crypto/entities/lwe_ciphertext.rs line 153 at r1 (raw file):

Previously, IceTDrinker wrote…

into_container ?

Done

---

tfhe/src/core_crypto/entities/lwe_ciphertext.rs line 266 at r1 (raw file):

Previously, IceTDrinker wrote…

same ?

Done

---

tfhe/src/core_crypto/experimental/algorithms/mod.rs line 1 at r2 (raw file):

Previously, IceTDrinker wrote…

make it pub like the others ?

Done

---

tfhe/src/core_crypto/experimental/entities/mod.rs line 1 at r2 (raw file):

Previously, IceTDrinker wrote…

pub ?

Done

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 126 at r2 (raw file):

Previously, IceTDrinker wrote…

vec![0; cm_dimension.0]

instead of the collect ?

Done

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 138 at r2 (raw file):

Previously, IceTDrinker wrote…

ksk.output_lwe_size().to_lwe_dimension() ?

just because it's written as ct_after_ks, doesn't mean we have to do it

Done

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 150 at r2 (raw file):

Previously, IceTDrinker wrote…

suspicious

why would max_nb_zeros be a float when converted to parameters ?

also this op is a ceil per se

Change to ceil

---

tfhe/src/core_crypto/experimental/algorithms/common_mask/cm_lwe_programmable_bootstrapping/mod.rs line 57 at r2 (raw file):

Previously, IceTDrinker wrote…

apparently we can apply different LUTs on different bodies, not sure if relevant here, but maybe we could have a TODO to have different functions ?

check the many lut stuff maybe for the way to manage the function pointers

Added a TODO

---

tfhe/src/core_crypto/experimental/fft64/common_mask/cm_bootstrap.rs line 279 at r2 (raw file):

Previously, IceTDrinker wrote…

the fft impl stuff is a legacy structure for the code, is there a way to put the relevant code in hte cm_bootstrap algo module and avoid having the methods implemented directly on the FourierCmBsk ?

Done

---

tfhe/src/core_crypto/experimental/fft64/common_mask/cm_bootstrap.rs line 413 at r2 (raw file):

Previously, IceTDrinker wrote…

old comments to remove above

Done

---

tfhe/src/core_crypto/experimental/fft64/common_mask/cm_bootstrap.rs line 430 at r2 (raw file):

Previously, IceTDrinker wrote…

old comments to remove above

Done

---

tfhe/src/core_crypto/experimental/fft64/common_mask/cm_ggsw.rs line 496 at r2 (raw file):

Previously, IceTDrinker wrote…

similarly, maybe a module for the ggsw ext prod algos ?

Done

---

tfhe/src/core_crypto/experimental/entities/common_mask/cm_lwe_bootstrap_key.rs line 10 at r2 (raw file):

Previously, IceTDrinker wrote…

Versioning to remove ?

Removed

---

tfhe/src/core_crypto/experimental/entities/common_mask/cm_lwe_ciphertext.rs line 3 at r2 (raw file):

Previously, IceTDrinker wrote…

is that useful for experimental primitives ?

Removed

---

tfhe/src/core_crypto/experimental/entities/common_mask/cm_lwe_ciphertext.rs line 34 at r2 (raw file):

Previously, IceTDrinker wrote…

do we use this for experimental tests ?

Removed

---

tfhe/src/core_crypto/experimental/entities/common_mask/cm_lwe_ciphertext.rs line 164 at r2 (raw file):

Previously, IceTDrinker wrote…

I think the conformance can be removed, in addition I believe it is wrong since there are no checks on the cm dimension

Removed

[mayeul-zama]

tfhe/src/core_crypto/experimental/fft64/common_mask/cm_bootstrap.rs line 279 at r2 (raw file):

Previously, mayeul-zama wrote…

Done

Shouldn't we do it to the PBS code?

[mayeul-zama]

@mayeul-zama made 4 comments and resolved 2 discussions.
Reviewable status: 5 of 51 files reviewed, 11 unresolved discussions (waiting on IceTDrinker, soonum, and SouchonTheo).

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 75 at r2 (raw file):

Previously, IceTDrinker wrote…

total number of what ?

Removed

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 83 at r2 (raw file):

Previously, IceTDrinker wrote…

cm_params don't store the new type directly ?

Done

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 86 at r2 (raw file):

Previously, IceTDrinker wrote…

same here, could be stored ?

Done

---

tfhe-benchmark/benches/core_crypto/cm_bench.rs line 87 at r2 (raw file):

Previously, IceTDrinker wrote…

there should be a function that does that automatically based on the modulus I think ?

I added a assert on the ciphertext_modulus. I don't think generalizing here is worth it

[IceTDrinker]

another batch of comments

@IceTDrinker partially reviewed 26 files and all commit messages, made 37 comments, and resolved 9 discussions.
Reviewable status: 31 of 51 files reviewed, 36 unresolved discussions (waiting on mayeul-zama, soonum, and SouchonTheo).

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 174 at r2 (raw file):

Previously, mayeul-zama wrote…

Removed

If you need it you can add a new type for GlweBodyCount, that's ok 👍

---

tfhe/src/core_crypto/experimental/fft64/common_mask/cm_bootstrap.rs line 279 at r2 (raw file):

Previously, mayeul-zama wrote…

Shouldn't we do it to the PBS code?

at least the bootstrap code yeah, but we could do it for the blind rotate code too, that's what I'm doing for the extended PBS

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_ciphertext.rs line 133 at r3 (raw file):

#[derive(Copy, Clone)]
pub struct CmLweCiphertextParameters<T: UnsignedInteger> {

parameters can be removed ? I believe they are only related to the conformance ? or am I mistaken ?

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_compression.rs line 14 at r4 (raw file):

#[test]
fn cm_compression() {
    cm_compression_generic(TEST_PARAMS_4_BITS_NATIVE_U64);

seems a bit odd to me to mix this param set ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_params.rs line 32 at r4 (raw file):

mod raw {
    use super::RawType;
    

leftover spaces, I saw the rustfmt::skip above

could you maybe dump the parameters once converted so that we can get rid of the raw params ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key.rs line 5 at r4 (raw file):

use super::cm_lwe_compression_key_part::{
    CmLweCompressionKeyPartCreationMetadata, CmLweCompressionKeyPartMutView,
    CmLweCompressionKeyPartView,

Maybe you can explain or point to a reference for how the key is structured ?

also are those key parts maybe CmLweCiphertextList ? similarly to how a part of the KSK for LWE can be seen as an LweCiphertextList ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key.rs line 13 at r4 (raw file):

#[derive(Clone, Copy, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
pub struct CmLweCompressionKey<C: Container>

is it called compression in the paper or is it seen as a packing key ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key.rs line 44 at r4 (raw file):

) -> usize {
    // One ciphertext per level encrypted under the output key
    decomp_level_count.0 * (output_lwe_dimension.0 + output_cm_dimension.0) * input_lwe_dimension.0

why is it multiplied by input_lwe_dimension here ?

Maybe I'm missing how this key is supposed to be structured

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key.rs line 61 at r4 (raw file):

        );
        assert_eq!(
            container.container_len() % (decomp_level_count.0 * (output_lwe_dimension.0 + output_cm_dimension.0) * output_cm_dimension.0),

if this size corresponds to something we could have a size function

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key.rs line 95 at r4 (raw file):

                / (self.output_lwe_dimension.0 + self.output_cm_dimension.0)
                / self.output_cm_dimension.0
                / self.decomp_level_count.0,

is there a size formula to replace all the divisions ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key.rs line 107 at r4 (raw file):

    }

    pub fn input_key_element_encrypted_size(&self) -> usize {

element here being a complete key then given the input_key_lwe_dimension being used ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key.rs line 163 at r4 (raw file):

    }

    pub fn as_mut_csr_lwe_ciphertext_list(&mut self) -> CmLweCiphertextListMutView<'_, Scalar> {

csr ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key.rs line 189 at r4 (raw file):

        input_key_lwe_dimension: LweDimension,
        output_lwe_dimension: LweDimension,
        output_key_cm_dimension: CmDimension,

inconsitency in the naming output/output_key, I think

it seems we don't refer to key sizes in the from_container of the LWE packing or LWE ksk in the non experimental part of the lib, so maybe we can drop the key from the naming here for parameters ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key.rs line 235 at r4 (raw file):

            ciphertext_modulus,
        } = meta;
        Self::from_container(

same here for the from container, my mistake

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 27 at r4 (raw file):

fn test_cm_pbs_noise_analysis(params: &CmApParams) {
    let cm_dimension = params.cm_dimension;
    let ciphertext_modulus = CiphertextModulus::<u64>::new_native();

same for all tests: use modulus from params

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 29 at r4 (raw file):

    let ciphertext_modulus = CiphertextModulus::<u64>::new_native();

    let small_lwe_noise_distribution = DynamicDistribution::new_gaussian(

use from params

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 115 at r4 (raw file):

    let noises_separated: Vec<Vec<f64>> = (0..cm_dimension.0)
        .map(|i| noises.iter().map(|list| list[i]).collect_vec())

flatten().collect() maybe ?

https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.flatten

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 130 at r4 (raw file):

    for noises in &noises_separated {
        let noises: &[f64] = noises;

type annotation needed ? 🧐

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 141 at r4 (raw file):

        println!("p_value normality: {}", test_result.p_value);

        assert!(check_both_ratio_under(expected_variance, variance.0, 1.1));

let's have a todo to manage confidence interval here

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 145 at r4 (raw file):

}

pub fn pbs_variance_132_bits_security_gaussian_impl_2(

do we have a way to maybe auto generate this formula ?

why is it called impl_2 btw ?

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 184 at r4 (raw file):

fn test_cm_pbs_ap_sequence(params: &CmApParams) {
    let cm_dimension = params.cm_dimension;
    let ciphertext_modulus = CiphertextModulus::<u64>::new_native();

remark for params use in tests, modulus, cm_dimension, noise distribution etc.

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 190 at r4 (raw file):

    );

    let big_lwe_noise_distribution =

glwe instead of big_lwe ?

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 253 at r4 (raw file):

    );

    let norm2 = 2_f64.powf(params.log_nu).round() as u64;

floor instead of round to avoid going overboard ?

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 255 at r4 (raw file):

    let norm2 = 2_f64.powf(params.log_nu).round() as u64;

    let max_nb_zeros_n = (params.max_nb_zeros_n + 1.) as usize;

could you maybe have a method on the params to return the proper count or do the transformation when you instantiate the parameters ?

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 293 at r4 (raw file):

        for _ in 0..3 {
            for i in ct_big.as_mut() {
                *i = i.wrapping_mul(norm2);

if it's a linear algebra clear mul, could we have something similar to what we have in the lwe_linear_algebra module ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_glwe_ciphertext.rs line 76 at r4 (raw file):

        assert_eq!(
            container.container_len(),
            (glwe_dimension.0 + cm_dimension.0) * polynomial_size.0

cm_glwe_ciphertext_size ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_ciphertext_list.rs line 147 at r4 (raw file):

            ciphertext_modulus,
        } = meta;
        Self::from_container(from, lwe_dimension, cm_dimension, ciphertext_modulus)

so: sorry about that but let's not keep it as we discussed, the create_from can be called in a loop, having additional checks may not allow to generate streamlined iter code

same for other create_from

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_keyswitch.rs line 24 at r4 (raw file):

    let in_lwe_noise_distribution = DynamicDistribution::new_gaussian(
        minimal_lwe_variance_for_132_bits_security_gaussian(in_lwe_dimension, 2_f64.powi(64)),

can you check the locations where CmApParams is used to update the ciphertext modulus to take it directly from the params ?

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_keyswitch.rs line 29 at r4 (raw file):

        minimal_lwe_variance_for_132_bits_security_gaussian(out_lwe_dimension, 2_f64.powi(64)),
    );
    let ciphertext_modulus = CiphertextModulus::<u64>::new_native();

use the one from params

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_keyswitch.rs line 35 at r4 (raw file):

    let ks_decomp_level_count = params.level_ks;

    let cm_dimension = CmDimension(10);

is it not part of the parameters ? 🧐

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_keyswitch_key.rs line 15 at r4 (raw file):

    decomp_base_log: DecompositionBaseLog,
    decomp_level_count: DecompositionLevelCount,
    input_lwe_dimension: LweDimension,

do we need to keep the input lwe dim here ?

if not does that mean that a "key part" for the compression is "just" a CmLweKeyswitchKey ? and so we could get rid of the key part ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_keyswitch_key.rs line 57 at r4 (raw file):

        );
        assert!(
            container.container_len().is_multiple_of(decomp_level_count.0 * (output_lwe_dimension.0 + cm_dimension.0)),

use the size function ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_keyswitch_key.rs line 154 at r4 (raw file):

    }

    pub fn as_mut_lwe_ciphertext_list(&mut self) -> CmLweCiphertextListMutView<'_, Scalar> {

as_mut_cm_lwe_ciphertext_list ?

probably similar stuff elsewhere for fn as_, a ctrl + F on that pattern should find them in experimental/cm stuff

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key_part.rs line 1 at r4 (raw file):

//! Module containing the definition of the [`CmLweCompressionKeyPart`].

is a key part the CM LWE encryption of a single input key under the output key ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key_part.rs line 89 at r4 (raw file):

        LweDimension(
            self.data.container_len()
                / (self.output_lwe_dimension.0 + self.output_cm_dimension.0)

use a size primitive as denominator

feels like the key part might "just" be a CmLweKeySwitchKey so we may be able to get rid of the key part and replace its uses by a CmLweKeySwitchKey ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_compression_key_part.rs line 157 at r4 (raw file):

    }

    pub fn as_mut_csr_lwe_ciphertext_list(&mut self) -> CmLweCiphertextListMutView<'_, Scalar> {

csr ?

to be checked elsewhere

[IceTDrinker]

@IceTDrinker partially reviewed 4 files and made 24 comments.
Reviewable status: 35 of 51 files reviewed, 60 unresolved discussions (waiting on mayeul-zama, soonum, and SouchonTheo).

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_bootstrap.rs line 19 at r4 (raw file):

use itertools::{izip, Itertools};

pub fn blind_rotate_scratch<Scalar>(

cm_blind_rotate_scratch ?

also scratch naming for publicly facing stuff is ${function_name}_requirement

so here would be blind_rotate_requirement

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_bootstrap.rs line 75 at r4 (raw file):

// CastInto required for PBS modulus switch which returns a usize
pub fn cm_blind_rotate_assign_raw<InputScalar, OutputScalar>(

raw ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_bootstrap.rs line 104 at r4 (raw file):

            let (tmp_poly, _) = stack.make_aligned_raw(poly.as_ref().len(), CACHELINE_ALIGN);

            let mut tmp_poly = Polynomial::from_container(&mut *tmp_poly);

reborrow should not be required with the modern versions of dynstack

check all stack.something calls

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_bootstrap.rs line 113 at r4 (raw file):

    let (ct1, stack) = stack.make_aligned_raw(ct0.as_ref().len(), CACHELINE_ALIGN);
    let mut ct1 = CmGlweCiphertextMutView::from_container(
        &mut *ct1,

reborrow should not be required with the modern versions of dynstack

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_bootstrap.rs line 193 at r4 (raw file):

        stack.collect_aligned(CACHELINE_ALIGN, accumulator.as_ref().iter().copied());
    let mut local_accumulator = CmGlweCiphertextMutView::from_container(
        &mut *local_accumulator_data,

reborrow should not be needed

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_external_product.rs line 18 at r4 (raw file):

use tfhe_fft::c64;

pub fn cm_add_external_product_assign_scratch<Scalar>(

scratch -> requirement

same for the rest of the file

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_external_product.rs line 76 at r4 (raw file):

    // the cost of filling it up with zeros. `is_output_uninit` is set to `false` once
    // it has been fully initialized for the first time.
    let output_fft_buffer = &mut *output_fft_buffer;

reborrow not useful

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_external_product.rs line 93 at r4 (raw file):

        // We loop through the levels (we reverse to match the order of the decomposition iterator.)
        ggsw.into_levels().rev().for_each(|ggsw_decomp_matrix| {

we should not have a rev() here, all levels during compute should be traversed without reversing the levels (it has been the case since 0.10 if I'm not mistaken)

encryption should be adapted to allow for this at runtime

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_external_product.rs line 171 at r4 (raw file):

#[cfg_attr(feature = "__profiling", inline(never))]
pub(crate) fn collect_next_term<'a, Scalar: UnsignedTorus>(

duplicated from the fft64 non experimental module ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_external_product.rs line 182 at r4 (raw file):

#[cfg_attr(feature = "__profiling", inline(never))]
pub(crate) fn update_with_fmadd(

duplicated from the non experimental module ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_external_product.rs line 301 at r4 (raw file):

    #[test]
    fn test_cm_ep() {

ep ?

also what is this test about ?

is it still needed if you have the AP test ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_encryption.rs line 20 at r4 (raw file):

use rayon::prelude::*;

pub fn cm_ggsw_encryption_multiplicative_factor<Scalar: UnsignedInteger>(

I believe this function is a duplicate of the existing one for the GGSWs

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_encryption.rs line 49 at r4 (raw file):

    glwe_secret_keys: &[GlweSecretKey<KeyCont>],
    output: &mut CmGgswCiphertext<OutputCont>,
    cleartexts: &[Cleartext<Scalar>],

pattern in core is to use CleartextList, but if it's annoying to change here, mark it as TODO

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_encryption.rs line 60 at r4 (raw file):

{
    assert!(
        output.polynomial_size() == glwe_secret_keys[0].polynomial_size(),

assert should be for all keys so glwe_secret_keys.iter().all(...) ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_encryption.rs line 68 at r4 (raw file):

    assert!(
        output.glwe_dimension() == glwe_secret_keys[0].glwe_dimension(),

same here

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_encryption.rs line 125 at r4 (raw file):

}

pub fn par_encrypt_constant_cm_ggsw_ciphertext<

same remarks apply here

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_encryption.rs line 214 at r4 (raw file):

fn encrypt_constant_cm_ggsw_level_matrix_row<Scalar, NoiseDistribution, KeyCont, OutputCont, Gen>(
    glwe_secret_keys: &[GlweSecretKey<KeyCont>],
    (row_index, last_row_index): (usize, usize),

maybe last_row_index should be renamed since with several bodies it's not really accurate anymore ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_ggsw_encryption.rs line 253 at r4 (raw file):

        bodies.as_mut().fill(Scalar::ZERO);

        let encoded = factors[row_index - last_row_index].wrapping_neg();

a small comment on the indexing and a rough sketch on how bodies are laid out would be welcome

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_glwe_encryption.rs line 89 at r4 (raw file):

{
    assert!(
        output.glwe_dimension() == glwe_secret_keys[0].glwe_dimension(),

.iter().all to check all glwe keys

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_glwe_encryption.rs line 96 at r4 (raw file):

    );
    assert!(
        output.polynomial_size() == glwe_secret_keys[0].polynomial_size(),

same

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_glwe_encryption.rs line 204 at r4 (raw file):

    );
    assert!(
        output_glwe_ciphertext.glwe_dimension() == glwe_secret_keys[0].glwe_dimension(),

.iter().all(...)

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_glwe_encryption.rs line 211 at r4 (raw file):

    );
    assert!(
        output_glwe_ciphertext.polynomial_size() == glwe_secret_keys[0].polynomial_size(),

.iter().all(...)

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_glwe_encryption.rs line 248 at r4 (raw file):

    );
    assert!(
        glwe_secret_keys[0].glwe_dimension() == input_cm_glwe_ciphertext.glwe_dimension(),

.iter().all(...)

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_glwe_encryption.rs line 254 at r4 (raw file):

    );
    assert!(
        glwe_secret_keys[0].polynomial_size() == input_cm_glwe_ciphertext.polynomial_size(),

.iter().all(...)

[IceTDrinker]

Have a meeting, so pushing the current batch of comments

@IceTDrinker made 1 comment.
Reviewable status: 35 of 51 files reviewed, 60 unresolved discussions (waiting on mayeul-zama, soonum, and SouchonTheo).

[IceTDrinker]

I still have 11 files to go, another batch, I need a break or my brain will melt

@IceTDrinker partially reviewed 5 files and made 20 comments.
Reviewable status: 40 of 51 files reviewed, 79 unresolved discussions (waiting on mayeul-zama, soonum, and SouchonTheo).

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_glwe_encryption.rs line 2 at r4 (raw file):

//! Module containing primitives pertaining to [`GLWE ciphertext
//! encryption`](`GlweCiphertext#glwe-encryption`).

docstring is inaccurate, maybe this could still refer to the encryption, but add that several bodies are encrypted sharing a mask

given it's the CM variant

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_glwe_sample_extraction.rs line 3 at r4 (raw file):

//! Module containing primitives pertaining to the operation usually referred to as a
//! _sample extract_ in the literature. Allowing to extract a single
//! [`LWE Ciphertext`](`CmLweCiphertext`) from a given [`GLWE ciphertext`](`CmGlweCiphertext`).

doc string is inaccurate, for cm several LWEs can get extracted right ?

also LWE and GLWE here would be CM ... ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 2 at r4 (raw file):

//! Module containing primitives pertaining to the generation of
//! [`standard LWE bootstrap keys`](`LweBootstrapKey`) and [`seeded standard LWE bootstrap

docstring not up to date

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 27 at r4 (raw file):

>(
    input_lwe_secret_key: &LweSecretKey<InputKeyCont>,
    output_glwe_secret_key: &GlweSecretKey<OutputKeyCont>,

we don't need several keys in the cm case ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 90 at r4 (raw file):

>(
    input_lwe_secret_key: &LweSecretKey<InputKeyCont>,
    output_glwe_secret_key: &GlweSecretKey<OutputKeyCont>,

we need several keys here normally ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 207 at r4 (raw file):

    Gen,
>(
    input_lwe_secret_key: &LweSecretKey<InputKeyCont>,

several keys ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 244 at r4 (raw file):

#[derive(Clone, Debug, PartialEq)]
pub struct CmBootstrapKeys<Scalar: UnsignedInteger> {

test only struct ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 251 at r4 (raw file):

}

pub fn generate_cm_pbs_keys<

test only function ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 258 at r4 (raw file):

    secret_random_generator: &mut SecretRandomGenerator<DefaultRandomGenerator>,
) -> CmBootstrapKeys<Scalar> {
    let ciphertext_modulus = CiphertextModulus::<Scalar>::new_native();

use from CmApParams

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 263 at r4 (raw file):

    let glwe_noise_distribution =
        DynamicDistribution::new_gaussian(minimal_glwe_variance_for_132_bits_security_gaussian(

use from CmApParams ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 266 at r4 (raw file):

            params.glwe_dimension,
            params.polynomial_size,
            2_f64.powi(64),

use from CmApParams

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 1 at r4 (raw file):

//! Module containing primitives pertaining to [`LWE ciphertext

CM missing in docstring

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 14 at r4 (raw file):

pub fn compress_lwe_ciphertexts_into_cm<Scalar, KSKCont, InputCont, OutputCont>(
    cm_lwe_compression_key: &CmLweCompressionKey<KSKCont>,
    input_lwe_ciphertexts: &[LweCiphertext<InputCont>],

maybe add a todo to manage generic iterators for later so that a vec of LWEs and an LweCiphertextList can be used here later on

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 24 at r4 (raw file):

    assert_eq!(
        cm_lwe_compression_key.input_key_lwe_dimension(),
        input_lwe_ciphertexts[0].lwe_size().to_lwe_dimension(),

.iter().all(...)

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 64 at r4 (raw file):

    );

    let input_ciphertext_modulus = input_lwe_ciphertexts[0].ciphertext_modulus();

.iter().all(...) check missing ?

shoudl the moduli be compatible with the output or is it irrelevant here ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 79 at r4 (raw file):

        .enumerate()
    {
        output_cm_lwe_ciphertext.get_mut_bodies().into_container()[i] =

as_mut() instead of into_container ?

since you index into it, cannot here be a body iterator ?

.zip_eq(output_cm_lwe_ciphertext.ger_mut_bodies().iter_mut() maybe ?

or .as_mut().iter_mut() ?

or maybe there is an issue while iterating ?

in that case does it make sense to fill the bodies before the loop ?

that's what is done in the basic keyswitch for LWEs maybe that does not apply here ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 80 at r4 (raw file):

    {
        output_cm_lwe_ciphertext.get_mut_bodies().into_container()[i] =
            output_cm_lwe_ciphertext.get_mut_bodies().into_container()[i]

same here ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression_key_generation.rs line 1 at r4 (raw file):

//! Module containing primitives pertaining to [`CRS LWE compression keys

look for CRS/CSR, I think I saw some CSR (typo) which must have been the CM old name

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression_key_generation.rs line 45 at r4 (raw file):

    );

    for output_lwe_sk in output_lwe_sks {

.iter().all() to have a single assert

[mayeul-zama]

@mayeul-zama made 33 comments and resolved 12 discussions.
Reviewable status: 25 of 52 files reviewed, 67 unresolved discussions (waiting on IceTDrinker, soonum, and SouchonTheo).

---

tfhe/src/core_crypto/entities/glwe_ciphertext.rs line 174 at r2 (raw file):

Previously, IceTDrinker wrote…

If you need it you can add a new type for GlweBodyCount, that's ok 👍

It wasn't needed

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_compression.rs line 14 at r4 (raw file):

Previously, IceTDrinker wrote…

seems a bit odd to me to mix this param set ?

I don't understand what you mean by mixing

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_keyswitch.rs line 24 at r4 (raw file):

Previously, IceTDrinker wrote…

can you check the locations where CmApParams is used to update the ciphertext modulus to take it directly from the params ?

Done

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_keyswitch.rs line 29 at r4 (raw file):

Previously, IceTDrinker wrote…

use the one from params

Done

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_keyswitch.rs line 35 at r4 (raw file):

Previously, IceTDrinker wrote…

is it not part of the parameters ? 🧐

Fixed

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 27 at r4 (raw file):

Previously, IceTDrinker wrote…

same for all tests: use modulus from params

Done

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 29 at r4 (raw file):

Previously, IceTDrinker wrote…

use from params

Done

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 115 at r4 (raw file):

Previously, IceTDrinker wrote…

flatten().collect() maybe ?

https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.flatten

The idea is to analyse noise in each slot independently
This line just does a transpose
Added a comment

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 130 at r4 (raw file):

Previously, IceTDrinker wrote…

type annotation needed ? 🧐

Removed

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 141 at r4 (raw file):

Previously, IceTDrinker wrote…

let's have a todo to manage confidence interval here

Added

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 145 at r4 (raw file):

Previously, IceTDrinker wrote…

do we have a way to maybe auto generate this formula ?

why is it called impl_2 btw ?

Not sure, I don't remember where it comes from
Removed the _2 suffix

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 184 at r4 (raw file):

Previously, IceTDrinker wrote…

remark for params use in tests, modulus, cm_dimension, noise distribution etc.

Done

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 253 at r4 (raw file):

Previously, IceTDrinker wrote…

floor instead of round to avoid going overboard ?

floor could give the wrong value
Added a an assert that the round is a very small correction.

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 255 at r4 (raw file):

Previously, IceTDrinker wrote…

could you maybe have a method on the params to return the proper count or do the transformation when you instantiate the parameters ?

Done for this one
But powf is not const so can't be used there

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 293 at r4 (raw file):

Previously, IceTDrinker wrote…

if it's a linear algebra clear mul, could we have something similar to what we have in the lwe_linear_algebra module ?

Done

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_glwe_sample_extraction.rs line 3 at r4 (raw file):

Previously, IceTDrinker wrote…

doc string is inaccurate, for cm several LWEs can get extracted right ?

also LWE and GLWE here would be CM ... ?

You can extract a single cm lwe ciphertext from a cm glwe ciphertext multiple times

Renamed CommonMask

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 27 at r4 (raw file):

Previously, IceTDrinker wrote…

we don't need several keys in the cm case ?

Removed unsed function

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 90 at r4 (raw file):

Previously, IceTDrinker wrote…

we need several keys here normally ?

Removed unsed function

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 207 at r4 (raw file):

Previously, IceTDrinker wrote…

several keys ?

Removed unsed function

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 244 at r4 (raw file):

Previously, IceTDrinker wrote…

test only struct ?

Not necessarly
I think it's fine here as it is experimental

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 251 at r4 (raw file):

Previously, IceTDrinker wrote…

test only function ?

Not necessarly
I think it's fine here as it is experimental

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 258 at r4 (raw file):

Previously, IceTDrinker wrote…

use from CmApParams

Done

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 263 at r4 (raw file):

Previously, IceTDrinker wrote…

use from CmApParams ?

Done

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 1 at r4 (raw file):

Previously, IceTDrinker wrote…

CM missing in docstring

Fixed

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 14 at r4 (raw file):

Previously, IceTDrinker wrote…

maybe add a todo to manage generic iterators for later so that a vec of LWEs and an LweCiphertextList can be used here later on

Done

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 24 at r4 (raw file):

Previously, IceTDrinker wrote…

.iter().all(...)

Added a loop

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 64 at r4 (raw file):

Previously, IceTDrinker wrote…

.iter().all(...) check missing ?

shoudl the moduli be compatible with the output or is it irrelevant here ?

Added

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 79 at r4 (raw file):

Previously, IceTDrinker wrote…

as_mut() instead of into_container ?

since you index into it, cannot here be a body iterator ?

.zip_eq(output_cm_lwe_ciphertext.ger_mut_bodies().iter_mut() maybe ?

or .as_mut().iter_mut() ?

or maybe there is an issue while iterating ?

in that case does it make sense to fill the bodies before the loop ?

that's what is done in the basic keyswitch for LWEs maybe that does not apply here ?

Now uses as_mut

We modify the whole cm_lwe later in the loop so we can't keep a &mut on the body

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression_key_generation.rs line 1 at r4 (raw file):

Previously, IceTDrinker wrote…

look for CRS/CSR, I think I saw some CSR (typo) which must have been the CM old name

Fixed

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression_key_generation.rs line 45 at r4 (raw file):

Previously, IceTDrinker wrote…

.iter().all() to have a single assert

That would make the assert not print the unequal values, which can be practical when debugging

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_glwe_ciphertext.rs line 76 at r4 (raw file):

Previously, IceTDrinker wrote…

cm_glwe_ciphertext_size ?

Done
Added a function for fourier size too

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_ciphertext.rs line 133 at r3 (raw file):

Previously, IceTDrinker wrote…

parameters can be removed ? I believe they are only related to the conformance ? or am I mistaken ?

Done

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_lwe_ciphertext_list.rs line 147 at r4 (raw file):

Previously, IceTDrinker wrote…

so: sorry about that but let's not keep it as we discussed, the create_from can be called in a loop, having additional checks may not allow to generate streamlined iter code

same for other create_from

As discussed, this is the current pattern in the lib

[IceTDrinker]

I think there is some confusion (or I'm missing something) around the coeff counts for noise/mask sampling for CM ggsw

@IceTDrinker partially reviewed 24 files and all commit messages, made 26 comments, and resolved 22 discussions.
Reviewable status: 49 of 52 files reviewed, 64 unresolved discussions (waiting on mayeul-zama, soonum, and SouchonTheo).

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_compression.rs line 14 at r4 (raw file):

Previously, mayeul-zama wrote…

I don't understand what you mean by mixing

is this test parameter a CM parameter set ? looks like it's a classic param ? I'm not sure

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 253 at r4 (raw file):

Previously, mayeul-zama wrote…

floor could give the wrong value
Added a an assert that the round is a very small correction.

I disagree, if you compute a cleartext mul, taking the ceil will be slightly wrong, or somewhat very wrong

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 255 at r4 (raw file):

Previously, mayeul-zama wrote…

Done for this one
But powf is not const so can't be used there

I see you update the way the params are stored, not sure I follow what you mean about powf ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_bootstrap_key_generation.rs line 207 at r4 (raw file):

Previously, mayeul-zama wrote…

Removed unsed function

I think you missed this one ? I still see it and it takes a single key

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_compression.rs line 14 at r4 (raw file):

Previously, mayeul-zama wrote…

Done

actually my comment mentionned managing iterators if possible, here the TODO does not reflect that, taking LWE slices is not a problem per se, it just wasn't the pattern we used back then to have contiguous memory everywhere it was possible, but it's not always user friendly

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_params.rs line 32 at r4 (raw file):

Previously, IceTDrinker wrote…

leftover spaces, I saw the rustfmt::skip above

could you maybe dump the parameters once converted so that we can get rid of the raw params ?

about getting rid of raw params, wdyt ?

---

tfhe/src/core_crypto/experimental/algorithms/test/common_mask/cm_lwe_programmable_bootstrapping.rs line 145 at r5 (raw file):

}

pub fn pbs_variance_132_bits_security_gaussian_impl(

this is CM specific right ?

could be worth having it in the name ?

otherwise we should have an implem in core_crypto::commons::noise_formulas normally

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_fourier_ggsw_ciphertext.rs line 189 at r5 (raw file):

        self.glwe_dimension
    }

missing cm_dimension accessor I think ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_fourier_ggsw_ciphertext.rs line 235 at r5 (raw file):

        self.glwe_dimension
    }

missing cm_dimension accessor I think ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_fourier_ggsw_ciphertext.rs line 377 at r5 (raw file):

        self.glwe_dimension
    }

missing cm_dimension accessor I think ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_keyswitch_key_generation.rs line 1 at r5 (raw file):

//! Module containing primitives pertaining to [`LWE keyswitch keys

CM LWE ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_keyswitch_key_generation.rs line 3 at r5 (raw file):

//! Module containing primitives pertaining to [`LWE keyswitch keys
//! generation`](`CmLweKeyswitchKey#key-switching-key`) and [`seeded LWE keyswitch keys
//! generation`](`SeededLweKeyswitchKey`).

seeded CM KSK exists ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_keyswitch_key_generation.rs line 42 at r5 (raw file):

    to the input LweSecretKey LweDimension. Destination: {:?}, input: {:?}",
        cm_lwe_keyswitch_key.input_lwe_dimension(),
        input_cm_lwe_sks[0].lwe_dimension()

check all keys

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_keyswitch_key_generation.rs line 49 at r5 (raw file):

    to the output LweSecretKey LweDimension. Destination: {:?}, output: {:?}",
        cm_lwe_keyswitch_key.output_lwe_dimension(),
        output_cm_lwe_sks[0].lwe_dimension()

same here

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_modulus_switch_noise_reduction.rs line 12 at r5 (raw file):

use itertools::{izip, Itertools};

pub(crate) fn round<Scalar: UnsignedInteger>(

check existing function from the original LWE module, a few functions in this module look duplicated

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_modulus_switch_noise_reduction.rs line 185 at r5 (raw file):

}

pub fn improve_lwe_ciphertext_modulus_switch_noise_for_binary_key_cm<Scalar, C1, C2>(

from afar feels like all functions can be removed, except the one taking a CM LWE ?

and even then does the original algo need an LWE or would an LWE mask work ?

no need to change the algo in this PR if it only needs a mask and not a full LWE

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_programmable_bootstrapping/cm_fft64.rs line 1 at r5 (raw file):

//! Module containing primitives pertaining to the [`LWE programmable

cm

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_programmable_bootstrapping/cm_fft64.rs line 2 at r5 (raw file):

//! Module containing primitives pertaining to the [`LWE programmable
//! bootstrap`](`crate::core_crypto::entities::LweBootstrapKey#programmable-bootstrapping`) using 64

not sure if that reference makes sense here

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_programmable_bootstrapping/cm_fft64.rs line 97 at r5 (raw file):

    // modulus is not the native one

    cm_blind_rotate_assign_raw(

what does raw mean ?

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_programmable_bootstrapping/cm_fft64.rs line 112 at r5 (raw file):

    fft: FftView<'_>,
) -> StackReq {
    cm_blind_rotate_assign_scratch::<OutputScalar>(

could replace call here and remove the scratch function ? not sure how things are structured

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_programmable_bootstrapping/cm_fft64.rs line 266 at r5 (raw file):

    }
}

do you need all the primitives from that file ?

also given some primitives don't take a CmDimension as input like the on here, so unclear to me they do anything useful for the CM

can you make a pass on this file and verify which functions are actually useful ?

it's ok to keep the ext_prod, but the cmux may not be super useful

also all flavors of algorithms are not necessarily required, keep the ones that are useful, like the mem_optimized pbs

---

tfhe/src/core_crypto/experimental/algorithms/common_mask_algorithms/cm_lwe_programmable_bootstrapping/cm_fft64.rs line 396 at r5 (raw file):

    fft: FftView<'_>,
) -> StackReq {
    cm_bootstrap_scratch::<OutputScalar>(glwe_dimension, cm_dimension, polynomial_size, fft)

remove scratch here and put implem directly ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_ggsw_ciphertext.rs line 120 at r5 (raw file):

) -> EncryptionNoiseSampleCount {
    (glwe_dimension.0 + cm_dimension.0)
        * glwe_ciphertext_encryption_noise_sample_count(polynomial_size)

that does not seem to match the cm_ggsw_ciphertext size ?

doesn't the CM GLWE have several bodies ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_ggsw_ciphertext.rs line 209 at r5 (raw file):

        let matrix_size = (glwe_dimension.0 + cm_dimension.0)
            * cm_glwe_ciphertext_size(glwe_dimension, cm_dimension, polynomial_size);

think you have a function for this ?

---

tfhe/src/core_crypto/experimental/entities/common_mask_entities/cm_ggsw_ciphertext_list.rs line 110 at r5 (raw file):

        ciphertext_modulus: CiphertextModulus<C::Element>,
    ) -> Self {
        let ggsw_size = decomp_level_count.0

do you have a function for this ?

I think there is something I'm not sure of

it looks like here the GGSW has level * (glwe_dim + cm_dim) * size(cm_glwe)

while in the ggsw_ciphertext file something about the number of noise samples seems to only consider one glwe per row