Skip to content
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.

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

Preserve explicit optimizer partition frontiers for TFHE circuit parametrization #702

Merged
merged 6 commits into from
Mar 8, 2024

Conversation

andidr
Copy link
Contributor

@andidr andidr commented Feb 7, 2024

The Concrete Optimizer is invoked on a representation of the program
in the high-level FHELinalg / FHE Dialects and yields a solution with
a one-to-one mapping of operations to keys. However, the abstractions
used by these dialects do not allow for references to keys and the
application of the solution is delayed util the pipeline reaches a
representation of the program in the lower-level TFHE dialect. Various
transformation applied by the pipeline along the way may break the
one-to-one mapping and add indirections into producer-consumer
relationships, resulting in abiguous or partial mappings of TFHE
operations to the keys. In particular, explicit frontiers between
optimizer partitions may not be recovered.

This commit preserves explicit frontiers between optimizer partitions
as optimizer.partition_frontier operations and lowers these to
keyswitch operations before parametrization of TFHE operations.

Resolves Issue #538.

@andidr
Copy link
Contributor Author

andidr commented Feb 7, 2024

@slab-ci concrete-python-tests-linux

Copy link
Contributor

@rudy-6-4 rudy-6-4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please split that in several PR next time, even if all other commits are small.

@andidr
Copy link
Contributor Author

andidr commented Feb 8, 2024

Please split that in several PR next time, even if all other commits are small.

Agreed that this diverts the focus from the actual commit fixing the parametrization pass by submerging it in CI fixes. The idea was to give the CI a chance to complete for the review and then to rebase the commits to main once the CI fixes are merged.

Copy link
Member

@BourgerieQuentin BourgerieQuentin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow our online discussion the following case is not handled as you don't find the extra keyswitch as ancestor due of loop nest

func.func @main(%arg0: tensor<2x!FHE.eint<1>>, %arg1: tensor<2x!FHE.eint<7>>) -> (tensor<2x!FHE.eint<8>>, tensor<2x!FHE.eint<5>>) {
  %cst = arith.constant dense<[0, 1]> : tensor<2xi64>
  %0 = "FHELinalg.apply_lookup_table"(%arg0, %cst) : (tensor<2x!FHE.eint<1>>, tensor<2xi64>) -> tensor<2x!FHE.eint<8>>
  %cst_0 = arith.constant dense<0> : tensor<128xi64>
  %1 = "FHELinalg.apply_lookup_table"(%arg1, %cst_0) : (tensor<2x!FHE.eint<7>>, tensor<128xi64>) -> tensor<2x!FHE.eint<8>>
  %2 = "FHELinalg.add_eint"(%0, %1) : (tensor<2x!FHE.eint<8>>, tensor<2x!FHE.eint<8>>) -> tensor<2x!FHE.eint<8>>
  %c4_i4 = arith.constant 4 : i4
  %cst_1 = arith.constant dense<0> : tensor<256xi64>
  %3 = "FHELinalg.apply_lookup_table"(%1, %cst_1) : (tensor<2x!FHE.eint<8>>, tensor<256xi64>) -> tensor<2x!FHE.eint<5>>
  return %2, %3 : tensor<2x!FHE.eint<8>>, tensor<2x!FHE.eint<5>>
}

got assertion

