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

New tensor type for FV scheme #4061

Closed
wants to merge 9 commits into from

Conversation

rav1kantsingh
Copy link
Member

Description

closes #3710
Creating a new tensor type for supporting the FV homomorphic encryption scheme written internally in PySyft.
might remain as a draft for some time because it still needs several changes to make it fully functional.

Affected Dependencies

will update it.

How has this been tested?

will update it

Checklist

@rav1kantsingh rav1kantsingh added Type: New Feature ➕ Introduction of a completely new addition to the codebase Status: In Progress 🌟 This is actively being worked on labels Aug 21, 2020
@rav1kantsingh rav1kantsingh added this to the BFV Tensor in Python milestone Aug 21, 2020
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

Review Jupyter notebook visual diffs & provide feedback on notebooks.


Powered by ReviewNB

@rav1kantsingh rav1kantsingh requested review from youben11 and a team August 21, 2020 10:29
@rav1kantsingh rav1kantsingh marked this pull request as draft August 21, 2020 10:30
@rav1kantsingh rav1kantsingh force-pushed the fv-tensor branch 2 times, most recently from 1593910 to 32a2581 Compare October 29, 2020 05:38
@rav1kantsingh rav1kantsingh marked this pull request as ready for review October 29, 2020 15:28
@rav1kantsingh rav1kantsingh requested a review from a team as a code owner October 29, 2020 15:28
Copy link
Member

@youben11 youben11 left a comment

Choose a reason for hiding this comment

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

I'm actually lost in how a new tensor should be setup, which operations should we add and so on. I will check on slack to see if someone can give it a look, especially for the failing tests.

Comment on lines 58 to 75
def __add__(self, *args, **kwargs):
self.prepareEvaluator()

if not args[0].shape == self.child.shape:
raise ValueError(
f"Cannot add tensors with different shape {args[0].shape} , {self.child.shape}"
)

args = args[0].flatten().tolist()

# if integer values; transform it to plaintext
if isinstance(args[0], int):
args = [self.encoder.encode(x) for x in args]

child = self.child.flatten().tolist()

data = [self.evaluator.add(x, y) for x, y in zip(child, args)]
data = np.array(data).reshape(self.child.shape)
Copy link
Member

Choose a reason for hiding this comment

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

Have you tried to implement the __radd__, __rsub__ methods so that it also can operate when it's the right operand. However, I'm not sure this is how we should do it in PySyft.

Comment on lines 1162 to 1163
# Homomorphic Encryption using private key; if private key available prefer it.
x_encrypted.child.encrypt(private_key, context)
Copy link
Member

Choose a reason for hiding this comment

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

Why is that?

Tensors, so compared to the @overloaded.method, you see
that the @overloaded.module does not hook the arguments.
"""
pass
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be implemented with the other operations?

@gmuraru
Copy link
Member

gmuraru commented Nov 5, 2020

Currently, the tests are failling here:

 def test_serde_coverage():
        """Checks all types in serde are tested"""
        for cls, _ in msgpack.serde.msgpack_global_state.simplifiers.items():
            # currently involves running grid node instance might create overhead
            # To do add method for running pygrid node instance to cover this testing
            if cls == syft.grid.clients.data_centric_fl_client.DataCentricFLClient:
                continue
            has_sample = cls in samples
>           assert has_sample, f"Serde for {cls} is not tested"
E           AssertionError: Serde for <class 'syft.frameworks.torch.tensors.interpreters.bfv.BFVTensor'> is not tested
E           assert False

You need to also add serialization/deserialize tests.

@overloaded.method
def add(self, _self, *args, **kwargs):
"""
Here is an example of how to use the @overloaded.method decorator. To see
Copy link
Member

Choose a reason for hiding this comment

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

MAYBE: Need to update the commnets :D

obj.child = data
return obj

def mm(self, *args, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

Q: Can't we use also overload this?

self.evaluator = None

def encrypt(self, key, context):
self.context = context
Copy link
Member

Choose a reason for hiding this comment

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

Q: Shouldn't we populate self and return self?

inputs = self.child.flatten().tolist()
new_child = [self.encryptor.encrypt(self.encoder.encode(x)) for x in inputs]

data = np.array(new_child).reshape(self.child.shape)
Copy link
Member

Choose a reason for hiding this comment

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

Q: It is necessary to be np.array can't it be tensor?

@madhavajay madhavajay added the 0.2.x Relating to the 0.2.x code branch label Nov 23, 2020
@gmuraru
Copy link
Member

gmuraru commented Nov 23, 2020

@IamRavikantSingh I changed this to point to syft_0.2.x

@gmuraru gmuraru changed the base branch from master to syft_0.2.x November 23, 2020 09:14
1. Add inplace decryption
2. Fix operations on ciphertext:
working: th.add(), a.add(b), a + b (same for sub, mul).
matrix multiplication only working with a.mm(b) yet.
@tudorcebere
Copy link
Collaborator

Hi! 0.2.x hit EOL, closing this as we no longer support it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.2.x Relating to the 0.2.x code branch Status: In Progress 🌟 This is actively being worked on Type: New Feature ➕ Introduction of a completely new addition to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create new Tensor type for BFV HE scheme
5 participants