Working on a tutorial on TFHE-rs <> Concrete ML interoperability: step 1 #1080

bcm-at-zama · 2024-10-02T07:49:10Z

Current state: I have something which does the job for Concrete.

I think it would be nice to review it and maybe merge (as a WIP). In particular, I have seen a few problems maybe (https://github.com/zama-ai/concrete-internal/issues/874) and few things I'd like to clarify (see the FIXME's). Maybe we can hide more to the users, for things which are not changeable by them.

frontends/concrete-python/examples/tfhers-cml/Cargo.toml

bcm-at-zama · 2024-10-02T07:54:12Z

frontends/concrete-python/examples/tfhers-cml/fix_omp_issues.sh

@@ -0,0 +1,57 @@
+#!/usr/bin/env bash
+


for mac to fix the OMP issues, I'll remove at the end

frontends/concrete-python/examples/tfhers-cml/src/main.rs

bcm-at-zama · 2024-10-02T07:57:48Z

frontends/concrete-python/examples/tfhers-cml/test.py

+# Params: users shouldn't change them
+LWE_DIM = 909
+GLWE_DIM = 1
+POLY_SIZE = 4096
+PBS_BASE_LOG = 15
+PBS_LEVEL = 2
+MSG_WIDTH = 2
+CARRY_WIDTH = 3
+ENCRYPTION_KEY_CHOICE = tfhers.EncryptionKeyChoice.BIG
+LWE_NOISE_DISTR = 0
+GLWE_NOISE_DISTR = 2.168404344971009e-19
+
+assert GLWE_DIM == 1, "glwe dim must be 1"


should we hide this and make it constant to users, if they shouldn't change it?

These aren't constant. You will basically choose TFHE-rs parameters (using whatever method: I don't know what's the strategy there for choosing parameters), then you copy/paste those here. What would be better in the future is to have a TFHE-rs function to export those parameters (as json), then import them in Concrete. This way we will avoid seeing them all around the code.

But our users will use constant TFHE-rs parameters, or not? Is it expected that TFHE-rs users change parameters?

Yes, it's is expected.

Hum, I'd like to discuss that part, I keep it for the weekly. Because, I mean, we have a lot of users who are far from being powerusers, they just want the simplest things.

bcm-at-zama · 2024-10-02T07:58:05Z

frontends/concrete-python/examples/tfhers-cml/test.py

+# Options
+# FIXME: explain
+FHEUINT_PRECISION = 8


to explain what can be modified here

bcm-at-zama · 2024-10-02T07:58:26Z

frontends/concrete-python/examples/tfhers-cml/test.py

+tfhers_params = tfhers.CryptoParams(
+    lwe_dimension=LWE_DIM,
+    glwe_dimension=GLWE_DIM,
+    polynomial_size=POLY_SIZE,
+    pbs_base_log=PBS_BASE_LOG,
+    pbs_level=PBS_LEVEL,
+    lwe_noise_distribution=LWE_NOISE_DISTR,
+    glwe_noise_distribution=GLWE_NOISE_DISTR,
+    encryption_key_choice=ENCRYPTION_KEY_CHOICE,
+)
+tfhers_type = tfhers.TFHERSIntegerType(
+    is_signed=False,
+    bit_width=FHEUINT_PRECISION,
+    carry_width=CARRY_WIDTH,
+    msg_width=MSG_WIDTH,
+    params=tfhers_params,
+)
+tfhers_int = partial(tfhers.TFHERSInteger, tfhers_type)


to hide to the user if they shouldn't change it?

bcm-at-zama · 2024-10-02T08:38:07Z

frontends/concrete-python/examples/tfhers-cml/test.py

+
+from numpy.random import randint
+
+# FIXME: should we move this to Concrete library directly, hidden to the user


hide to the user if it's constant?

https://github.com/zama-ai/concrete-internal/issues/884

bcm-at-zama · 2024-10-02T08:38:18Z

frontends/concrete-python/examples/tfhers-cml/test.py

+
+    return tfhers_params, tfhers_type, tfhers_int
+
+# FIXME Params: users shouldn't change them, should we hide it


hide to the user if it's constant?

https://github.com/zama-ai/concrete-internal/issues/884

bcm-at-zama · 2024-10-02T08:38:47Z

frontends/concrete-python/examples/tfhers-cml/test.py

+assert GLWE_DIM == 1, "glwe dim must be 1"
+
+# Options: the user can change the following
+# FIXME: explain FHEUINT_PRECISION, ie can it be changed


I'll ask about this, it's not clear to me if FHEUINT_PRECISION can change or not

This value can change yes, but we mainly support 8 bits for now

bcm-at-zama · 2024-10-02T08:39:22Z

frontends/concrete-python/examples/tfhers-cml/test.py

+
+assert GLWE_DIM == 1, "glwe dim must be 1"
+
+# Options: the user can change the following


I've set a single place where users have to apply their changes (as long as they have just 2 inputs in their functions)

https://github.com/zama-ai/concrete-internal/issues/884

bcm-at-zama · 2024-10-02T08:39:39Z

frontends/concrete-python/examples/tfhers-cml/test.py

+    return (concrete_x + concrete_y) % 47
+
+# The user must specify the range of the TFHE-rs inputs
+# FIXME: why can't we set the limit at 256? It's needed for FHEUint8


There is an issue here if I try to pick a random 8b byte (not 7b), I'll ask for help
CC @youben11

what's the issue?

I got this: https://github.com/zama-ai/concrete-internal/issues/877

bcm-at-zama · 2024-10-02T08:42:41Z

frontends/concrete-python/examples/tfhers-cml/test.py

+    input_idx_to_key = {0: buff, 1: buff}
+    tfhers_bridge.keygen_with_initial_keys(input_idx_to_key_buffer=input_idx_to_key)
+
+    # FIXME: remove the secret key before saving. The secret key can be used for


I think there is an issue here, we shouldn't save secret key, it's dangerous if it goes into production, I'll fill an issue for the examples/tfhers file

ie, https://github.com/zama-ai/concrete-internal/issues/874

bcm-at-zama · 2024-10-02T08:47:35Z

frontends/concrete-python/examples/tfhers-cml/test.py

+    # FIXME: how does it decrypt? we are on the server side, we shouldn't have
+    # the secret key. I think it's because the secret key is saved in concrete_keyset_path
+
+    # x = circuit.decrypt(tfhers_uint8_x)


this part shouldn't work, ie shouldn't be able to decrypt, but it does
I did https://github.com/zama-ai/concrete-internal/issues/874

bcm-at-zama · 2024-10-02T16:52:44Z

It's not the final step but I would be interested in having a review of the current state, if possible. CC @BourgerieQuentin @youben11

bcm-at-zama · 2024-10-02T16:53:15Z

Next steps will be about trying to add CML in the loop.

youben11 · 2024-10-03T10:18:34Z

frontends/concrete-python/examples/tfhers-cml/Cargo.toml

+tfhe = { git = "https://github.com/zama-ai/tfhe-rs.git", rev = "cfb9532f6336c7e8fec754d2dbe2e1195b9c0de7", features = ["integer"] }
+
+[target.x86_64-unknown-linux-gnu.dependencies]
+tfhe = { git = "https://github.com/zama-ai/tfhe-rs.git", rev = "cfb9532f6336c7e8fec754d2dbe2e1195b9c0de7", features = ["integer", "x86_64-unix"] }


You can update to 0.8.0 now

youben11 · 2024-10-03T10:26:06Z

frontends/concrete-python/examples/tfhers-cml/test.py

+assert GLWE_DIM == 1, "glwe dim must be 1"
+
+# Options: the user can change the following
+# FIXME: explain FHEUINT_PRECISION, ie can it be changed


This value can change yes, but we mainly support 8 bits for now

bcm-at-zama · 2024-10-07T14:31:02Z

CC @youben11 : I wanted to move to TFHE-rs, but it's on hold for now, since

I use compiled CP, and our last CP is 2.8.1, including nightlies on https://pypi.zama.ai/cpu/concrete-python/index.html
so when I apply

diff --git a/frontends/concrete-python/examples/tfhers-cml/Cargo.toml b/frontends/concrete-python/examples/tfhers-cml/Cargo.toml
index e91a4808..dff3ff4a 100644
--- a/frontends/concrete-python/examples/tfhers-cml/Cargo.toml
+++ b/frontends/concrete-python/examples/tfhers-cml/Cargo.toml
@@ -6,13 +6,13 @@ edition = "2021"
 bincode = "1.3.3"
 rand = "0.8"
 
-tfhe = { git = "https://github.com/zama-ai/tfhe-rs.git", rev = "cfb9532f6336c7e8fec754d2dbe2e1195b9c0de7", features = ["integer"] }
+tfhe = { version = "0.8.0", features = ["integer"] }
 
 [target.x86_64-unknown-linux-gnu.dependencies]
-tfhe = { git = "https://github.com/zama-ai/tfhe-rs.git", rev = "cfb9532f6336c7e8fec754d2dbe2e1195b9c0de7", features = ["integer", "x86_64-unix"] }
+tfhe = { version = "0.8.0", features = ["integer", "x86_64-unix"] }
 
 [target.aarch64-unknown-linux-gnu.dependencies]
-tfhe = { git = "https://github.com/zama-ai/tfhe-rs.git", rev = "cfb9532f6336c7e8fec754d2dbe2e1195b9c0de7", features = ["integer", "aarch64-unix"] }
+tfhe = { version = "0.8.0", features = ["integer", "aarch64-unix"] }
 
 [target.x86_64-pc-windows-gnu.dependencies]
-tfhe = { git = "https://github.com/zama-ai/tfhe-rs.git", rev = "cfb9532f6336c7e8fec754d2dbe2e1195b9c0de7", features = ["integer", "x86_64"] }
\ No newline at end of file
+tfhe = { version = "0.8.0", features = ["integer", "x86_64"] }
diff --git a/frontends/concrete-python/examples/tfhers-cml/src/main.rs b/frontends/concrete-python/examples/tfhers-cml/src/main.rs
index 7433fc07..b74d313b 100644
--- a/frontends/concrete-python/examples/tfhers-cml/src/main.rs
+++ b/frontends/concrete-python/examples/tfhers-cml/src/main.rs
@@ -43,7 +43,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>>{
     let (client_key, _) = generate_keys(config);
 
     // Get lwe_secret_key, to be reused to generate the server key in Concrete format
-    let (integer_ck, _, _) = client_key.clone().into_raw_parts();
+    let (integer_ck, _, _, _) = client_key.clone().into_raw_parts();
     let shortint_ck = integer_ck.into_raw_parts();
     assert!(BLOCK_PARAMS.encryption_key_choice == EncryptionKeyChoice::Big);
     let (glwe_secret_key, _, _) = shortint_ck.into_raw_parts();

I have a crash in the deserialize_fheuint8("server_dir/ciphertext_r.txt");.

bcm-at-zama · 2024-10-07T14:31:19Z

so updating the CP is for later.

bcm-at-zama · 2024-10-09T11:13:42Z

I have a problem, https://github.com/zama-ai/concrete-internal/issues/885

bcm-at-zama · 2024-10-09T15:39:05Z

frontends/concrete-python/examples/tfhers-cml/src/main.rs

+
+const BLOCK_PARAMS: ClassicPBSParameters = tfhe::shortint::prelude::PARAM_MESSAGE_2_CARRY_3_KS_PBS;
+
+fn write_json_to_file<T: Serialize>(value: &T, filename: &str) {


it works!
https://github.com/zama-ai/concrete-internal/issues/884

bcm-at-zama · 2024-10-10T10:27:20Z

Now in the hands of @youben11 . We've said with @BourgerieQuentin and @youben11 that the blockers for continuing this PR were:

the panick, https://github.com/zama-ai/concrete-internal/issues/885
the support for arrays, https://github.com/zama-ai/concrete-internal/issues/875

BourgerieQuentin · 2024-11-27T09:23:30Z

Taking by @youben11 in other PR

bcm-at-zama · 2024-11-27T09:48:05Z

I think it's a bit sad to not have taken what I had done in frontends/concrete-python/examples/tfhers-cml/src/main.rs: having a single .rs with the calls to .py inside was really much clearer to me, and it was a way to have it tested in our CI. This .rs was really done in the way I understood our clients would use the functionality.

I would consider doing something in this spirit, if you close this PR. I am available to discuss it more precisely if you want.

BourgerieQuentin · 2024-11-27T13:43:03Z

@youben11 wdyt?

youben11 · 2024-11-27T14:10:43Z

The PR is doing something different than what we are doing in the Concrete CI. For Concrete examples/tests it was natural to work on an example similar to the first one, instead of changing toolings. The goal was to have something working e2e as quick as possible. When we have something working, it might be worth to have a Rust script for clients (if that's what they want), but I don't see it as necessary for now.

bcm-at-zama · 2024-11-27T14:40:11Z

The PR is doing something different than what we are doing in the Concrete CI. For Concrete examples/tests it was natural to work on an example similar to the first one, instead of changing toolings.

Which tooling did I change?

The goal was to have something working e2e as quick as possible. When we have something working, it might be worth to have a Rust script for clients (if that's what they want), but I don't see it as necessary for now.

To me:

we want the example to be tested in the CI:
- how is it done currently with the https://github.com/zama-ai/concrete/tree/main/frontends/concrete-python/examples/tfhers? since we are supposed to launch some script files, I don't think the full stuff is tested in the CI, or I miss something
- it was done with the .rs of this PR, I think: as soon as you compile and run the .rs, it checks things are working fine
this PR was not meant to replace examples/tfhers, it was meant to be in addition
and this PR was meant to smoothly converge to a .rs which would test TFHE-rs and CML together, where:
- it would be easy for users to change the CML function easily in a single place
- it's what is done currently with C

bcm-at-zama · 2024-11-27T14:40:52Z

But sure, I can let you do as you want, the most important thing is that the compatibility works for customers and that they know how to use it simply.

youben11 · 2024-11-28T07:33:12Z

Which tooling did I change?

We are using the tfhers_utils for tests. It's our Rust utility for testing compatibility. It's a single place to manage TFHE-rs versions etc. It would be harder to manage two Rust codebase which does the same thing.

we want the example to be tested in the CI

They are in tests/execution/test_examples.py:

def test_tfhers_example():
    path_to_test_script = f"{os.path.dirname(os.path.abspath(__file__))}/../../examples/tfhers/"
    test_script_filename = "test.sh"
    assert (
        os.system(f"cd {path_to_test_script} && sh {test_script_filename}") == 0
    ), "test script failed"

this PR was not meant to replace examples/tfhers, it was meant to be in addition

Yes, but it was different. It's simpler to have two examples using the same tooling and steps, when all the difference resides in the computation they are performing (one is doing some simple scalar operations, while the other is performing ML on tensors).

I'm also not against this PR or having something that runs entirely in Rust. This can be merged separately in a place where we see fit. However, It was just easier for me to start from the example we had than updating your PR while you where away.

bcm-at-zama · 2024-11-28T08:31:51Z

ok, indeed, I would have had to add a way to test .rs files directly in concrete/frontends/concrete-python/examples/
ok I didn't know you run your .sh somewhere in the CI
still, at the end, I would vote for a single example for our users, with a [main].rs and a [concrete].py / [concrete-ml].py, since I think our tests are the best way to explain things to users, and the current system is very complicated to show to an external person

But yes, I let you do as you want, and we'll see how it is when it's ready and if it's simple enough for the customers.

youben11 · 2024-11-28T08:59:22Z

Like I said, I don't mind pushing this PR to have complete standalone ML example. It was just not the preferable way (at least to me) to have something working e2e in the CI as quickly as possible. The focus was to have a green test, not an example.

bcm-at-zama · 2024-11-28T09:33:00Z

The focus was to have a green test, not an example.

Sure. Then, before the final release to the customer, we'll have maybe an example in another form

cla-bot bot added the cla-signed label Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

View reviewed changes

frontends/concrete-python/examples/tfhers-cml/Cargo.toml Outdated Show resolved Hide resolved

bcm-at-zama commented Oct 2, 2024

View reviewed changes

frontends/concrete-python/examples/tfhers-cml/src/main.rs Show resolved Hide resolved

bcm-at-zama commented Oct 2, 2024

View reviewed changes

frontends/concrete-python/examples/tfhers-cml/src/main.rs Show resolved Hide resolved

bcm-at-zama commented Oct 2, 2024

View reviewed changes

frontends/concrete-python/examples/tfhers-cml/src/main.rs Outdated Show resolved Hide resolved

bcm-at-zama commented Oct 2, 2024

View reviewed changes

frontends/concrete-python/examples/tfhers-cml/src/main.rs Show resolved Hide resolved

bcm-at-zama commented Oct 2, 2024

View reviewed changes

bcm-at-zama force-pushed the first_tfhe_rs_cml_test branch 3 times, most recently from 65a49a7 to f0cb443 Compare October 2, 2024 08:36

bcm-at-zama commented Oct 2, 2024

View reviewed changes

bcm-at-zama changed the title ~~[WIP] Working on a tutorial on TFHE-rs <> Concrete ML interoperability~~ Working on a tutorial on TFHE-rs <> Concrete ML interoperability: step 1 Oct 2, 2024

bcm-at-zama force-pushed the first_tfhe_rs_cml_test branch from f0cb443 to a914cbd Compare October 2, 2024 16:50

bcm-at-zama marked this pull request as ready for review October 2, 2024 16:52

bcm-at-zama requested review from youben11 and BourgerieQuentin October 2, 2024 16:52

youben11 reviewed Oct 3, 2024

View reviewed changes

bcm-at-zama added 12 commits October 9, 2024 13:11

docs(frontend): c

2043752

docs(frontend): vectorizing

8d2ebbf

docs(frontend): vectorizing

2d6401c

docs(frontend): vectorizing

16b3eba

docs(frontend): vectorizing

0aba7bf

docs(frontend): vectorizing

b9b742d

docs(frontend): vectorizing in .py, does not work

5b5eeb7

docs(frontend): vectorizing in .py

18ab8b3

docs(frontend): vectorizing in .py

f76ed2a

docs(frontend): vectorizing in .py

abfb28d

docs(frontend): vectorizing in .py

133bad9

docs(frontend): vectorizing in .py

ed52cd8

bcm-at-zama added 3 commits October 9, 2024 15:46

docs(frontend): getting params from TFHE-rs

6c03781

docs(frontend): getting params from TFHE-rs

9f6ebda

docs(frontend): getting params from TFHE-rs

90c06c1

bcm-at-zama commented Oct 9, 2024

View reviewed changes

docs(frontend): TFHE-rs 0.7.1

18dc502

BourgerieQuentin closed this Nov 27, 2024


		from numpy.random import randint

		# FIXME: should we move this to Concrete library directly, hidden to the user


		return tfhers_params, tfhers_type, tfhers_int

		# FIXME Params: users shouldn't change them, should we hide it


		assert GLWE_DIM == 1, "glwe dim must be 1"

		# Options: the user can change the following


		const BLOCK_PARAMS: ClassicPBSParameters = tfhe::shortint::prelude::PARAM_MESSAGE_2_CARRY_3_KS_PBS;

		fn write_json_to_file<T: Serialize>(value: &T, filename: &str) {

Working on a tutorial on TFHE-rs <> Concrete ML interoperability: step 1 #1080

Working on a tutorial on TFHE-rs <> Concrete ML interoperability: step 1 #1080

Conversation

bcm-at-zama commented Oct 2, 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

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcm-at-zama commented Oct 7, 2024

bcm-at-zama commented Oct 7, 2024

bcm-at-zama commented Oct 9, 2024

Choose a reason for hiding this comment

bcm-at-zama commented Oct 10, 2024

BourgerieQuentin commented Nov 27, 2024

bcm-at-zama commented Nov 27, 2024

BourgerieQuentin commented Nov 27, 2024

youben11 commented Nov 27, 2024

bcm-at-zama commented Nov 27, 2024

bcm-at-zama commented Nov 27, 2024

youben11 commented Nov 28, 2024

bcm-at-zama commented Nov 28, 2024

youben11 commented Nov 28, 2024

bcm-at-zama commented Nov 28, 2024

bcm-at-zama commented Oct 2, 2024 •

edited

Loading