Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cphase comparison with CZ failing after dependency update #6663

Closed
NoureldinYosri opened this issue Jul 10, 2024 · 1 comment · Fixed by #6669
Closed

cphase comparison with CZ failing after dependency update #6663

NoureldinYosri opened this issue Jul 10, 2024 · 1 comment · Fixed by #6669
Labels
kind/bug-report Something doesn't seem to work. triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add

Comments

@NoureldinYosri
Copy link
Collaborator

Description of the issue
test following test case started failing at HEAD when run with the latest dependency versions

Cirq/cirq-core/cirq/ops/common_gates_test.py

Lines 902 to 904 in 32d4833

def test_parameterized_cphase():
assert cirq.cphase(sympy.pi) == cirq.CZ
assert cirq.cphase(sympy.pi / 2) == cirq.CZ**0.5

I temporarly added annotated it with xfail until we fix it.
@NoureldinYosri NoureldinYosri added kind/bug-report Something doesn't seem to work. triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Jul 10, 2024
@NoureldinYosri NoureldinYosri mentioned this issue Jul 10, 2024
Merged
@dstrain115
Copy link
Collaborator

dstrain115 commented Jul 17, 2024 •
edited
Loading

cirq cync: Comparison sympy.pi now no longer equals math.pi. Solution is unclear.
We could add a float() conversion, but this seems to defeat the purpose of the test.
(Possible solution: check equality against sympy.pi)

@dstrain115 dstrain115 added triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Jul 17, 2024
pavoljuhas added a commit to pavoljuhas/Cirq that referenced this issue Jul 19, 2024
@pavoljuhas
Fix equality for cphase gate initialized with sympy constant
5a24661 
Problem: In sympy-1.13.0 the `sympy.S.One == 1.0` and
`sympy.S.Half == 0.5` expressions evaluate to False which causes
inequality between `cirq.cphase(sympy.pi)` and `cirq.CZ`.

Solution: Check equality with parametrized exponents converted to float.

Fixes: quantumlib#6663
@pavoljuhas pavoljuhas mentioned this issue Jul 19, 2024
Merged
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
@pavoljuhas
Fix equality for cphase gate initialized with sympy constant (quantum…
1c18617 
…lib#6669)

Problem: In sympy-1.13.0 the expressions `sympy.S.One == 1.0` and
`sympy.S.Half == 0.5` evaluate to False which causes
inequality between `cirq.cphase(sympy.pi)` and `cirq.CZ`.

Solution: Check equality with parameterized exponents converted to float.

Fixes: quantumlib#6663
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/bug-report Something doesn't seem to work. triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix equality of cphase gates initialized from sympy constants pavoljuhas/Cirq
2 participants
@NoureldinYosri @dstrain115