feat: Add tfhe-rs compatibility for model inputs /outputs #997

andrei-stoian-zama · 2025-01-16T17:47:52Z

Adds the ciphertext_format parameter to compile calls.

Needs new fhext & new compiler to run.

bcm-at-zama · 2025-01-22T17:47:33Z

src/concrete/ml/sklearn/base.py

@@ -506,9 +514,33 @@ def _get_module_to_compile(self) -> Union[Compiler, QuantizedModule]:
            Union[Compiler, QuantizedModule]: The module instance to compile.
        """

+    def _get_tfhres_module_to_compile(self, inputset):


typo? tfhres -> tfhers

bcm-at-zama · 2025-01-22T17:51:24Z

src/concrete/ml/sklearn/base.py

+                # For now, use the virtual library when simulating
+                # circuits that use CRT  encoding because the official simulation is too slow
+                # FIXME: https://github.com/zama-ai/concrete-ml-internal/issues/4391
+                if USE_OLD_VL or is_crt_encoding:


I didn't know USE_OLD_VL what still there

it's disabled but still there

is there a reason to keep it? do you still use it? (I guess it's not tested so certainly, soon, it's no more working, anyway)

Old vl is simply CP graph execution, so it is tested in Concrete. but you're right in the sense that we could easily remove it - we kept it thinking it might be useful at some point, but it's not anymore

it is tested in Concrete
I don't know if they still test it. At some point, they'll certainly remove it as well

bcm-at-zama · 2025-01-22T17:54:47Z

tests/sklearn/test_sklearn_models.py

+    model.compile(x)
+
+    # Check correctness with TFHE-rs inputs/outputs
+    assert numpy.all(y_pred_tfhers == y_pred_disable)


shouldn't you also test vs purely concrete results?

fhe results (with tfhers inputs) should be equal disable for default p_error. Other tests check that concrete fhe and disable are also equal to disable. using fhe=disable here makes this test faster, it's already quite a slow one

I have not understood your answer.

My question is: do we test that results with results "with Concrete format" and "with TFHE-rs format" are the same? (either in simulate or in FHE). If we have that + existing tests that results in TFHE-rs are equivalent in simulate and fhe, that's fine with me.

we compare tfhe-rs inputs vs disable in this test and we compare concrete inputs vs simulate and disable in other tests.

jfrery · 2025-01-23T09:53:08Z

src/concrete/ml/sklearn/base.py

    def compile(
        self,
        X: Data,
+        ciphertext_format: CiphertextFormat = CiphertextFormat.CONCRETE,


can you allow simple string like we do for fhe arg? e.g. I would like to call compile with model.compile(X, ciphertext_format="tfhe-rs")

jfrery · 2025-01-23T09:54:55Z

src/concrete/ml/sklearn/base.py

-            q_y_pred_list = []
-            for q_X_i in q_X:
-                # Expected encrypt_run_decrypt output shape is (1, n_features) while q_X_i
-                # is of shape (n_features,)
-                q_X_i = numpy.expand_dims(q_X_i, 0)


wasn't this necessary?

jfrery · 2025-01-23T09:57:15Z

tests/sklearn/test_sklearn_models.py

+
+    # Check that we can first compile to Concrete, then to
+    # TFHE-rs input/outputs then to concrete again
+    model.compile(x)


You could add model.predict with concrete maybe and add this to the comparisons. We expect them to be equal to disable and to execute with tfhe-rs input

ok, as Benoit asks for it too I'll add it

github-actions · 2025-01-29T09:08:21Z

⚠️ This PR contains unsigned commits. To get your PR merged, please sign those commits (git rebase --exec 'git commit -S --amend --no-edit -n' @{upstream}) and force push them to this branch (git push --force-with-lease).

If you're new to commit signing, there are different ways to set it up:

Sign commits with gpg

Follow the steps below to set up commit signing with gpg:

Sign commits with ssh-agent

Follow the steps below to set up commit signing with ssh-agent:

Sign commits with 1Password

You can also sign commits using 1Password, which lets you sign commits with biometrics without the signing key leaving the local 1Password process.

Learn how to use 1Password to sign your commits.

github-actions · 2025-01-30T22:09:05Z

Coverage failed ❌

Coverage details

---------- coverage: platform linux, python 3.8.18-final-0 -----------
Name                              Stmts   Miss  Cover   Missing
---------------------------------------------------------------
src/concrete/ml/sklearn/base.py     781      1    99%   801
src/concrete/ml/sklearn/glm.py      160     83    48%   82-108, 114-138, 326-353, 359-383
---------------------------------------------------------------
TOTAL                              8619     84    99%

61 files skipped due to complete coverage.

github-actions · 2025-01-30T22:17:54Z

⚠️ Known flaky tests have been rerun ⚠️

One or several tests initially failed but were identified as known flaky. tests. Therefore, they have been rerun and passed. See below for more details.

Failed tests details

Known flaky tests that initially failed:

tests/torch/test_brevitas_qat.py::test_brevitas_tinymnist_cnn[True-True-3]

cla-bot bot added the cla-signed label Jan 16, 2025

andrei-stoian-zama changed the title ~~chore: keygen and encrypt~~ feat: Add tfhe-rs compatibility for model inputs /outputs Jan 16, 2025

andrei-stoian-zama marked this pull request as ready for review January 22, 2025 17:39

andrei-stoian-zama requested a review from a team as a code owner January 22, 2025 17:39

bcm-at-zama reviewed Jan 22, 2025

View reviewed changes

jfrery reviewed Jan 23, 2025

View reviewed changes

andrei-stoian-zama force-pushed the feat/add_tfhers_ciphertext_compilation branch from a29f818 to 81cad15 Compare January 29, 2025 07:09

andrei-stoian-zama and others added 6 commits January 29, 2025 10:12

chore: keygen and encrypt

9ef31ef

feat: tfhres interop for more models

63db3a4

feat: support only 8b tree classifiers

729a757

chore: update deps

89971ef

fix: poetry

20b42ea

chore: update licenses

7f2131f

andrei-stoian-zama force-pushed the feat/add_tfhers_ciphertext_compilation branch from 28050f6 to 7f2131f Compare January 29, 2025 09:12

andrei-stoian-zama added 2 commits January 29, 2025 10:39

fix: tests vs concrete

3057dcc

fix: parameter ordering

f08724f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add tfhe-rs compatibility for model inputs /outputs #997

feat: Add tfhe-rs compatibility for model inputs /outputs #997

andrei-stoian-zama commented Jan 16, 2025

bcm-at-zama Jan 22, 2025

bcm-at-zama Jan 22, 2025

andrei-stoian-zama Jan 22, 2025

bcm-at-zama Jan 23, 2025

andrei-stoian-zama Jan 28, 2025

bcm-at-zama Jan 28, 2025

bcm-at-zama Jan 22, 2025

andrei-stoian-zama Jan 22, 2025

bcm-at-zama Jan 23, 2025

andrei-stoian-zama Jan 28, 2025

jfrery Jan 23, 2025

jfrery Jan 23, 2025

jfrery Jan 23, 2025

andrei-stoian-zama Jan 28, 2025

github-actions bot commented Jan 29, 2025

github-actions bot commented Jan 30, 2025

github-actions bot commented Jan 30, 2025

Known flaky tests that initially failed:

feat: Add tfhe-rs compatibility for model inputs /outputs #997

Are you sure you want to change the base?

feat: Add tfhe-rs compatibility for model inputs /outputs #997

Conversation

andrei-stoian-zama commented Jan 16, 2025

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

github-actions bot commented Jan 29, 2025

github-actions bot commented Jan 30, 2025

Coverage failed ❌

github-actions bot commented Jan 30, 2025

⚠️ Known flaky tests have been rerun ⚠️

Known flaky tests that initially failed: