-
Notifications
You must be signed in to change notification settings - Fork 49
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
LWE: add evaluation key type and technique #1067
base: main
Are you sure you want to change the base?
LWE: add evaluation key type and technique #1067
Conversation
Hey! Sorry for the delay - I'm finally back online for development now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for starting this!
I will explore replacing the BGV / CKKS dialects with the new lwe types in a PR EOW
@@ -348,4 +353,60 @@ def LWE_ModulusChainAttr : AttrDef<LWE_Dialect, "ModulusChain"> { | |||
// let genVerifyDecl = 1; // Verify index into list | |||
} | |||
|
|||
def LWE_BVKeySwitchAttr : AttrDef<LWE_Dialect, "BVKeySwitch"> { | |||
let mnemonic = "bv_keyswitch_technique"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: just remove the _technique
here and below
let description = [{ | ||
An attribute describing the BV technique for keyswitch. | ||
|
||
`base` is the radix base used in decomposition of the coefficient modulus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`base` is the radix base used in decomposition of the coefficient modulus | |
`base` is the radix base used in the digit decomposition of the coefficient modulus |
|
||
let parameters = (ins | ||
"IntegerAttr":$base, | ||
"IntegerAttr":$dnum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for considering the RNS variants! I understand it's important to know the dnums so that the type converter knows the type / shape of the key switching key. For the lowering, we'll need the actual RNS moduli, which we'll pull from the RNS modulus of the ring in the keyswitching key, right?
|
||
let assemblyFormat = "`<` struct(params) `>`"; | ||
|
||
// let genVerifyDecl = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could implement the type constraint here.
|
||
let parameters = (ins | ||
"IntegerAttr":$base, | ||
"IntegerAttr":$dnum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OptionalParameter? (can also update the assembly format for an optional print)
"KeyAttr":$to_key, | ||
"::mlir::polynomial::RingAttr":$ring, | ||
// can not be ArrayRefParameter<"KeySwitchAttr"> | ||
ArrayRefParameter<"Attribute">:$keyswitch_techniques |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this an array ref parameter? Wouldn't only a sinlgle technique apply to a given key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intentionally left the HYBRID keyswitching technique as a combination of BV attr and GHS attr, so that we do not have to define another LWE_HYBRIDKeySwitchAttr
where all base
/dnum
/extra_modulus
will be included. If we accept such elaboration, then we can define it and here can be a single technique.
Related to #1057.
I roughly prototyped it from openfhe/pke/keyswitch (BGV/CKKS), yet I do not know if this is accurate for binfhe schemes (CGGI).
Example: