New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Interfaces to generalize noise calculation in FHE/FHELinalg ops #416

Merged

youben11 merged 2 commits into main from manp-linalg-generic

Sep 12, 2023

Contributor

mkmks commented May 11, 2023

Instead of having one getSQManp implementation per op with a lot of repetition, the noise calculation is now modular.

Ops that implements UnaryEint/BinaryInt/BinaryEint interfaces share the operand noise presence check.
For many scalar ops no further calculation is needed. If it's not the case, an op can override sqMANP.
Integer operand types lookups are abstracted into BinaryInt::operandIntType()
Finding largest operand value for a type is abstracted into BinaryInt::operandMaxConstant
Noise calculation for matmul ops is simplified and it's now general enough to work for matmul_eint_int, matmul_int_eint and dot_eint_int at once.

Unless a sqMANP implementation is very short, the bulk of the noise calculation is moved to pure functions that don't access OpState directly (such as sqMANP_matmul). A sqMANP implementation is, thus, responsible to pass the necessary information from OpState through parameters. This was done to make it explicit that only a small part of OpState is needed for calculating noise, which helped, for example, generalizing matmul_eint_int, matmul_int_eint and dot_eint_int to a single sqMANP_matmul.

I have a hunch that sqMANP_conv2d could also be expressed through sqMANP_matmul (or it can be generalized to express sqMANP_conv2d`) but I haven't done so yet.

In the future, one could also find a way to declare mlir::tensor ops as implementing UnaryEint/BinaryInt/BinaryEint to throw away even more boilerplate. But there doesn't seem to be a way to that through ODS, so I gave up on it for the time being.

cla-bot bot commented May 11, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @mkmks on file. In order for us to review and merge your code, please sign:

For individual contribution: our CLA
for Bounty submission, if you are an individual: our T&C
for Bounty submission, if you are a company: our T&C
to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

mkmks force-pushed the manp-linalg-generic branch from 12301d2 to 0140a76 Compare

May 12, 2023 07:55

cla-bot bot commented May 12, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @mkmks on file. In order for us to review and merge your code, please sign:

For individual contribution: our CLA
for Bounty submission, if you are an individual: our T&C
for Bounty submission, if you are a company: our T&C
to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

mkmks force-pushed the manp-linalg-generic branch from 0140a76 to e3bddf3 Compare

May 12, 2023 09:47

cla-bot bot commented May 12, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @mkmks on file. In order for us to review and merge your code, please sign:

For individual contribution: our CLA
for Bounty submission, if you are an individual: our T&C
for Bounty submission, if you are a company: our T&C
to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

mkmks force-pushed the manp-linalg-generic branch from e3bddf3 to c477308 Compare

May 12, 2023 10:37

cla-bot bot commented May 12, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @mkmks on file. In order for us to review and merge your code, please sign:

For individual contribution: our CLA
for Bounty submission, if you are an individual: our T&C
for Bounty submission, if you are a company: our T&C
to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

mkmks force-pushed the manp-linalg-generic branch from c477308 to 015e3d7 Compare

May 12, 2023 10:55

cla-bot bot commented May 12, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @mkmks on file. In order for us to review and merge your code, please sign:

For individual contribution: our CLA
for Bounty submission, if you are an individual: our T&C
for Bounty submission, if you are a company: our T&C
to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

mkmks force-pushed the manp-linalg-generic branch from 015e3d7 to 42a39a8 Compare

May 16, 2023 07:35

cla-bot bot commented May 16, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @mkmks on file. In order for us to review and merge your code, please sign:

For individual contribution: our CLA
for Bounty submission, if you are an individual: our T&C
for Bounty submission, if you are a company: our T&C
to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

BourgerieQuentin force-pushed the manp-linalg-generic branch from 42a39a8 to e46cf4d Compare

May 25, 2023 07:56

cla-bot bot commented May 25, 2023

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @mkmks on file. In order for us to review and merge your code, please sign:

For individual contribution: our CLA
for Bounty submission, if you are an individual: our T&C
for Bounty submission, if you are a company: our T&C
to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

BourgerieQuentin requested changes

View reviewed changes

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHELinalg/IR/FHELinalgOps.td Outdated Show resolved Hide resolved

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHELinalg/IR/FHELinalgOps.td Outdated Show resolved Hide resolved

compilers/concrete-compiler/compiler/include/concretelang/Dialect/FHELinalg/IR/FHELinalgOps.td Outdated Show resolved Hide resolved

compilers/concrete-compiler/compiler/lib/Dialect/FHE/Analysis/MANP.cpp Outdated Show resolved Hide resolved

compilers/concrete-compiler/compiler/lib/Dialect/FHE/Analysis/MANP.cpp Show resolved Hide resolved

compilers/concrete-compiler/compiler/lib/Dialect/FHE/Analysis/MANP.cpp

                 }
+                return llvm::APInt{1, 1, false};
               }
               /// Calculates the squared Minimal Arithmetic Noise Padding of an
               /// `FHELinalg.dot_eint_eint` operation.
               static llvm::APInt getSqMANP(mlir::concretelang::FHELinalg::DotEint op,

Member

BourgerieQuentin May 25, 2023

I guess this is not yet ported to the interface mechanism but there are no hard reason?

compilers/concrete-compiler/compiler/lib/Dialect/FHE/Analysis/MANP.cpp Show resolved Hide resolved

compilers/concrete-compiler/compiler/lib/Dialect/FHE/Analysis/MANP.cpp Outdated Show resolved Hide resolved

compilers/concrete-compiler/compiler/lib/Dialect/FHE/Analysis/MANP.cpp Show resolved Hide resolved

compilers/concrete-compiler/compiler/lib/Dialect/FHE/Analysis/MANP.cpp

-              static llvm::APInt getSqMANP(mlir::tensor::FromElementsOp op,
-                                           llvm::ArrayRef<const MANPLattice *> operandMANPs) {
+              static std::optional<llvm::APInt>
+              getSqMANP(mlir::tensor::FromElementsOp op,

Member

BourgerieQuentin May 25, 2023

Ok I see you dan't use the interfaces mechanism for ops that we don't define, maybe there are a way to attach interface to ops like in the bufferization mechanism (maybe @andidr you know how to achieve that).

Contributor

andidr May 25, 2023

Its is possible to do that with external models. The file compilers/concrete-compiler/compiler/lib/Dialect/Concrete/Transforms/BufferizableOpInterfaceImpl.cpp shows this for BufferizableOpInterface. You just need to ensure that the code attaching the interface is executed at the beginning (e.g., by adding something to CompilationContext::getMLIRContext() in compilers/concrete-compiler/compiler/lib/Support/CompilerEngine.cpp).

Contributor

aquint-zama commented May 25, 2023

cla-bot bot added the cla-signed label

cla-bot bot commented May 25, 2023

The cla-bot has been summoned, and re-checked this pull request!

mkmks force-pushed the manp-linalg-generic branch from e46cf4d to 70f88d7 Compare

September 5, 2023 13:08

youben11 force-pushed the manp-linalg-generic branch 2 times, most recently from 2714a6d to 272b1d9 Compare

September 8, 2023 05:33

Member

youben11 commented Sep 8, 2023

@slab-ci concrete-python-test

slab-ci bot commented Sep 8, 2023

Failed to run command: command 'concrete-python-test' not found in config file

Member

youben11 commented Sep 8, 2023

@slab-ci concrete-python-tests-linux

youben11 force-pushed the manp-linalg-generic branch 3 times, most recently from ee46150 to ba396d4 Compare

September 11, 2023 10:26

Member

youben11 commented Sep 11, 2023

@slab-ci concrete-python-tests-linux

Member

youben11 commented Sep 11, 2023

@slab-ci compiler-cpu-build

1 similar comment

Member

BourgerieQuentin commented Sep 11, 2023

@slab-ci compiler-cpu-build


          refactor(compiler): generalize noise calculation in FHE/FHELinalg

c581544

Instead of having one `getSQManp` implementation per op with a lot of repetition, the noise
calculation is now modular.

- Ops that implements`UnaryEint`/`BinaryInt`/`BinaryEint` interfaces share the operand noise
presence check.
- For many scalar ops no further calculation is needed. If it's not the case, an op can override
`sqMANP`.
- Integer operand types lookups are abstracted into `BinaryInt::operandIntType()`
- Finding largest operand value for a type is abstracted into `BinaryInt::operandMaxConstant`
- Noise calculation for matmul ops is simplified and it's now general enough to work for
`matmul_eint_int`, `matmul_int_eint` and `dot_eint_int` at once.


          docs(compiler): build latest docs

734f986

youben11 force-pushed the manp-linalg-generic branch from ba396d4 to 734f986 Compare

September 12, 2023 08:14

BourgerieQuentin approved these changes

View reviewed changes

youben11 merged commit d16ce81 into main

24 of 28 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment