-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add newly serializable gates to supported grid device gates #6499
Add newly serializable gates to supported grid device gates #6499
Conversation
@verult @wcourtney is this what is needed to pass validation steps like ?
|
This validation happens client side using the gate set provided on the device description:
The gateset is currently just a static superset of all supported gates, configured here, but should eventually rely on the gates specific to the device config. |
I think we are talking about different things, the same gate/gateset can have different preresentations in Cirq. I think this is why we have Cirq/cirq-google/cirq_google/devices/grid_device.py Lines 75 to 88 in 0f4822b
Cirq/cirq-google/cirq_google/devices/grid_device.py Lines 100 to 101 in 0f4822b
A gate failes validation if it's not a member of the Cirq/cirq-google/cirq_google/devices/grid_device.py Lines 251 to 260 in 2700f95
This PR just says that cirq.I, cirq.HPowGate, cirq.ops.SinqleQubitCliffordGate can be passed along as XZ gates, we can change this later if we want. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6499 +/- ##
==========================================
- Coverage 97.76% 97.76% -0.01%
==========================================
Files 1105 1105
Lines 95130 95030 -100
==========================================
- Hits 93001 92902 -99
+ Misses 2129 2128 -1 ☔ View full report in Codecov by Sentry. |
Looking at the only usage of @verult has more context about the gateset design |
IIRC the main reason why the two fields are distinct is that Apologies for the confusion - I'm planning to add documentation for adding gates soon. |
@verult PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last bit of change, everything else lgtm
@@ -118,11 +118,12 @@ class _GateRepresentations: | |||
cirq.YPowGate, | |||
cirq.PhasedXPowGate, | |||
cirq.HPowGate, | |||
cirq.IdentityGate, | |||
cirq.GateFamily(cirq.I), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Just cirq.I
would be good, to be consistent with the other deserialized_forms
. These are what we want to display in the gateset object, so here we keep it as simple as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cirq.I is an object (an instance of cirq.IdentityGate), the others are classes. when I use cirq.I the code breaks because it assumes classes.
followup to #6479