OpenFermion icon indicating copy to clipboard operation
OpenFermion copied to clipboard

`SymbolicOperator.isclose` bug for operators with many terms?

Open peterse opened this issue 3 years ago • 0 comments

the following lines of code (v1.3.0 release branch) implementing SymbolicOperator.isclose are doing something strange when comparing two operators:

if not (isinstance(a, sympy.Expr) or isinstance(b, sympy.Expr)):
    tol *= max(1, abs(a), abs(b))

This behavior means that operators x and y whose terms have large coefficients will satisfy x == y even when x - y > 0. Here is a minimal reproducing example

from openfermion import FermionOperator
x = FermionOperator("0^ 0")
y = FermionOperator("0^ 0")

# construct two identical operators up to some number of terms
num_terms_before_ineq = 30
for i in range(num_terms_before_ineq):
    x += FermionOperator(f" (10+0j) [0^ {i}]")
    y += FermionOperator(f" (10+0j) [0^ {i}]")
    
# add a final term that is equal within tol but gets missed by the isclose check
xfinal = FermionOperator(f" (1+0j) [0^ {num_terms_before_ineq + 1}]")
yfinal = FermionOperator(f" (2+0j) [0^ {num_terms_before_ineq + 1}]")
# these two terms are not equal within tol..
print(xfinal == yfinal) # >>> False
print(xfinal - yfinal) # (-1+0j) [0^ 31]

# ...but these two terms will be, because the `tol` argument to `isclose` was corrupted
x += xfinal
y += yfinal
print(x == y) # >>> True
print(x - y) # (-1+0j) [0^ 31]

you can tweak num_terms_before_ineq smaller and the output will correctly reflect that x and y are not equal within the desired tolerance.

Otherwise if this is the intended behavior (i.e. this comparison is meant to be equality within a relative tolerance w/r to magnitude of the coefficient vectors for x and y), the docstring for SymbolicOperator.isclose should be updated accordingly.

peterse avatar Jan 23 '22 20:01 peterse