Repository: OpenMined/PySyft
Pull request: fix comparison between FPT and AST
## Description
this PR fixes #4705 
comparison function of additive_share.py is modified.

## Affected Dependencies
none

## How has this been tested?
- test cases are added

## 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)
- [x] My changes are covered by tests


Problem statement:
Comparison between FPT and AST doesn't always works
## Description
Some cases of handling comparison between FixedPrecision and AdditiveSharingTensor are supported, but some are not. We should systematize this.

## How to Reproduce
```python
t1 = torch.tensor([1.2, 1]).fix_precision().share(*workers, crypto_provider=crypto_provider, protocol="fss")
t2 = torch.tensor([1.2, 1]).fix_precision()

t1 > t2 # FAILS but t1 < t2 works
```

## Stacktrace
```
AttributeError                            Traceback (most recent call last)
<ipython-input-10-c55d3fcd7179> in <module>
      2 t2 = torch.tensor([1.2, 1]).fix_precision()#.share(*workers, crypto_provider=crypto_provider, protocol="fss", requires_grad=True)
      3 
----> 4 t1 > t2

~/code/PySyft/syft/generic/frameworks/hook/hook.py in overloaded_native_method(self, *args, **kwargs)
    218                 # Send the new command to the appropriate class and get the response
    219                 method = getattr(new_self, method_name)
--> 220                 response = method(*new_args, **new_kwargs)
    221 
    222                 # For inplace methods, just directly return self

~/code/PySyft/syft/generic/frameworks/overload.py in _hook_method_args(self, *args, **kwargs)
     25 
     26             # Send it to the appropriate class and get the response
---> 27             response = attr(self, new_self, *new_args, **new_kwargs)
     28 
     29             # Put back SyftTensor on the tensors found in the response

~/code/PySyft/syft/frameworks/torch/tensors/interpreters/precision.py in __gt__(self, _self, other)
    821     def __gt__(self, _self, other):
    822         print("FPT gt", _self, other)
--> 823         result = _self.__gt__(other)
    824         return result.type(self.torch_dtype) * self.base ** self.precision_fractional
    825 

~/code/PySyft/syft/frameworks/torch/mpc/__init__.py in method(self, *args, **kwargs)
     33         def method(self, *args, **kwargs):
     34             f = protocol_store[(name, self.protocol)]
---> 35             return f(self, *args, **kwargs)
     36 
     37         return method

~/code/PySyft/syft/frameworks/torch/tensors/interpreters/additive_shared.py in __gt__(self, other)
    938     @crypto_protocol("fss")
    939     def __gt__(self, other):
--> 940         return (other + 1) <= self
    941 
    942     def ge(self, other):

~/code/PySyft/syft/generic/frameworks/hook/hook.py in overloaded_native_method(self, *args, **kwargs)
    156                     # arguments
    157                     if not isinstance(args[0].child, PointerTensor):
--> 158                         self = type(args[0].child)().on(self, wrap=True)
    159                         args = [args[0]]
    160                         return overloaded_native_method(self, *args, **kwargs)

AttributeError: 'dict' object has no attribute 'on'

```

Hints from issue discussion:

Commit message: fixed comparison in AST and added tests

Patch to review:
diff --git a/syft/frameworks/torch/tensors/interpreters/additive_shared.py b/syft/frameworks/torch/tensors/interpreters/additive_shared.py
index 56bb860b45b..fb2bfc273d0 100644
--- a/syft/frameworks/torch/tensors/interpreters/additive_shared.py
+++ b/syft/frameworks/torch/tensors/interpreters/additive_shared.py
@@ -929,7 +929,7 @@ def __gt__(self, other):
 
     @crypto_protocol("fss")
     def __gt__(self, other):
-        return (other + 1) <= self
+        return -self <= -(other + 1)
 
     def ge(self, other):
         return (self - other).positive()
diff --git a/test/torch/tensors/test_additive_shared.py b/test/torch/tensors/test_additive_shared.py
index 7b4d7eef033..39962f56b12 100644
--- a/test/torch/tensors/test_additive_shared.py
+++ b/test/torch/tensors/test_additive_shared.py
@@ -741,6 +741,17 @@ def test_eq(workers, protocol):
 
     assert (x == y).get().float_prec()
 
+    # for x as AST and  y as FPT
+    # we currently support this set of operation only for fss protocol
+    x = (
+        torch.tensor([-3.1])
+        .fix_prec()
+        .share(*args, crypto_provider=crypto_provider, protocol="fss")
+    )
+    y = torch.tensor([-3.1]).fix_prec()
+
+    assert (x == y).get().float_prec()
+
 
 @pytest.mark.parametrize("protocol", ["snn", "fss"])
 @pytest.mark.parametrize("force_preprocessing", [True, False])
@@ -802,6 +813,28 @@ def test_comp(workers, protocol, force_preprocessing):
     assert ((x > y).get().float_prec() == (t1 > t2)).all()
     assert ((x < y).get().float_prec() == (t1 < t2)).all()
 
+    # for x as AST and  y as FPT
+    # we currently support this set of operation only for fss protocol
+    t1 = torch.tensor([-2.1, 1.8])
+    t2 = torch.tensor([-3.1, 0.3])
+    x = t1.fix_prec().share(*args, crypto_provider=crypto_provider, protocol="fss")
+    y = t2.fix_prec()
+
+    assert ((x >= y).get().float_prec() == (t1 >= t2)).all()
+    assert ((x <= y).get().float_prec() == (t1 <= t2)).all()
+    assert ((x > y).get().float_prec() == (t1 > t2)).all()
+    assert ((x < y).get().float_prec() == (t1 < t2)).all()
+
+    t1 = torch.tensor([[-2.1, 1.8], [-1.1, -0.7]])
+    t2 = torch.tensor([[-3.1, 0.3], [-1.1, 0.3]])
+    x = t1.fix_prec().share(*args, crypto_provider=crypto_provider, protocol="fss")
+    y = t2.fix_prec()
+
+    assert ((x >= y).get().float_prec() == (t1 >= t2)).all()
+    assert ((x <= y).get().float_prec() == (t1 <= t2)).all()
+    assert ((x > y).get().float_prec() == (t1 > t2)).all()
+    assert ((x < y).get().float_prec() == (t1 < t2)).all()
+
 
 @pytest.mark.parametrize("protocol", ["snn", "fss"])
 def test_max(workers, protocol):


Code review comments:
diff_hunk: @@ -802,6 +813,28 @@ def test_comp(workers, protocol, force_preprocessing):
     assert ((x > y).get().float_prec() == (t1 > t2)).all()
     assert ((x < y).get().float_prec() == (t1 < t2)).all()
 
+    # for x as AST and  y as FPT
path: test/torch/tensors/test_additive_shared.py
text: @user1:
Q: Could you move those to another test function? (because this function would be called 2 times - one with protocol = "fss" and once with protocol "snn"). You can also parameterize that function with only "fss"

@author:
Done