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.

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

equal_up_to_global_phase() depends on the order of its arguments for EigenGates #5980

Closed
richrines1 opened this issue Jan 6, 2023 · 3 comments
Assignees
Labels
area/gates good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. 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

@richrines1
Copy link
Contributor

Description of the issue

the wrapping of the modulo operation in EigenGate._equal_up_to_global_phase_ makes it dependent on the sign of the difference between the exponents. As a result cirq.equal_up_to_global_phase() depends on the ordering of its arguments when the two gates' exponents differ by a negligible factor (see below)

i think all it would take to fix this is adding an abs() to the linked lines, e.g.:

canonical_diff = abs(exponents[0] - exponents[1]) % period
return np.isclose(canonical_diff, 0, atol=atol)

or alternatively replacing both with linalg.tolerance.near_zero_mod()

How to reproduce the issue

gate1 = cirq.Z
gate2 = cirq.Z ** (1 + 1e-10)

assert cirq.equal_up_to_global_phase(gate2, gate1, atol=1e-5)  # ok
assert cirq.equal_up_to_global_phase(gate1, gate2, atol=1e-5)  # fails

Cirq version

1.2.0.dev20230105212249

@richrines1 richrines1 added the kind/bug-report Something doesn't seem to work. label Jan 6, 2023
@tanujkhattar tanujkhattar added good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. area/gates triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add labels Jan 18, 2023
@web3quantum
Copy link

@tanujkhattar I would like to work on this issue.

@tanujkhattar
Copy link
Collaborator

@web3quantum Are you working on this issue? Do you have any questions or expected timeline by which you would be able to send a PR?

@siddharth-mehta
Copy link
Contributor

Hi, I was planning to work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gates good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. 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
Development

No branches or pull requests

4 participants