Implemented Subtraction operation of FV Scheme #3775

rav1kantsingh · 2020-06-23T14:23:55Z

Description

closes #3702
Implemented Subtraction operation of the FV scheme.

Can Subtract two ciphertexts or a plaintext from a ciphertext.

Affected Dependencies

None

How has this been tested?

Added tests which check the subtracted values from the operation.

Checklist

I have followed the Contribution Guidelines and Code of Conduct
I have commented my code following the OpenMined Styleguide
I have labeled this PR with the relevant Type labels
My changes are covered by tests

codecov · 2020-06-23T15:11:58Z

Codecov Report

Merging #3775 into master will decrease coverage by 0.07%.
The diff coverage is 93.20%.

@@            Coverage Diff             @@
##           master    #3775      +/-   ##
==========================================
- Coverage   94.76%   94.68%   -0.08%     
==========================================
  Files         186      186              
  Lines       18477    18571      +94     
==========================================
+ Hits        17509    17584      +75     
- Misses        968      987      +19

Impacted Files	Coverage Δ
syft/frameworks/torch/he/fv/util/operations.py	`93.54% <90.32%> (-1.78%)`	⬇️
syft/frameworks/torch/he/fv/evaluator.py	`94.04% <92.00%> (-3.52%)`	⬇️
test/torch/tensors/test_fv.py	`100.00% <100.00%> (ø)`
syft/workers/node_client.py	`27.88% <0.00%> (-2.12%)`	⬇️

bcebere · 2020-06-23T19:29:42Z

syft/frameworks/torch/he/fv/evaluator.py

-            return self._add_plain_cipher(op2, op1)
+            return self._sub_cipher_plain(op1, op2)
+
+        elif isinstance(op1, PlainText) and isinstance(op2, CipherText):


Maybe a method for the ifs? to check if both are ciphertexts, different or throw exception.
to reduce the duplicated code.

rav1kantsingh · 2020-06-24T14:58:03Z

syft/frameworks/torch/he/fv/evaluator.py

+class ParamTypes(Enum):
+    """Enumeration for type checking of parameters.
+    """
+
+    CTCT = 1
+    PTPT = 2
+    CTPT = 3
+    PTCT = 4
+
+
+def _typecheck(op1, op2):
+    """Check the type of parameters used and return correct enum type."""
+    if isinstance(op1, CipherText) and isinstance(op2, CipherText):
+        return ParamTypes.CTCT
+    elif isinstance(op1, PlainText) and isinstance(op2, PlainText):
+        return ParamTypes.PTPT
+    elif isinstance(op1, CipherText) and isinstance(op2, PlainText):
+        return ParamTypes.CTPT
+    elif isinstance(op1, PlainText) and isinstance(op2, CipherText):
+        return ParamTypes.PTCT
+    else:
+        return None


@bcebere I have tried to update it as per your review, please check and suggest if anything better can be done 🙂

Maybe throw directly, instead of renurning None?

But then I would have to remove the name of operation. Can you suggest another error message without operation name.

bcebere

Just a small comment, otherwise LGTM! Great work!

bcebere · 2020-06-24T17:33:21Z

syft/frameworks/torch/he/fv/evaluator.py

+class ParamTypes(Enum):
+    """Enumeration for type checking of parameters.
+    """
+
+    CTCT = 1
+    PTPT = 2
+    CTPT = 3
+    PTCT = 4
+
+
+def _typecheck(op1, op2):
+    """Check the type of parameters used and return correct enum type."""
+    if isinstance(op1, CipherText) and isinstance(op2, CipherText):
+        return ParamTypes.CTCT
+    elif isinstance(op1, PlainText) and isinstance(op2, PlainText):
+        return ParamTypes.PTPT
+    elif isinstance(op1, CipherText) and isinstance(op2, PlainText):
+        return ParamTypes.CTPT
+    elif isinstance(op1, PlainText) and isinstance(op2, CipherText):
+        return ParamTypes.PTCT
+    else:
+        return None


Maybe throw directly, instead of renurning None?

youben11

LGTM! I just put some remarks about docs

youben11 · 2020-06-28T09:00:31Z

syft/frameworks/torch/he/fv/evaluator.py

+            ct (Ciphertext): First argument.
+            pt (Plaintext): Second argument.


Can you describe what the argument is for the operation here?

youben11 · 2020-06-28T09:00:41Z

syft/frameworks/torch/he/fv/evaluator.py

+            ct (Ciphertext): First argument.
+            pt (Plaintext): Second argument.


Can you describe what the argument is for the operation here?

youben11 · 2020-06-28T09:00:48Z

syft/frameworks/torch/he/fv/evaluator.py

+            ct1 (Ciphertext): First argument.
+            ct2 (Ciphertext): Second argument.


Can you describe what the argument is for the operation here?

youben11 · 2020-06-28T09:05:04Z

syft/frameworks/torch/he/fv/util/operations.py

+    """return subtraction of two polynomials with all coefficients of
+    polynomial %q(coefficient modulus)"""


Can you describe what the arguments are for the operation here?

rav1kantsingh added Type: New Feature ➕ Introduction of a completely new addition to the codebase Crypto Team labels Jun 23, 2020

rav1kantsingh added this to the BFV Tensor in Python milestone Jun 23, 2020

rav1kantsingh requested review from youben11 and a team June 23, 2020 14:23

rav1kantsingh changed the title ~~Add Subtraction operation of FV Scheme~~ Implemented Subtraction operation of FV Scheme Jun 23, 2020

bcebere reviewed Jun 23, 2020

View reviewed changes

rav1kantsingh commented Jun 24, 2020

View reviewed changes

rav1kantsingh requested a review from bcebere June 24, 2020 17:17

bcebere approved these changes Jun 24, 2020

View reviewed changes

rav1kantsingh added 3 commits June 26, 2020 12:52

Add Subtraction operation of FV Scheme

85fb124

Review Update

3cf4cf9

refractor

53392bf

rav1kantsingh force-pushed the subtraction-operation branch from 3ea3fc5 to 53392bf Compare June 26, 2020 07:27

rav1kantsingh mentioned this pull request Jun 27, 2020

Multiplication operation of FV scheme. #3711

Merged

4 tasks

youben11 approved these changes Jun 28, 2020

View reviewed changes

Docs Update

cf98a7e

youben11 merged commit fe8311a into OpenMined:master Jun 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented Subtraction operation of FV Scheme #3775

Implemented Subtraction operation of FV Scheme #3775

rav1kantsingh commented Jun 23, 2020

codecov bot commented Jun 23, 2020 •

edited

Loading

bcebere Jun 23, 2020

rav1kantsingh Jun 24, 2020 •

edited

Loading

bcebere Jun 24, 2020

rav1kantsingh Jun 24, 2020

bcebere left a comment

bcebere Jun 24, 2020

youben11 left a comment

youben11 Jun 28, 2020

youben11 Jun 28, 2020

youben11 Jun 28, 2020

youben11 Jun 28, 2020

		ct (Ciphertext): First argument.
		pt (Plaintext): Second argument.

		ct1 (Ciphertext): First argument.
		ct2 (Ciphertext): Second argument.

		"""return subtraction of two polynomials with all coefficients of
		polynomial %q(coefficient modulus)"""

Implemented Subtraction operation of FV Scheme #3775

Implemented Subtraction operation of FV Scheme #3775

Conversation

rav1kantsingh commented Jun 23, 2020

Description

Affected Dependencies

How has this been tested?

Checklist

codecov bot commented Jun 23, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

rav1kantsingh Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcebere left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youben11 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 23, 2020 •

edited

Loading

rav1kantsingh Jun 24, 2020 •

edited

Loading