CircuitOperation proto #3891
CircuitOperation proto #3891
Conversation
95-martin-orion
requested review from
cduck,
vtomole,
wcourtney and
a team
as code owners
|
Initial prototypes for (de-)serializers using this format are promising as far as backwards-compatibility is concerned. I haven't been able to fully test them yet, but I'd like to get this solidified before committing too much time towards it. |
|
This PR is part of #3634. |
|
See this follow-up PR on my fork for how this looks when used in serialization. |
cirq/google/api/v2/program.proto
Outdated
| // Indicates that qubit "source" should be replaced with "target". | ||
| message QubitPair { | ||
| Qubit source = 1; | ||
| Qubit target = 2; |
There was a problem hiding this comment.
Sorry to nitpick, but I still feel like "source" and "target" is ambiguous. Is this saying that the qubit id appearing in the outer circuit is the source and should be replaced by the target value in the inner circuit, or the other way around? I'd really prefer to use something like "inner" and "outer" because the containment relationship is unambiguous (one circuit contains the other as an operation, not the other way around).
There was a problem hiding this comment.
Discussed offline: inner and outer only make sense in the context of CircuitOperation, where we have comments documenting the behavior of these maps.
Switched these to key/value to more closely align with the non-proto CircuitOperation object.
|
Holding out for approval from @dstrain115 prior to merging. |
| // of repetitions or a list of repetition IDs. | ||
| RepetitionSpecification repetition_specification = 2; | ||
|
|
||
| // Map from qubits in the "inner" circuit (referenced by |
There was a problem hiding this comment.
I am not sure I understand why it is called inner and outer circuit, but which one is which is clarified in the comment, so I guess it is fine.
95-martin-orion
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
|
Automerge cancelled: A required status check is not present. Missing statuses: ['Build docs', 'Build protos', 'Changed Notebooks Isolated Test against Cirq stable', 'Changed files test', 'Coverage check', 'Doc test', 'Format check', 'Lint check', 'Misc check', 'Notebook formatting', 'Pytest MacOS (3.7)', 'Pytest MacOS (3.8)', 'Pytest Ubuntu (3.7)', 'Pytest Ubuntu (3.8)', 'Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Type check', 'cla/google'] |
CirqBot
removed
automerge
balopat
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
|
Automerge cancelled: A required status check is not present. Missing statuses: ['cla/google'] |
CirqBot
removed
automerge
95-martin-orion
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
Follow up for #3891. Part of #3634. This PR adds the `CircuitOp(De)serializer` classes and updates existing behavior to handle them appropriately, including: - Addition of `Op(De)serializer` superclasses for operation (de-)serializers - Backwards-compatible conversion of `GateOp(De)serializer` fields to non-public, with public properties - Updates and deprecations for names made awkward by this change (e.g. `gate_type` -> `internal_type`) Changes **not** included in this PR: - Add `CircuitOperation` (de-)serializers to the default QCS gateset
Follow up for quantumlib#3891. Part of quantumlib#3634. This PR adds the `CircuitOp(De)serializer` classes and updates existing behavior to handle them appropriately, including: - Addition of `Op(De)serializer` superclasses for operation (de-)serializers - Backwards-compatible conversion of `GateOp(De)serializer` fields to non-public, with public properties - Updates and deprecations for names made awkward by this change (e.g. `gate_type` -> `internal_type`) Changes **not** included in this PR: - Add `CircuitOperation` (de-)serializers to the default QCS gateset
Follow up for quantumlib#3891. Part of quantumlib#3634. This PR adds the `CircuitOp(De)serializer` classes and updates existing behavior to handle them appropriately, including: - Addition of `Op(De)serializer` superclasses for operation (de-)serializers - Backwards-compatible conversion of `GateOp(De)serializer` fields to non-public, with public properties - Updates and deprecations for names made awkward by this change (e.g. `gate_type` -> `internal_type`) Changes **not** included in this PR: - Add `CircuitOperation` (de-)serializers to the default QCS gateset
Proposed changes to the v2 API to support CircuitOperations.
I've made an effort to keep this extensible, particularly regarding measurement keys and repetitions as both are expected to change in the future. Please let me know if you see any potential pain points for future changes in this space.