Conversion between TFHErs fheint and Concrete ciphertexts/keys #945

youben11 · 2024-07-18T10:00:11Z

No description provided.

github-actions

Benchmark

Benchmark suite	Current: `278def1`	Previous: `0060f01`	Ratio
`v0 PBS table generation`	`60839502` ns/iter (`± 1588090`)	`60865468` ns/iter (`± 3212038`)	`1.00`
`v0 PBS simulate dag table generation`	`38096598` ns/iter (`± 393959`)	`37760545` ns/iter (`± 342103`)	`1.01`
`v0 WoP-PBS table generation`	`50395693` ns/iter (`± 1820335`)	`50040680` ns/iter (`± 4218342`)	`1.01`

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

aPere3

👍

backends/concrete-cpu/implementation/src/c_api/fheint.rs

aPere3 · 2024-07-19T07:18:18Z

compilers/concrete-compiler/compiler/include/concretelang/ClientLib/ClientLib.h

+Result<TransportValue>
+importTfhersFheUint8(llvm::ArrayRef<uint8_t> serializedFheUint8);
+Result<std::vector<uint8_t>> exportTfhersFheUint8(TransportValue value,
+                                                  TfhersFheIntDescription info);
+Result<TfhersFheIntDescription>
+getTfhersFheUint8Description(llvm::ArrayRef<uint8_t> serializedFheUint8);
+


I think this would be better put into Commons. If ever we need to use that in the ServerLib as well.

compilers/concrete-compiler/compiler/include/concretelang/Common/Values.h

BourgerieQuentin

First review (test not yet reviewed)

backends/concrete-cpu/implementation/src/c_api/fheint.rs

BourgerieQuentin · 2024-08-28T15:31:26Z

backends/concrete-cpu/implementation/src/c_api/fheint.rs

