Update/New Cirq Parsing Capabilities #411
Conversation
Adds a few new built-in gate names along with their corresponding unitaries. Extends the default built-in cirq conversion dictionary to include the 24 single-qubit clifford gate names, along with a few other one and two-qubit gates that did not have support yet.
Fix the docstrings in a few modelpacks to properly reflect their contents.
Add a function for mapping from cirq objects to pygsti gate names.
Add a new class method that allows for instantiation of a pygsti circuit from a cirq one. Parsing the input and converting it layer-by-layer in pygsti format.
Add unit tests, one for conversion from cirq to pygsti, and another for conversion to openqasm.
Adds handling for implied idles and global idle specification when doing cirq to pygsti conversion. Also adds associated unit tests, and fixes a few aliasing problems with the cirq to pygsti name conversion.
Modifies the unitary_to_standard_gatename function to add the option for it to identify standard gate names for unitaries that match up to an overall global phase. Also can optionally return what that phase is if a match is found.
Add a fallback behavior which uses a new function for checking if a unitary corresponds to a built-in gate up to an overall phase to try and search for a matching gate if one is missing from the conversion dictionary. A warning is raised when this happens to alert the user.
Update the cirq integration demo notebook to include a demonstration of new cirq-to-pygsti conversion capabilities.
coreyostrove
requested review from
rileyjmurray,
a team,
tjproct and
kmrudin
as code owners
There was a problem hiding this comment.
Thanks for also including tutorial updates :)
| std_unitaries['Gx'] = std_unitaries['Gxpi2'] | ||
| std_unitaries['Gy'] = std_unitaries['Gypi2'] | ||
| std_unitaries['Gz'] = std_unitaries['Gzpi2'] | ||
|
|
||
| return std_unitaries | ||
|
|
||
|
|
||
| def unitary_to_standard_gatename(unitary): | ||
| def unitary_to_standard_gatename(unitary, up_to_phase = False, return_phase = False): |
There was a problem hiding this comment.
Nice, we've had some confusion about the phase difference before. Great to make this explicit.
test/unit/objects/test_circuit.py
Outdated
There was a problem hiding this comment.
Thank you for the tests! :)
pygsti/circuits/circuit.py
Outdated
| implied idles as part of a circuit layer containing | ||
| other explicitly specified gates. | ||
|
|
||
| global_idle : bool or string or Label, optional (default False) |
There was a problem hiding this comment.
A question/clarification on implicit vs global idle behavior. The default is False for both, but this results in Gi's being dropped for implicit idles everywhere EXCEPT for global idles - that is, in cell 17 of the example, we have Gi labels in partial idle layers, but the global idle layer does have Gis. I kinda expected no Gis anywhere in this case.
Maybe more succintly put, I expected the "True" behavior of global idle to be the default (i.e. cell 20), and to recover the current (i.e. cell 17) behavior, a user would have to explicitly put "Gi" for the global_idle string here. Another advantage of this approach is then the global_idle argument only takes one type of argument (a label-like one) rather than switching behavior on boolean/label-like arguments.
I am open to being convinced that current behavior is most desired though!
P.S. Another place where Jupytext examples/tutorials would probably make this discussion easier...
There was a problem hiding this comment.
Yeah, I was conflicted on the default behavior here. The original decision to make replacement of the global idle layers non-default is that unlike with implicit idles where the convention is pretty universally accepted (i.e. not including them), the convention for global idles is less so. I know we have Label(()), but I've always kind of disliked this convention (from a dev perspective it is kind of annoying and creates lots of edge cases to check for), so I initially wanted to force the choice of substitution to be an affirmative one. As you can see from the final implementation I eventually flip-flopped on this and did add a default label replacement choice of Label(()), so I think making this default would be alright. I'll probably change the names of these kwargs to be more descriptive while I'm at it.
Rename the kwargs for implied idle and global idle handling and change the default behavior of global idle handling to convert these by default. Updates the tutorial notebook and unit tests to reflect these changes.
|
Thanks for the feedback, @sserita. I have just pushed some updates which rename the kwargs of implied and global idle handling to |
This PR updates the existing parser for conversion of pyGSTi Circuit objects to Cirq Circuit objects. Also introduced is a new parser for converting from Cirq Circuits objects into pyGSTi Circuit objects. A few other minor changes include:
Updated the function
unitary_to_standard_gatenameintools.internalgatesto allow the option for checking for equivalence of unitaries to a built-in gate up to an overall global phase.New unit tests (including one for qasm conversion, which was previously not unit tested) and updates to the cirq integration demo notebook to include the new functionality.
Note: I branched this off of #403, which is why there is so much testing update stuff in the commit log (once that gets merged in it should go away from this PR), stuff relevant to this update begins 3/13/24 w/ 185a59a.