-
Notifications
You must be signed in to change notification settings - Fork 1.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
Merge serializable_forms and deserialized_forms #6520
Merge serializable_forms and deserialized_forms #6520
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6520 +/- ##
==========================================
- Coverage 97.78% 97.78% -0.01%
==========================================
Files 1105 1105
Lines 95109 95109
==========================================
- Hits 93000 92999 -1
- Misses 2109 2110 +1 ☔ View full report in Codecov by Sentry. |
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.
This seems fine to me.
* Merge serializable_forms and deserialized_forms * Documentation for adding gates * Update documentation for adding CompilationTargetGatesets
This PR simplifies the experience of updating the supported gateset for
GridDevice
.The original reasons of separating of
serializable_forms
anddeserialized_forms
were twofolds:FSimGateFamily(gates_to_accept=[cirq.CZ])
does not acceptcirq.GateFamily(cirq.CZ)
, and similarly for other 2q gates.FSimGateFamily(gates_to_accept=[cirq.CZ])
andcirq.GateFamily(cirq.CZ)
were considered redundant for accepting the CZ gate).cirq.PhasedXZGate
instead ofcirq.GateFamily(cirq.PhasedXZGate)
).I prefer to merge these fields now because
cirq.GateFamily(cirq.CZ)
shouldn't be accepted by the gateset. Although rare, I don't think this is what we want.cc @wcourtney