concretecompiler: /home/yundsi/code/concrete/compilers/concrete-compiler/compiler/lib/Dialect/TFHE/Transforms/TFHECircuitSolutionParametrization.cpp:150: const concrete_optimizer::dag::ConversionKeySwitchKey& mlir::concretelang::{anonymous}::CircuitSolutionWrapper::lookupConversionKeyswitchKey(uint64_t, uint64_t) const: Assertion `convKSKIt != solution.circuit_keys.conversion_keyswitch_keys.cend() && "Required conversion key must be available"' failed.
Aborted (core dumped)

@andidr
Copy link
Contributor Author

andidr commented Feb 20, 2024

Updated this to include explicit optimizer partition frontiers.

@andidr andidr force-pushed the andi/type-inference-fixes branch 2 times, most recently from 965b02e to f8ec3f4 Compare February 20, 2024 14:42
@andidr andidr changed the title Preserve original values of extra conversions in TFHE circuit parametrization Preserve explicit optimizer partition frontiers for TFHE circuit parametrization Feb 20, 2024
@andidr andidr force-pushed the andi/type-inference-fixes branch 9 times, most recently from 54384a1 to 9de637b Compare February 22, 2024 05:19
@zama-ai zama-ai deleted a comment from slab-ci bot Feb 22, 2024
@BourgerieQuentin
Copy link
Member

@slab-ci concrete-python-tests-linux

Copy link
Member

@BourgerieQuentin BourgerieQuentin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be great to add the example that I spotted (same as 538 but with linalg ops) and remove skipping of min max test in concrete python test.

        name.endswith("xfail_if_multi")
        and configuration.parameter_selection_strategy == fhe.ParameterSelectionStrategy.MULTI
    )
    if can_be_known_failure:
        pytest.skip(
            reason="Compiler parametrization pass make the compilation fail with an assertion"
        )

@andidr andidr requested a review from aPere3 February 23, 2024 09:58
@andidr andidr force-pushed the andi/type-inference-fixes branch 3 times, most recently from b66979c to 86c327e Compare February 23, 2024 14:50
@andidr
Copy link
Contributor Author

andidr commented Feb 23, 2024

@slab-ci concrete-python-tests-linux

@andidr andidr force-pushed the andi/type-inference-fixes branch 2 times, most recently from a45f265 to 151c394 Compare March 5, 2024 04:49
@andidr
Copy link
Contributor Author

andidr commented Mar 5, 2024

@slab-ci concrete-python-tests-linux

@andidr andidr force-pushed the andi/type-inference-fixes branch 2 times, most recently from 71f2701 to 763fb84 Compare March 6, 2024 13:49
…erenceUtils

The main debugging function is
`TypeInferenceUtils::dumpAllState(mlir::Operation* op)` which dumps
the entire state of type inference for the function containing `op`.
This adds a new dialect called `Optimizer` with operations related to
the Concrete Optimizer. Currently, there is only one operation
`optimizer.partition_frontier` that can be inserted between a producer
and a consumer which belong to different partitions computed by the
optimizer. The purpose of this operation is to preserve explicit key
changes from the invocation of the optimizer on high-level dialects
(i.e., FHELinalg / FHE) until the IR is provided with actual
references to keys in low-level dialects (i.e., TFHE).
@andidr andidr force-pushed the andi/type-inference-fixes branch 2 times, most recently from 11cddff to fbc18a6 Compare March 7, 2024 14:20
…gh the pipeline

The Concrete Optimizer is invoked on a representation of the program
in the high-level FHELinalg / FHE Dialects and yields a solution with
a one-to-one mapping of operations to keys. However, the abstractions
used by these dialects do not allow for references to keys and the
application of the solution is delayed until the pipeline reaches a
representation of the program in the lower-level TFHE dialect. Various
transformations applied by the pipeline along the way may break the
one-to-one mapping and add indirections into producer-consumer
relationships, resulting in ambiguous or partial mappings of TFHE
operations to the keys. In particular, explicit frontiers between
optimizer partitions may not be recovered.

This commit preserves explicit frontiers between optimizer partitions
as `optimizer.partition_frontier` operations and lowers these to
keyswitch operations before parametrization of TFHE operations.
@andidr andidr force-pushed the andi/type-inference-fixes branch from fbc18a6 to 3219929 Compare March 7, 2024 14:42
@andidr andidr merged commit 3219929 into main Mar 8, 2024
29 of 31 checks passed
@andidr andidr deleted the andi/type-inference-fixes branch March 8, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants