Support Cirq 1.7 change in exponents in circuit diagrams#1397
Open
mhucka wants to merge 3 commits into
Open
Conversation
In versions of Cirq below 1.7, `cirq.EigenGate._diagram_exponent()`
automatically canonicalized exponents into the interval (-period/2,
period/2]. Because `DoubleExcitationGate` has a diagram period of 2, an
exponent of `2.3` used to be canonicalized to `0.3` for diagram purposes
(because 2.3 mod 2 ≡ 0.3).
In the just-released Cirq 1.7.0, the canonicalization logic was removed
from `EigenGate._diagram_exponent()`. It now simply rounds the numeric
exponent to the specified precision without doing a modulo:
```python
def _diagram_exponent(self, args: protocols.CircuitDiagramInfoArgs):
if not isinstance(self._exponent, (int, float)):
return self._exponent
result = float(self._exponent)
if args.precision is not None:
result = np.around(result, args.precision).item()
return result
```
As a result, `DoubleExcitation(d, b, a, c) ** 2.3` is now rendered in
diagrams with `^2.3` instead of `^0.3`, which caused some assertions in
`four_qubit_gates_test.py` to fail.
Since we need to support both 1.7.0 and lower versions of Cirq, this PR
adds a step to put the exponent in the canonical form that was used
before. This seems like the best approach to preserve backward
compatibility and still work with the latest version of Cirq.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the _circuit_diagram_info_ method in four_qubit_gates.py to canonicalize the gate exponent into the (-1, 1] range. The reviewer noted that the type check isinstance(exponent, (int, float)) does not cover NumPy numeric types, which are common in this context, and suggested a simplified mathematical expression for the canonicalization logic.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In versions of Cirq below 1.7,
cirq.EigenGate._diagram_exponent()automatically canonicalized exponents into the interval (-period/2, period/2]. BecauseDoubleExcitationGatehas a diagram period of 2, an exponent of2.3used to be canonicalized to0.3for diagram purposes (because 2.3 mod 2 ≡ 0.3).In the just-released Cirq 1.7.0, the canonicalization logic was removed from
EigenGate._diagram_exponent()– I think it was in PR quantumlib/Cirq#7954. As a result,DoubleExcitation(d, b, a, c) ** 2.3is now rendered in diagrams with^2.3instead of^0.3. Some tests infour_qubit_gates_test.pycompared circuit diagrams, and those tests fail in Cirq 1.7.0Since we need to support both 1.7.0 and lower versions of Cirq, this PR adds a step to put the exponent in the canonical form that was used before. This seems like the best approach to preserve backward compatibility and still work with the latest version of Cirq.