[tfhers compatibility] frontend support #820
Conversation
youben11
force-pushed
the
tfhe-rs-compat-v0
branch
2 times, most recently
from
16a7aa0
to
4db9e8f
Compare
| tfhers_to_native = { | ||
| inputs_and_output_share_precision, | ||
| } |
There was a problem hiding this comment.
What about from native?
Also is this the condition we really want?
There was a problem hiding this comment.
We basically want to have the input precision (the one of the tfhers integer) to rule it, as tfhers inputs should come from an external computation. We can't just decrease tfhers int bitwidth, so the native one have to follow.
There was a problem hiding this comment.
Can't we assign it the precision obtained from the inputset? (e.g., maybe all values that will be given to the circuit is smaller than 10 so it's enough to process 5 chunks from TFHE-rs)
There was a problem hiding this comment.
It really depends on the usecase, but as I'm currently seeing it, we don't control the tfhers computation part, and we aren't asked to look at it, and what are its outputs, all we know is it's output bitwidth. So we shouldn't have a meaningful inputset to use here to try to know how many chunks should we use.
Cc @BourgerieQuentin wdyt?
There was a problem hiding this comment.
But that'll still be how the circuit is compiled. If they want to have the full range, they should give it in the inputset.
There was a problem hiding this comment.
yes it was what we said, so the output precision can differ
youben11
force-pushed
the
tfhe-rs-compat-v0
branch
from
4db9e8f
to
668f4e8
Compare
| @@ -116,6 +117,20 @@ def typeof(self, value: Union[ValueDescription, Node]) -> ConversionType: | |||
| assert isinstance(value.dtype, Integer) | |||
| bit_width = value.dtype.bit_width | |||
|
|
|||
| # TODO: what about the element type? only unsigned? or not eint at all? | |||
There was a problem hiding this comment.
I guess it depends on how it's done in TFHE-rs. We should think about which operations would be performed on this type and use eint or esint accordingly. We should also test correct transfer of sign (if input was -3 in TFHE-rs, it should correctly be converted to -3 in Concrete).
There was a problem hiding this comment.
we will have at least to see how they do sign encoding and consider that as part of the MLIR conversion
| tfhers_to_native = { | ||
| inputs_and_output_share_precision, | ||
| } |
There was a problem hiding this comment.
Can't we assign it the precision obtained from the inputset? (e.g., maybe all values that will be given to the circuit is smaller than 10 so it's enough to process 5 chunks from TFHE-rs)
| int8 = partial(TFHERSIntegerType, True, 8) | ||
| uint8 = partial(TFHERSIntegerType, False, 8) | ||
| int16 = partial(TFHERSIntegerType, True, 16) | ||
| uint16 = partial(TFHERSIntegerType, False, 16) |
There was a problem hiding this comment.
Just an _ in the beginning would be enough IMO 🙂
| tfhers_to_native = { | ||
| inputs_and_output_share_precision, | ||
| } |
There was a problem hiding this comment.
yes it was what we said, so the output precision can differ
|
@slab-ci concrete-python-tests-linux |
youben11
force-pushed
the
tfhe-rs-compat-v0
branch
from
e812d77
to
0470ba3
Compare
this comes from the need of supporting tfhers integers, which would get erased by this recreation of the dtype, so instead we would update the dtype inplace
youben11
force-pushed
the
tfhe-rs-compat-v0
branch
from
0470ba3
to
41389f8
Compare
- add new tfhers types - add conversion functions between concrete and tfhers types - support conversion function in the compilation pipeline
youben11
force-pushed
the
tfhe-rs-compat-v0
branch
from
41389f8
to
94b3e52
Compare
| from concrete import fhe | ||
| from concrete.fhe import tfhers |
There was a problem hiding this comment.
I think it'd be better if we can do:
from concrete import fhe, tfhers
There was a problem hiding this comment.
This means a different module, and I'm not sure it makes sense as they would depend on each other
No description provided.