Update TFHE-rs to 0.10.0 and use safe serialization for secret keys

[youben11]

[github-actions]

Benchmark

Benchmark suite	Current: ccf491e	Previous: 4d6afba	Ratio
v0 PBS table generation	58836953 ns/iter (± 891893)	61206032 ns/iter (± 915254)	0.96
v0 PBS simulate dag table generation	37239574 ns/iter (± 338641)	39067802 ns/iter (± 517224)	0.95
v0 WoP-PBS table generation	52402617 ns/iter (± 291817)	53923124 ns/iter (± 503951)	0.97

This comment was automatically generated by workflow using github-action-benchmark.

[BourgerieQuentin]

@IceTDrinker If I well understand that should break GPU code as levels has been reversed in bsk right?

[BourgerieQuentin]

btw gpu test seems not launched... 🤔

[BourgerieQuentin]

Else LGTM

[IceTDrinker]

@IceTDrinker If I well understand that should break GPU code as levels has been reversed in bsk right?

if you are using code that is not the cuda backend then yes, also to note that it breaks GPU PBS levels in your case (going from 0.8 to 0.10 the KS levels are preserved) and the order in which the circuirt bootstrap does the pfpks for the wopbs

[IceTDrinker]

which primitives do you have in your GPU code ?

[BourgerieQuentin]

ksk/bsk conversion, ks/bs, levelled op (with the old cuda backend, i.e. if fixed in cnversion function we are not up to date)

[IceTDrinker]

no wopbs on GPU ?

[BourgerieQuentin]

nop

[BourgerieQuentin]

@youben11 check python tests (cannot serialize sk)

[IceTDrinker]

so KSK and packing KSK levels are the same in 0.8 and 0.10, bsk are reversed in 0.10 vs 0.8

[youben11]

@youben11 check python tests (cannot serialize sk)

weird this only happened after I rebased on main! Initial run had only one flaky test https://github.com/zama-ai/concrete/actions/runs/11911805689/job/33194835318

[youben11]

Compiler GPU tests passed https://github.com/zama-ai/concrete/actions/runs/11930839707/job/33252527434

[IceTDrinker]

Compiler GPU tests passed https://github.com/zama-ai/concrete/actions/runs/11930839707/job/33252527434

Hmm ok

[IceTDrinker]

do the GPU tests have pbs_level_count > 1 ?

[youben11]

do the GPU tests have pbs_level_count > 1 ?

@aPere3 do you have an idea?

[BourgerieQuentin]

Don't known for the GPU test but we could have level > 1 there are no restriction on the search space

[IceTDrinker]

Don't known for the GPU test but we could have level > 1 there are no restriction on the search space

just that the level reversal would not show up on a single level BSK, but if you are confident it's all good, I won't stop you 🙂

[BourgerieQuentin]

Ok so I'm not confident :)

[IceTDrinker]

what happened with serialization ?

[youben11]

The old computation of buffer size no longer worked. It's due to additional headers now included in the safe serialisation. The fix was to add some additional (approximate) room for the headers.

[IceTDrinker]

The old computation of buffer size no longer worked. It's due to additional headers now included in the safe serialisation. The fix was to add some additional (approximate) room for the headers.

we don't account for those headers ? @nsarlin-zama

[youben11]

No. It's not your computation, but our own. We can't easily use yours as we don't yet hold the value at the point of creating the buffer.

Our process is like:

• I have a Cpp key I want to serialze
• I create a Cpp buffer to hold it (need the size here)
• I serialize into the buffer

AFAICT, if we need to call your function, then we need to copy the key, which is not quite efficient. So since it's not a cheap operation then having a homemade estimation (which is not 100% accurate) sound better

[IceTDrinker]

No. It's not your computation, but our own. We can't easily use yours as we don't yet hold the value at the point of creating the buffer.

Our process is like:

• I have a Cpp key I want to serialze
• I create a Cpp buffer to hold it (need the size here)
• I serialize into the buffer

AFAICT, if we need to call your function, then we need to copy the key, which is not quite efficient. So since it's not a cheap operation then having a homemade estimation (which is not 100% accurate) sound better

You have the key at step one, so call the size right after ? I might be missing something

[youben11]

I can call it, but It would require a copy as

    let lwe_sk: LweSecretKey<&[u64]> = LweSecretKey::from_container(slice::from_raw_parts(
        lwe_sk,
        concrete_cpu_lwe_secret_key_size_u64(lwe_dimension),
    ));
    tfhe::safe_serialization::safe_serialized_size(&lwe_sk)

wouldn't work. It would require a LweSecretKey<Vec>, which then require a copy, and computing on our side avoid the copy.

[IceTDrinker]

I can call it, but It would require a copy as

    let lwe_sk: LweSecretKey<&[u64]> = LweSecretKey::from_container(slice::from_raw_parts(
        lwe_sk,
        concrete_cpu_lwe_secret_key_size_u64(lwe_dimension),
    ));
    tfhe::safe_serialization::safe_serialized_size(&lwe_sk)

wouldn't work. It would require a LweSecretKey, which then require a copy, and computing on our side avoid the copy.

hmm... no ?

use a view ?

LweSecretKey<&[u64]> ?

Edit: sorry did not see the code, but it should work there is no copy there

[IceTDrinker]

Why does calling the function on a view not work ? If it does not it's a bug on our end

[youben11]

I will get this if I try to do it

error[E0277]: the trait bound `&[u64]: Unversionize` is not satisfied
   --> src/c_api/secret_key.rs:226:60
    |
226 |     let a = tfhe::safe_serialization::safe_serialized_size(&lwe_sk);
    |             ---------------------------------------------- ^^^^^^^ the trait `Unversionize` is not implemented for `&[u64]`, which is required by `tfhe::core_crypto::entities::LweSecretKey<&[u64]>: Versionize`
    |             |
    |             required by a bound introduced by this call
    |
    = help: the trait `Unversionize` is implemented for `[T; N]`
    = note: required for `tfhe::core_crypto::entities::LweSecretKey<&[u64]>` to implement `tfhe_versionable::derived_traits::Version`
    = note: required for `LweSecretKeyVersions<&[u64]>` to implement `tfhe_versionable::derived_traits::VersionsDispatch<tfhe::core_crypto::entities::LweSecretKey<&[u64]>>`
    = note: required for `tfhe::core_crypto::entities::LweSecretKey<&[u64]>` to implement `Versionize`
note: required by a bound in `safe_serialized_size`
   --> /home/ayoub/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tfhe-0.10.0/src/safe_serialization.rs:466:44
    |
466 | pub fn safe_serialized_size<T: Serialize + Versionize + Named>(object: &T) -> bincode::Result<u64> {
    |                                            ^^^^^^^^^^ required by this bound in `safe_serialized_size`

[IceTDrinker]

I will get this if I try to do it

error[E0277]: the trait bound `&[u64]: Unversionize` is not satisfied
   --> src/c_api/secret_key.rs:226:60
    |
226 |     let a = tfhe::safe_serialization::safe_serialized_size(&lwe_sk);
    |             ---------------------------------------------- ^^^^^^^ the trait `Unversionize` is not implemented for `&[u64]`, which is required by `tfhe::core_crypto::entities::LweSecretKey<&[u64]>: Versionize`
    |             |
    |             required by a bound introduced by this call
    |
    = help: the trait `Unversionize` is implemented for `[T; N]`
    = note: required for `tfhe::core_crypto::entities::LweSecretKey<&[u64]>` to implement `tfhe_versionable::derived_traits::Version`
    = note: required for `LweSecretKeyVersions<&[u64]>` to implement `tfhe_versionable::derived_traits::VersionsDispatch<tfhe::core_crypto::entities::LweSecretKey<&[u64]>>`
    = note: required for `tfhe::core_crypto::entities::LweSecretKey<&[u64]>` to implement `Versionize`
note: required by a bound in `safe_serialized_size`
   --> /home/ayoub/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tfhe-0.10.0/src/safe_serialization.rs:466:44
    |
466 | pub fn safe_serialized_size<T: Serialize + Versionize + Named>(object: &T) -> bincode::Result<u64> {
    |                                            ^^^^^^^^^^ required by this bound in `safe_serialized_size`

ah ha!

@nsarlin-zama this should be an easy fix I would think ?

[nsarlin-zama]

@nsarlin-zama this should be an easy fix I would think ?

This is not so easy, it would require to rework a bit the proc macro as currently the impl of Versionize and Unversionize are linked together...