Repository: OpenMined/PySyft
Pull request: Added support for negative numbers in reciprocal method #4048
## Description
This PR fixes #4048
Added support for negative numbers when use different methods in Reciprocal.
Uses a signum function to achieve this
Suggestions are welcome! 

## Affected Dependencies
- None

## How has this been tested?
-  No additional tests are needed. Changed the default value in test_reciprocal(): function to include negative numbers too
-  Ran pytest test/torch/tensors/test_precision.py without issue
## Checklist
- [x] I have followed the [Contribution Guidelines](https://github.com/OpenMined/.github/blob/master/CONTRIBUTING.md) and [Code of Conduct](https://github.com/OpenMined/.github/blob/master/CODE_OF_CONDUCT.md)
- [x] I have commented my code following the [OpenMined Styleguide](https://github.com/OpenMined/.github/blob/master/STYLEGUIDE.md)
- [x] I have labeled this PR with the relevant [Type labels](https://github.com/OpenMined/.github/labels?q=Type%3A)
- [ ] My changes are covered by tests


Problem statement:
Reciprocal test enchantment negative numbers
## Description
Test the ```reciprocal``` method from precision using negative numbers

## Expected Behavior
All tests are passing

## Screenshots
If applicable, add screenshots to help explain your problem.

## Additional Context
It might require changes to the reciprocal method

Hints from issue discussion:
I guess the Log and NR method are a problem. Division works fine.. Will look into the issue
Adding you :)
> I guess the Log and NR method are a problem. Division works fine.. Will look into the issue

Also, there might be worth checking out how [CrypTen](https://github.com/facebookresearch/CrypTen) is doing - if they take into consideration negative values.

If not, one idea (it might not be the greatest) is to use symmetry.
hey could you review my PR?
https://github.com/OpenMined/PySyft/pull/4065

Commit message: Negative numbers support for reciprocal

Patch to review:
diff --git a/syft/frameworks/torch/tensors/interpreters/precision.py b/syft/frameworks/torch/tensors/interpreters/precision.py
index d906c96fd2d..f00c42ba9af 100644
--- a/syft/frameworks/torch/tensors/interpreters/precision.py
+++ b/syft/frameworks/torch/tensors/interpreters/precision.py
@@ -468,6 +468,23 @@ def matmul(self, *args, **kwargs):
     __matmul__ = matmul
     mm = matmul
 
+    def signum(self):
+        r"""
+            Calculation of signum function for a given tensor
+        """
+        sgn = self > 0
+        zero = self == 0
+        pos = sgn
+        neg = sgn - 1
+        sgn = pos + neg + zero
+        return sgn
+
+    def modulus(self):
+        r"""
+            Calculation of modulus for a given tensor
+        """
+        return self.signum() * self
+
     def reciprocal(self, method="NR", nr_iters=10):
         r"""
         Calculate the reciprocal using the algorithm specified in the method args.
@@ -488,15 +505,17 @@ def reciprocal(self, method="NR", nr_iters=10):
         """
 
         if method.lower() == "nr":
-            result = 3 * (0.5 - self).exp() + 0.003
+            new_self = self.modulus()
+            result = 3 * (0.5 - new_self).exp() + 0.003
             for i in range(nr_iters):
-                result = 2 * result - result * result * self
-            return result
+                result = 2 * result - result * result * new_self
+            return result * self.signum()
         elif method.lower() == "division":
             ones = self * 0 + 1
             return ones / self
         elif method.lower() == "log":
-            return (-self.log()).exp()
+            new_self = self.modulus()
+            return (-new_self.log()).exp() * self.signum()
         else:
             raise ValueError(f"Invalid method {method} given for reciprocal function")
 
diff --git a/test/torch/tensors/test_precision.py b/test/torch/tensors/test_precision.py
index f88531c3a98..0be9e292efd 100644
--- a/test/torch/tensors/test_precision.py
+++ b/test/torch/tensors/test_precision.py
@@ -106,7 +106,7 @@ def test_methods_for_linear_module(method, parameter):
 def test_reciprocal(workers):
     bob, alice, james = (workers["bob"], workers["alice"], workers["james"])
 
-    tensor = torch.tensor([1.0, 2.0, 3.0])
+    tensor = torch.tensor([-2.0, 6.0, 2.0, 3.0, -5.0, -0.5])
     x = tensor.fix_prec()
     result = x.reciprocal(method="division").float_prec()
     assert torch.isclose(tensor.reciprocal(), result, rtol=1e-2).all()


Code review comments:
diff_hunk: @@ -468,6 +468,23 @@ def matmul(self, *args, **kwargs):
     __matmul__ = matmul
     mm = matmul
 
+    def signum(self):
+        r"""
+            Calculation of signum function for a given tensor
+        """
+        sgn = self > 0
+        zero = self == 0
+        pos = sgn
+        neg = sgn - 1
+        sgn = pos + neg + zero
+        return sgn
+
+    def modulus(self):
+        r"""
path: syft/frameworks/torch/tensors/interpreters/precision.py
text: @user1:
```suggestion
        """
```

Here too

@author:
okay

diff_hunk: @@ -468,6 +468,23 @@ def matmul(self, *args, **kwargs):
     __matmul__ = matmul
     mm = matmul
 
+    def signum(self):
+        r"""
path: syft/frameworks/torch/tensors/interpreters/precision.py
text: @user1:
```suggestion
        """
```
You don't need this unless you are using special notations/math in the docstrings

@author:
okay!

diff_hunk: @@ -468,6 +468,23 @@ def matmul(self, *args, **kwargs):
     __matmul__ = matmul
     mm = matmul
 
+    def signum(self):
+        r"""
+            Calculation of signum function for a given tensor
+        """
+        sgn = self > 0
+        zero = self == 0
+        pos = sgn
+        neg = sgn - 1
+        sgn = pos + neg + zero
path: syft/frameworks/torch/tensors/interpreters/precision.py
text: @user1:
```suggestion
        sgn = (self > 0) - (self < 0)
```

Seems a lot more intuitive, straightforward this way to me

@author:
agreed! lesser code this way