fix: handle optional SymPy support and zero-pruning in SymbolicOperat… - #1452
fix: handle optional SymPy support and zero-pruning in SymbolicOperat…#1452anishbhujbal7 wants to merge 4 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces optional SymPy support for symbolic operators, allowing SymPy expressions, symbols, and basic types to be used as coefficients. It refactors the compress method to handle symbolic expressions and adds corresponding unit tests. The review feedback highlights a regression in the refactored compress method where small imaginary and real parts of SymPy expressions are no longer pruned, which could affect numerical stability and hermiticity. To address this, it is recommended to simplify the coefficients first and then apply the original pruning logic, as well as add a test case to verify this behavior.
| if HAS_SYMPY and isinstance(coeff, (sympy.Expr, sympy.Symbol, sympy.Basic)): | ||
| # SymPy symbolic handling | ||
| if coeff == 0 or coeff.is_zero is True: | ||
| continue | ||
|
|
||
| # Simplify the symbolic expression | ||
| simplified_coeff = sympy.simplify(coeff) | ||
| if simplified_coeff == 0 or simplified_coeff.is_zero is True: | ||
| continue | ||
|
|
||
| # Check if simplified expression evaluates to a float/number under abs_tol | ||
| if simplified_coeff.is_number: | ||
| try: | ||
| if abs(complex(simplified_coeff)) <= abs_tol: | ||
| continue | ||
| except (TypeError, ValueError): | ||
| pass | ||
|
|
||
| new_terms[term] = simplified_coeff | ||
| continue |
There was a problem hiding this comment.
The refactored compress method introduces a regression where small imaginary and real parts of SymPy expressions (both symbolic and numeric) are no longer pruned. The original implementation used sympy.simplify(sympy.im(coeff) <= abs_tol) == True to discard negligible imaginary/real parts, which is crucial for maintaining hermiticity and numerical stability in downstream calculations.
By simplifying the coefficient first and then applying the original pruning logic, we can fix the x - x zero-pruning issue while preserving the real/imaginary part pruning.
| if HAS_SYMPY and isinstance(coeff, (sympy.Expr, sympy.Symbol, sympy.Basic)): | |
| # SymPy symbolic handling | |
| if coeff == 0 or coeff.is_zero is True: | |
| continue | |
| # Simplify the symbolic expression | |
| simplified_coeff = sympy.simplify(coeff) | |
| if simplified_coeff == 0 or simplified_coeff.is_zero is True: | |
| continue | |
| # Check if simplified expression evaluates to a float/number under abs_tol | |
| if simplified_coeff.is_number: | |
| try: | |
| if abs(complex(simplified_coeff)) <= abs_tol: | |
| continue | |
| except (TypeError, ValueError): | |
| pass | |
| new_terms[term] = simplified_coeff | |
| continue | |
| if HAS_SYMPY and isinstance(coeff, sympy.Basic): | |
| simplified_coeff = sympy.simplify(coeff) | |
| if sympy.simplify(sympy.im(simplified_coeff) <= abs_tol) == True: | |
| simplified_coeff = sympy.re(simplified_coeff) | |
| if sympy.simplify(sympy.re(simplified_coeff) <= abs_tol) == True: | |
| simplified_coeff = 1j * sympy.im(simplified_coeff) | |
| if sympy.simplify(abs(simplified_coeff) <= abs_tol) != True: | |
| new_terms[term] = simplified_coeff | |
| continue |
References
- When refactoring numerical accumulation or operator transforms, preserve the existing multi-step thresholding/dropping logic (e.g., double thresholding) if bit-for-bit identical output is required to avoid breaking tight tests.
| def test_compress_sympy_coefficients(self): | ||
| import sympy | ||
|
|
||
| x = sympy.Symbol('x') | ||
|
|
||
| # Operator with x - x (evaluates to 0 on compress) | ||
| op = MockOperator1(((0, 1), (1, 0)), x - x) | ||
| op.compress() | ||
| self.assertEqual(len(op.terms), 0) |
There was a problem hiding this comment.
To prevent future regressions where small imaginary or real parts of symbolic expressions are not pruned during compression, we should add a test case that specifically verifies this behavior.
| def test_compress_sympy_coefficients(self): | |
| import sympy | |
| x = sympy.Symbol('x') | |
| # Operator with x - x (evaluates to 0 on compress) | |
| op = MockOperator1(((0, 1), (1, 0)), x - x) | |
| op.compress() | |
| self.assertEqual(len(op.terms), 0) | |
| def test_compress_sympy_coefficients(self): | |
| import sympy | |
| x = sympy.Symbol('x') | |
| # Operator with x - x (evaluates to 0 on compress) | |
| op = MockOperator1(((0, 1), (1, 0)), x - x) | |
| op.compress() | |
| self.assertEqual(len(op.terms), 0) | |
| # Operator with x + 1e-15j (imaginary part should be pruned if x is real) | |
| x_real = sympy.Symbol('x', real=True) | |
| op2 = MockOperator1(((0, 1), (1, 0)), x_real + 1e-15j) | |
| op2.compress() | |
| self.assertEqual(op2.terms[((0, 1), (1, 0))], x_real) |
|
Hi maintainers! The CI
All tests are passing ( |
1 participant
Summary of Changes
This PR fixes issues in
SymbolicOperatorandQubitOperatorwhen usingSymPyexpressions as coefficients, specifically regarding zero-pruning, tolerance filtering, and runtime dependency safety.Dependency Guarding (
HAS_SYMPY):sympyreferences with globalHAS_SYMPYguards to ensure minimal environments withoutsympyinstalled do not encounter runtime crashes during import or operations.Improved Zero-Pruning & Tolerance Filtering in
compress():compress()to checksimplified_coeff.is_zeroand handle relational/symbolic comparisons cleanly.abs_tol) usingsimplified_coeff.is_numberso symbolic expressions that evaluate to small numerical values are pruned as expected without breaking non-numeric symbolic terms.Symbolic Equivalence Testing:
qubit_operator_test.pyandsymbolic_operator_test.py) to evaluate symbolic expressions usingsympy.simplify()rather than relying on strict==identity, preventing false assertion failures on equivalent expressions likexvs1.0*x.Issue Addressed
Closes #1053
Testing & Verification
SymbolicOperatorandQubitOperatorwith SymPy enabled and disabled.pytest src/ -s --ignore=src/openfermion/testing/examples_test.py):