-
Notifications
You must be signed in to change notification settings - Fork 147
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
fix: key compression always #726
Changes from all commits
abf3e6c
3703f73
d866668
cabc34b
ffbea40
3a8dca0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -347,7 +347,7 @@ def _get_training_quantized_module( | |
# Enable the underlying FHE circuit to be composed with itself | ||
# This feature is used in order to be able to iterate in the clear n times without having | ||
# to encrypt/decrypt the weight/bias values between each loop | ||
configuration = Configuration(composable=True) | ||
configuration = Configuration(composable=True, compress_evaluation_keys=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. compress pbs keys for training There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. which makes me thinkg, it should be using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, you're right, same as for DF which was not enabled before There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually same as below, we should probably revert this change : it's enabled by QM's compilation anyway, so it's a bit confusing to add it here DF is different because it directly compiles with CP tho yes |
||
|
||
composition_mapping = {0: 2, 1: 3} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,7 @@ def run_hybrid_llm_test( | |
# Multi-parameter strategy is used in order to speed-up the FHE executions | ||
configuration = Configuration( | ||
single_precision=False, | ||
compress_input_ciphertexts=True, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these circuits can be fully leveled or not, always good to test with default compilation parameters There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so why not also enabling key compression ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is enabled by the QM compilation (the env variable defaults to 1) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so is |
||
) | ||
|
||
# Create a hybrid model | ||
|
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.
sanity checks should run with default compile parameters