Skip to content

Conversation

@verult
Copy link
Collaborator

@verult verult commented Mar 22, 2024

This PR simplifies the experience of updating the supported gateset for GridDevice.

The original reasons of separating of serializable_forms and deserialized_forms were twofolds:

  1. FSimGateFamily(gates_to_accept=[cirq.CZ]) does not accept cirq.GateFamily(cirq.CZ), and similarly for other 2q gates.
  2. The deserialized gateset should be as simple as possible to accept all valid gates, i.e.:
  • It should contain the minimum set of gates / GateFamilies to accept all valid gates (for example FSimGateFamily(gates_to_accept=[cirq.CZ]) and cirq.GateFamily(cirq.CZ) were considered redundant for accepting the CZ gate).
  • When possible, prefer gate types over GateFamilies (e.g. cirq.PhasedXZGate instead of cirq.GateFamily(cirq.PhasedXZGate)).

I prefer to merge these fields now because

  1. Having a single field is much easier for developers who wants to update the gateset to understand.
  2. The original design decision was that cirq.GateFamily(cirq.CZ) shouldn't be accepted by the gateset. Although rare, I don't think this is what we want.
  3. Adding GateFamilies instead of gate types is not much more complicated, and is valuable if it simplifies the experience of updating the gateset.

cc @wcourtney

@verult verult requested review from a team, cduck, vtomole and wcourtney as code owners March 22, 2024 18:38
@codecov
Copy link

codecov bot commented Mar 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.78%. Comparing base (decf16d) to head (d81a17d).
Report is 2 commits behind head on main.

❗ Current head d81a17d differs from pull request most recent head d037f0f. Consider uploading reports for the commit d037f0f to get more accurate results

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.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@dstrain115 dstrain115 left a 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.

@verult verult enabled auto-merge (squash) March 22, 2024 20:40
@verult verult merged commit edd8393 into quantumlib:main Mar 22, 2024
jselig-rigetti pushed a commit to jselig-rigetti/Cirq that referenced this pull request May 28, 2024
* Merge serializable_forms and deserialized_forms

* Documentation for adding gates

* Update documentation for adding CompilationTargetGatesets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants