Change Partition Operation #858

youben11 · 2024-05-30T08:04:24Z

No description provided.

frontends/concrete-python/concrete/fhe/compilation/compiler.py

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHE/IR/FHEOps.td

compilers/concrete-compiler/compiler/lib/Conversion/FHETensorOpsToLinalg/TensorOpsToLinalg.cpp

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHE/IR/FHEOps.td

compilers/concrete-compiler/compiler/lib/Dialect/FHE/Transforms/Optimizer.cpp

frontends/concrete-python/concrete/fhe/compilation/compiler.py

umut-sahin

Just a few suggestions on the Python side, looks good 👍

frontends/concrete-python/tests/representation/test_node.py

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

frontends/concrete-python/concrete/fhe/compilation/module_compiler.py

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHE/IR/FHEOps.h

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHE/IR/FHEAttrs.td

BourgerieQuentin · 2024-06-05T11:51:38Z

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHE/IR/FHEAttrs.td

+        "int":$maxNoiseLevel,
+        "FloatAttr":$log2PFail,
+        "bool": $bigEncryptionKey,
+        DefaultValuedParameter<"int", "-1">: $ciphertextModulus


Is that possible to have verifier on the attr? I guess we should as example not support for ciphertextModulus != 64 at least at the beginning.

I don't think it's possible AFAIK

hasVerifier doesn't seem to be supported at least

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHE/IR/FHEOps.td

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHE/IR/FHEAttrs.td

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

BourgerieQuentin

Let's wait to plug with the optimizer, I schedule a sync tomorrow to discuss of that PR

BourgerieQuentin · 2024-08-01T08:35:34Z

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHE/IR/FHEAttrs.td

+        "int":$glweDim,
+        "int":$polySize
+    );
+    let assemblyFormat = "`<` `name` $name `,` `lwe_dim` $lweDim `,` `glwe_dim` $glweDim `,` `poly_size` $polySize `>`";


still missing noise level, right? (cc @rudy-6-4)
Maybe also we can name parameters n, k, N to be more compact ?

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHE/IR/FHEOps.td

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHELinalg/IR/FHELinalgOps.td

compilers/concrete-compiler/compiler/lib/Bindings/Python/FHEModule.cpp

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

BourgerieQuentin · 2024-08-01T14:01:51Z

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

@@ -3973,4 +3983,92 @@ def zeros(self, resulting_type: ConversionType) -> Conversion:
            original_bit_width=1,
        )

+    def get_partition_name(self, partition: tfhers.TFHERSParams) -> str:


I'm wondering why you generates names, I fell it should be the responsability of the user to do it.

names are used to identify partitions, which is abstracted to the user. So generating unique names for a given set of parameters is how to differentiate between these partitions and I don't see why this should be left to the user.

I see. Maybe the name should be optional (but is not a big deal to make it mandatory), I see some advantage to make it explicit, as example if we can specify name we can use concrete to switch from private thfe-rs key to an other which share the same parameters.

BourgerieQuentin · 2024-08-01T14:18:03Z

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


 import numpy as np

 from ..dtypes import Integer


+class TFHERSParams:


Maybe here we should have two part of parameters, one is for crypto parameters needed for changing partition (lwe_dim,glwe_dim,poly_size, ciphertext_modulus, noise_level) the other one is for the radix encoding (message_modulus, carry_modulus).
That will allow to separate the change_partition and the trans encoding between radix and native encoding, it will allow also the composability with other things than tfhe-rs.

we could also maybe remove unnecessary parameters

BourgerieQuentin · 2024-08-01T14:21:27Z

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

+    params: TFHERSParams
+
+    def __init__(
+        self,


Here it could a TFHERSIntergerType will be composed of crypto params and radix encoding parameter,

compilers/concrete-compiler/compiler/lib/Dialect/FHE/Transforms/Optimizer.cpp

github-actions

Benchmark

Benchmark suite	Current: `52736d4`	Previous: `29811c9`	Ratio
`v0 PBS table generation`	`60986750` ns/iter (`± 3180220`)	`61096369` ns/iter (`± 3565593`)	`1.00`
`v0 PBS simulate dag table generation`	`40885868` ns/iter (`± 2036630`)	`40854653` ns/iter (`± 272815`)	`1.00`
`v0 WoP-PBS table generation`	`49846938` ns/iter (`± 1239929`)	`50100311` ns/iter (`± 2555686`)	`0.99`

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

youben11 · 2024-08-15T07:56:48Z

@BourgerieQuentin regarding parameterization of TFHERSIntegerType:

I think the type is specialized and already has a specific encoding. Supporting other kind of integers would build on the Integer type instead of customizing this one.
The reason TFHERSParams are separated into a different object is to make it possible to import/export them separately. Maybe they can also be put into the int type? Also because they were too much initially

I may be missing something, but I don't see why we should separate them. Specially now.

BourgerieQuentin · 2024-08-22T13:05:34Z

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

@@ -3973,4 +3983,92 @@ def zeros(self, resulting_type: ConversionType) -> Conversion:
            original_bit_width=1,
        )

+    def get_partition_name(self, partition: tfhers.TFHERSParams) -> str:


I see. Maybe the name should be optional (but is not a big deal to make it mandatory), I see some advantage to make it explicit, as example if we can specify name we can use concrete to switch from private thfe-rs key to an other which share the same parameters.

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

mypy was complaining hint was a module

we are checking if the selected parameter selection can handle tfhers integers

as test runs two times, once in mono, and once in multi, we can just skip the test unless we are in multi

youben11 requested review from rudy-6-4, BourgerieQuentin and umut-sahin May 30, 2024 08:04

cla-bot bot added the cla-signed label May 30, 2024

youben11 force-pushed the tfhe-rs-compat-plug-opt branch from 0c87d72 to e651fc9 Compare May 30, 2024 08:32

aPere3 reviewed May 30, 2024

View reviewed changes

frontends/concrete-python/concrete/fhe/compilation/compiler.py Outdated Show resolved Hide resolved

rudy-6-4 reviewed May 30, 2024

View reviewed changes

frontends/concrete-python/concrete/fhe/compilation/compiler.py Outdated Show resolved Hide resolved

rudy-6-4 reviewed May 30, 2024

View reviewed changes

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHE/IR/FHEOps.td Show resolved Hide resolved

rudy-6-4 reviewed May 30, 2024

View reviewed changes

compilers/concrete-compiler/compiler/lib/Conversion/FHETensorOpsToLinalg/TensorOpsToLinalg.cpp Outdated Show resolved Hide resolved

BourgerieQuentin reviewed May 31, 2024

View reviewed changes

umut-sahin reviewed Jun 3, 2024

View reviewed changes

youben11 force-pushed the tfhe-rs-compat-plug-opt branch 2 times, most recently from 035bd63 to be92025 Compare June 4, 2024 12:52

BourgerieQuentin requested changes Jun 5, 2024

View reviewed changes

youben11 force-pushed the tfhe-rs-compat-plug-opt branch 3 times, most recently from 7f5524b to 15dbba8 Compare July 31, 2024 10:10

BourgerieQuentin reviewed Aug 1, 2024

View reviewed changes

youben11 commented Aug 12, 2024

View reviewed changes

compilers/concrete-compiler/compiler/lib/Dialect/FHE/Transforms/Optimizer.cpp Outdated Show resolved Hide resolved

youben11 force-pushed the tfhe-rs-compat-plug-opt branch from 9d888e6 to 189d273 Compare August 13, 2024 10:47

github-actions bot reviewed Aug 13, 2024

View reviewed changes

youben11 force-pushed the tfhe-rs-compat-plug-opt branch 2 times, most recently from 70668b7 to 499cda9 Compare August 14, 2024 15:37

youben11 requested a review from BourgerieQuentin August 15, 2024 07:49

BourgerieQuentin approved these changes Aug 22, 2024

View reviewed changes

youben11 added 4 commits August 22, 2024 15:02

chore(frontend): remove mypy warning about imported func

c495ea9

mypy was complaining hint was a module

fix(frontend): support tfhers int with multi-param only

1163b2f

feat(compiler/frontend): change partition operation

f4eb11f

feat(frontend): parametrize tfhers integers

b3d5be8

youben11 added 8 commits August 22, 2024 15:03

feat(compiler): add partition attr to change partition to/from it

ee10690

feat(frontend): forward tfhers params in change_partition op

15eb727

docs(compiler): add change_partition/partition_attr to the MLIR docs

d605f1e

refactor(frontend): move tfhers/params check to converter

7cc06e8

we are checking if the selected parameter selection can handle tfhers integers

chore(frontend): format

fa80d46

refactor(compiler): plug with change_partition support of optimizer

0bb418d

test(frontend): check input/output match tfhers params

2d3f8ad

test(frontend): more tests and coverage for tfhers

613ecbe

youben11 force-pushed the tfhe-rs-compat-plug-opt branch from 9e77056 to 29811c9 Compare August 22, 2024 14:06

test(frontend): run only in multi instead of changing config

52736d4

as test runs two times, once in mono, and once in multi, we can just skip the test unless we are in multi

youben11 force-pushed the tfhe-rs-compat-plug-opt branch from 29811c9 to 52736d4 Compare August 22, 2024 14:39

youben11 merged commit 2e90f53 into main Aug 22, 2024
36 of 43 checks passed

youben11 deleted the tfhe-rs-compat-plug-opt branch August 22, 2024 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change Partition Operation #858

Change Partition Operation #858

youben11 commented May 30, 2024

umut-sahin left a comment

BourgerieQuentin Jun 5, 2024

youben11 Jun 6, 2024

youben11 Jun 6, 2024

BourgerieQuentin left a comment

BourgerieQuentin Aug 1, 2024

BourgerieQuentin Aug 1, 2024

youben11 Aug 13, 2024

BourgerieQuentin Aug 22, 2024

BourgerieQuentin Aug 1, 2024

BourgerieQuentin Aug 1, 2024

BourgerieQuentin Aug 1, 2024

github-actions bot left a comment •

edited

Loading

youben11 commented Aug 15, 2024

BourgerieQuentin Aug 22, 2024

Change Partition Operation #858

Change Partition Operation #858

Conversation

youben11 commented May 30, 2024

umut-sahin 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

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

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 left a comment • edited Loading

Choose a reason for hiding this comment

Benchmark

youben11 commented Aug 15, 2024

Choose a reason for hiding this comment

github-actions bot left a comment •

edited

Loading