+    serialized_data_len: usize,
+) -> TfhersFheIntDescription {
+    nounwind(|| {
+        // deserialize fheuint8


TODO: We should see if we can have a generic code

@IceTDrinker any thoughts on this? In short: can we have a generic function which serialize/deserialize FheUint/Int of different widths?

make it generic over the FheUintId and FheIntId traits

FheUint and FheInt are built like so:

FheUint<Id: FheUintId>
FheInt<Id: FheIntId>

the FheUint8 is just an instantiation of the type with a dedicated ID for 8 bits

BourgerieQuentin · 2024-08-29T08:23:16Z

backends/concrete-cpu/implementation/src/c_api/fheint.rs

+}
+
+#[no_mangle]
+pub unsafe extern "C" fn concrete_cpu_tfhers_uint8_to_lwe_array(


Could we merge both function to deserialize just once?

I understand you mean with the get description function. This could be done, but the two functions are actually unrelated. With recent changes, the get description function is no longer necessary, but it could be used to get some information for debugging for example. So I don't think we should merge them.

You can see how it's used in my following push

Ok but should be used at least to verify that the imported ciphertext parameters are equals to ones given by the developer, wdyt?

backends/concrete-cpu/implementation/src/c_api/fheint.rs

compilers/concrete-compiler/compiler/lib/ClientLib/ClientLib.cpp

frontends/concrete-python/concrete/fhe/tfhers/context.py

BourgerieQuentin · 2024-08-29T09:48:28Z

frontends/concrete-python/concrete/fhe/tfhers/context.py

+        # pylint: enable=protected-access
+
+
+def new_context(


Why this wrapper around the constructor?

a matter of taste 😛

Is that python idomatic?

frontends/concrete-python/concrete/fhe/tfhers/context.py

frontends/concrete-python/tests/dtypes/test_tfhers.py

aPere3 · 2024-08-30T07:55:12Z

backends/concrete-cpu/implementation/src/c_api/fheint.rs

+            serialized_data_ptr,
+            serialized_data_len,
+        ));
+        let fheuint: FheUint8 = match bincode::deserialize_from(&mut serialized_data) {


Is it possible to make sure that the bincode version we use here is compatible with the version used on tfhe-rs side ? Maybe I am wrong, but if a new version of bincode changes the binary format, it may break the serialization doesn't it ? Could it be a security issue ?

Maybe I am way over my head here so feel free to ignore 😆

to be future proof and benefit from the data compatibility you would want to use the versionize primitives before serializing, then you are free to use whichever serde-compatible serialization backend

see e.g. here https://github.com/zama-ai/tfhe-rs/tree/main/utils/tfhe-versionable Nicolas is the one who worked on the design of this if support is needed

I don't think it's TFHErs vs Concrete versions. It's the user of TFHErs which will be doing serialization, so he should make sure he uses the same version as in Concrete. I don't think TFHErs has something to do in here. For example, you can see the tool I'm using for testing (tfhers-utils). It's using bincode for serialization and doesn't rely on TFHErs for that. Unless serialization of TFHErs struct would rely on something implemeneted in TFHErs that I'm not aware of.

@IceTDrinker I was not worried by the versionning of TFHE-rs structures, but more in the compatibility of the serialization format implemented by bincode.

@youben11 you are right, but Concrete is delivered as a binary (with a fixed version for bincode), but we don't know what users will do in their rust code, nor if they will be aware that the version of bincode is important.

@IceTDrinker Not sure ! I did not go into many details but it seems they changed their spec since last major release (https://github.com/bincode-org/bincode/commits/trunk/docs/spec.md).

@youben11 what do you mean by follow the framework ? The basic user (me) won't look at the repo tree at the commit of the concrete binary to extract the bincode version. He will just do Cargo.toml bincode="2" and call it a day 🤣

I any case, I am not overly worried about that 😉 Though it may be interesting to think about adding bincode version in the serialization if they are not reliable.

I mean we should document that. Otherwise, we should provide ser/deser functions maybe? I'm no expert in Rust serialization, but I would always think how to serialize something before sending it to another package for deserialization. Or is bincode what everyone uses? Think of when we will be using versionize: we will have to document how someone is expected to serialize a ciphertext before using it in Concrete.

Actually ciphertexts will come from tfhe-rs users (blockchain as example). So we should use the same serialization framework than casual tfhe-rs user will use, if I well understand the versionable framework is something generic, but is that will be used for tfhe-rs integers @IceTDrinker ? From my understanding you should implement versioning on tfhe-rs side for thf-rs integers and we should use that.

we already have versioning for ciphertexts, Nicolas will be back on thursday so if Ayoub needs a quick tour it can likely be done then

Ok great 👍

aPere3 · 2024-08-30T07:58:19Z

backends/concrete-cpu/implementation/src/c_api/fheint.rs

+        }
+        let fheuint = match FheUint8::from_expanded_blocks(
+            blocks,
+            tfhe::integer::ciphertext::DataKind::Unsigned(4),


Does it mean we only support 4bits ?

looks like the number of bits of the DataKind is not really used to check anything in the function afterwards 🤔 only the fact that it's unsigned matters

I would recommend matching the numebr of bits in the Unsigned to the type being reconstructed so here 8 bits

Good catch! Looking into this, and why it was working for other values, I found that in reality, the 4 was never used underneath as it was recomputed from blocks. I made a fix by computing the datakind based sign and width

(just saw Arthur's comment as it wasn't appearing initially)

frontends/concrete-python/concrete/fhe/tfhers/context.py

this was previously hardcoded

This is useful for it to be used in TFHErs

this is done to have the same format as tfhers

this will be used to test Concrete <> TFHErs compatibility

Concrete doing the keygen, while TFHErs doing enc/dec

we were doing a deserialization previously to get the fheint description, but we will now construct it from the type instead. It's still possible for the user to get the description from the buffer and use it for import (using the Compiler API).

youben11 · 2024-09-03T10:44:26Z

@slab-ci concrete-python-tests-linux

cla-bot bot added the cla-signed label Jul 18, 2024

youben11 requested a review from aPere3 July 18, 2024 10:00

github-actions bot reviewed Jul 18, 2024

View reviewed changes

aPere3 reviewed Jul 19, 2024

View reviewed changes

youben11 force-pushed the feat/import-export-fheuint branch from fe987ad to 11868ad Compare July 24, 2024 11:04

youben11 force-pushed the feat/import-export-fheuint branch from 11868ad to 61d7008 Compare August 28, 2024 11:25

youben11 changed the title ~~Conversion between TFHErs fheint and Concrete ciphertexts~~ Conversion between TFHErs fheint and Concrete ciphertexts/keys Aug 28, 2024

youben11 force-pushed the feat/import-export-fheuint branch 2 times, most recently from 651d11b to 6bc67fe Compare August 28, 2024 14:39

BourgerieQuentin requested changes Aug 29, 2024

View reviewed changes

aPere3 reviewed Aug 30, 2024

View reviewed changes

youben11 requested a review from BourgerieQuentin September 2, 2024 11:34

BourgerieQuentin approved these changes Sep 3, 2024

View reviewed changes

youben11 added 16 commits September 3, 2024 10:38

fix(compiler): use initializer constructor

45c1386

feat(backend, compiler): convert TFHErs to Concrete ciphertext and back

4838d56

feat(compiler): get variance and keyid for a given input

f2368ca

feat(compiler): provide keyid and variance while importing

c783f2a

this was previously hardcoded

chore(backend): build if src/* has been changed

760e20c

feat(compiler/frontend): serialize a given secret key

cb22c48

This is useful for it to be used in TFHErs

fix(frontend): make tfhers ciphertext lsb first

355a130

this is done to have the same format as tfhers

refactor(frontend): finalize tfhers parameters API

2dec986

feat(frontend): TFHErs Context

c7592e8

test(frontend): add utility to enc/dec TFHErs encrypted integers

4738dd8

this will be used to test Concrete <> TFHErs compatibility

test(frontend): add e2e tfhers compatibility tests (concrete keygen)

07c27f9

Concrete doing the keygen, while TFHErs doing enc/dec

feat(frontend/compiler): set correct value for noise and degree

6f3499a

refactor(compiler): compute fheuint buffer size in rust (more accurate)

05b502a

fix(compiler): remove hardcoded datakind and construct it from desc

704562a

refactor(frontend): rename tfhers.Context to tfhers.Bridge

278def1

youben11 force-pushed the feat/import-export-fheuint branch from 81767d8 to 278def1 Compare September 3, 2024 09:38

youben11 merged commit a951789 into main Sep 3, 2024
31 of 44 checks passed

youben11 deleted the feat/import-export-fheuint branch September 3, 2024 13:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conversion between TFHErs fheint and Concrete ciphertexts/keys #945

Conversion between TFHErs fheint and Concrete ciphertexts/keys #945

youben11 commented Jul 18, 2024

github-actions bot left a comment •

edited

Loading

aPere3 left a comment

aPere3 Jul 19, 2024

BourgerieQuentin left a comment

BourgerieQuentin Aug 28, 2024

youben11 Aug 30, 2024

IceTDrinker Aug 30, 2024

IceTDrinker Aug 30, 2024 •

edited

Loading

BourgerieQuentin Aug 29, 2024

youben11 Aug 29, 2024

youben11 Aug 30, 2024

BourgerieQuentin Sep 3, 2024

BourgerieQuentin Aug 29, 2024

youben11 Aug 30, 2024

BourgerieQuentin Sep 3, 2024

aPere3 Aug 30, 2024

IceTDrinker Aug 30, 2024

IceTDrinker Aug 30, 2024

youben11 Aug 30, 2024

aPere3 Aug 30, 2024

aPere3 Aug 30, 2024

youben11 Aug 30, 2024

BourgerieQuentin Sep 3, 2024

IceTDrinker Sep 3, 2024

BourgerieQuentin Sep 3, 2024

aPere3 Aug 30, 2024

IceTDrinker Aug 30, 2024

IceTDrinker Aug 30, 2024

youben11 Aug 30, 2024

youben11 Aug 30, 2024

youben11 commented Sep 3, 2024

Conversion between TFHErs fheint and Concrete ciphertexts/keys #945

Conversion between TFHErs fheint and Concrete ciphertexts/keys #945

Conversation

youben11 commented Jul 18, 2024

github-actions bot left a comment • edited Loading

Choose a reason for hiding this comment

Benchmark

aPere3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BourgerieQuentin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IceTDrinker Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youben11 commented Sep 3, 2024

github-actions bot left a comment •

edited

Loading

IceTDrinker Aug 30, 2024 •

edited

Loading