-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add a protocol
kwarg to encrypt()
method to select between MPC and Paillier HE
#3234
Conversation
Checkout to earlier commit for version with both
Codecov Report
@@ Coverage Diff @@
## master #3234 +/- ##
=========================================
Coverage ? 94.54%
=========================================
Files ? 146
Lines ? 15920
Branches ? 0
=========================================
Hits ? 15052
Misses ? 868
Partials ? 0
|
Please always use other than your master branch for creating PR |
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.
Very clean and thorough PR, congrats!
I've added some comments below :)
test/torch/tensors/test_native.py
Outdated
|
||
x = torch.randint(10, (1, 5), dtype=torch.float32) | ||
x_encrypted = x.encrypt(workers=[bob, alice], crypto_provider=james, args_fix_prec={"base": 10}) | ||
assert torch.all(torch.eq(x_encrypted.get().float_prec(), x)) |
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.
Oh we've forgotten to add a generic decrypt method!!
Would you be willing to do also this one? :)
for MPC .decrypt() would call .get().float_prec()
and for paillier, it would'nt change
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.
Sure. Do I submit it in another PR?
I've incorporated the suggestions mentioned.
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.
In this PR would be more coherent if you can do it !
Thanks to @Jasopaum
protocol
kwarg to encrypt()
method to select between MPC and Paillier HE
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.
One last question and this is good for me!
""" | ||
|
||
if protocol.lower() == "mpc": | ||
x_encrypted = self.copy() |
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.
Do you need copy() here?
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.
@LaRiffle It served a purpose of not making it an inplace operation
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.
Oh I see
Ok I got your point, let's keep it your way
Fixes #3212