Change names of parameters to isclose() and deprecate tol

Per comments in PR review, this renames `rel_tol` and `abs_tol` to be
`rtol` and `atol`, respectively, as well ask keeps the previous `tol`
parameter but marks it as deprecated.

Author: mhucka

src/openfermion/ops/operators/symbolic_operator.py

@@ -605,7 +605,7 @@ def __pow__(self, exponent):
 
     def __eq__(self, other):
         """Approximate numerical equality (not true equality)."""
-        return self.isclose(other, rel_tol=EQ_TOLERANCE, abs_tol=EQ_TOLERANCE)
+        return self.isclose(other)
 
     def __ne__(self, other):
         return not (self == other)
@@ -618,7 +618,7 @@ def __next__(self):
         term, coefficient = next(self._iter)
         return self.__class__(term=term, coefficient=coefficient)
 
-    def isclose(self, other, rel_tol=EQ_TOLERANCE, abs_tol=EQ_TOLERANCE):
+    def isclose(self, other, tol=None, rtol=EQ_TOLERANCE, atol=EQ_TOLERANCE):
         """Check if other (SymbolicOperator) is close to self.
 
         Comparison is done for each term individually. Return True
@@ -627,36 +627,55 @@ def isclose(self, other, rel_tol=EQ_TOLERANCE, abs_tol=EQ_TOLERANCE):
 
         Args:
             other(SymbolicOperator): SymbolicOperator to compare against.
-            rel_tol(float): Relative tolerance.
-            abs_tol(float): Absolute tolerance.
+            tol(float): This parameter is deprecated since version 1.8.0.
+                Use `rtol` and/or `atol` instead. If `tol` is provided, it
+                is used as the value of `atol`.
+            rtol(float): Relative tolerance used in comparing each term in
+                self and other.
+            atol(float): Absolute tolerance used in comparing each term in
+                self and other.
         """
         if not isinstance(self, type(other)):
             return NotImplemented
 
+        if tol is not None:
+            if rtol != EQ_TOLERANCE or atol != EQ_TOLERANCE:
+                raise ValueError(
+                    'Parameters rtol and atol are mutually exclusive with the'
+                    ' deprecated parameter tol; use either tol or the other two,'
+                    ' not in combination.'
+                )
+            warnings.warn(
+                'Parameter tol is deprecated. Use rtol and/or atol instead.',
+                DeprecationWarning,
+                stacklevel=2,  # Identify the location of the warning.
+            )
+            atol = tol
+
         # terms which are in both:
         for term in set(self.terms).intersection(set(other.terms)):
             a = self.terms[term]
             b = other.terms[term]
             if isinstance(a, sympy.Expr) or isinstance(b, sympy.Expr):
-                if self._issmall(a - b, abs_tol) is False:
+                if self._issmall(a - b, atol) is False:
                     return False
-            elif not abs(a - b) <= abs_tol + rel_tol * max(abs(a), abs(b)):
+            elif not abs(a - b) <= atol + rtol * max(abs(a), abs(b)):
                 return False
-        # terms only in one (compare to 0.0 so only abs_tol)
+        # terms only in one (compare to 0.0 so only atol)
         for term in set(self.terms).symmetric_difference(set(other.terms)):
             if term in self.terms:
                 coeff = self.terms[term]
                 if isinstance(coeff, sympy.Expr):
-                    if self._issmall(coeff, abs_tol) is False:
+                    if self._issmall(coeff, atol) is False:
                         return False
-                elif not abs(coeff) <= abs_tol:
+                elif not abs(coeff) <= atol:
                     return False
             else:
                 coeff = other.terms[term]
                 if isinstance(coeff, sympy.Expr):
-                    if self._issmall(coeff, abs_tol) is False:
+                    if self._issmall(coeff, atol) is False:
                         return False
-                elif not abs(coeff) <= abs_tol:
+                elif not abs(coeff) <= atol:
                     return False
         return True
 

src/openfermion/ops/operators/symbolic_operator_test.py

@@ -860,7 +860,48 @@ def test_pow_high_term(self):
         term = DummyOperator1(ops, coeff)
         high = term**10
         expected = DummyOperator1(ops * 10, coeff**10)
-        self.assertTrue(high.isclose(expected, rel_tol=1e-12, abs_tol=1e-12))
+        self.assertTrue(high.isclose(expected, rtol=1e-12, atol=1e-12))
+
+    def test_isclose_parameter_deprecation(self):
+        op1 = DummyOperator1('0^ 1', 1.0)
+        op2 = DummyOperator1('0^ 1', 1.001)
+
+        with self.assertWarns(DeprecationWarning):
+            op1.isclose(op2, tol=0.01)
+
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", category=DeprecationWarning)
+            self.assertTrue(op1.isclose(op2, tol=0.001))
+            self.assertFalse(op1.isclose(op2, tol=0.0001))
+
+    def test_isclose_parameter_combos(self):
+        op1 = DummyOperator1('0^ 1', 1.0)
+        op2 = DummyOperator1('0^ 1', 1.001)
+
+        with self.assertRaises(ValueError):
+            op1.isclose(op2, tol=0.01, rtol=1e-5)
+
+        with self.assertRaises(ValueError):
+            op1.isclose(op2, tol=0.01, atol=1e-5)
+
+    def test_isclose_atol_rtol(self):
+        op1 = DummyOperator1('0^ 1', 1.0)
+        op2 = DummyOperator1('0^ 1', 1.001)
+
+        op_a = DummyOperator1('0^ 1', 1.0)
+        op_b = DummyOperator1('0^ 1', 1.001)
+        self.assertTrue(op_a.isclose(op_b, atol=0.001))
+        self.assertFalse(op_a.isclose(op_b, atol=0.0001))
+
+        op_c = DummyOperator1('0^ 1', 1000)
+        op_d = DummyOperator1('0^ 1', 1001)
+        self.assertTrue(op_c.isclose(op_d, rtol=0.001))
+        self.assertFalse(op_c.isclose(op_d, rtol=0.0001))
+
+        op_e = DummyOperator1('0^ 1', 1.0)
+        op_f = DummyOperator1('0^ 1', 1.001)
+        self.assertTrue(op_e.isclose(op_f, rtol=1e-4, atol=1e-3))
+        self.assertFalse(op_e.isclose(op_f, rtol=1e-4, atol=1e-5))
 
     def test_isclose(self):
         op1 = DummyOperator1()
@@ -869,8 +910,8 @@ def test_isclose(self):
         op1 += DummyOperator1('2^ 3', 1)
         op2 += DummyOperator1('0^ 1', 1000000)
         op2 += DummyOperator1('2^ 3', 1.001)
-        self.assertFalse(op1.isclose(op2, abs_tol=1e-4))
-        self.assertTrue(op1.isclose(op2, abs_tol=1e-2))
+        self.assertFalse(op1.isclose(op2, atol=1e-4))
+        self.assertTrue(op1.isclose(op2, atol=1e-2))
 
         # Case from https://github.com/quantumlib/OpenFermion/issues/764
         x = FermionOperator("0^ 0")