Add control_keys protocol #4490

daxfohl · 2021-09-08T17:46:32Z

Use control keys in circuit construction to block controlled gates "earliest" append method from falling back to before the measurement.

Parts 5&7 of https://tinyurl.com/cirq-feedforward. (I grouped them together in a single PR so that circuit.append could be a POC that control_keys protocol design works as intended).

Replaces #4466 since underlying measurement_keys has changed.

Use control keys in circuit construction to block controlled gates "earliest" append method from falling back to before the measurement.

cirq-core/cirq/circuits/circuit_test.py

daxfohl · 2021-09-08T20:06:06Z

@95-martin-orion This is ready for review

cirq-core/cirq/circuits/circuit_operation.py

95-martin-orion · 2021-09-09T17:26:38Z

cirq-core/cirq/circuits/circuit.py

+    def _control_key_names_(self):
+        return {key for op in self.all_operations() for key in protocols.control_key_names(op)}


[No change needed] We'll need to be careful about this - the circuit itself isn't controlled, just the gates inside it.

Yeah, my initial thought was to have this return only external keys like CircuitOperations. But for diagramming we'll need a list of all the control keys so we can draw the creg lines.

That said, I agree it's odd that the Circuit would have different list than the CircuitOperation. Maybe it's best to have them both hide internal keys, and the diagramming tool can just iterate the circuit's ops and aggregate the control key names manually.

(Note to self: I think it's better to have the two be consistent, or people will get confused. Probably best to provide an explicit include_internal flag in control_key_names()).

I don't think we should be exposing internal cbits as control_keys. This would really make the semantics confusing as you noted. control_keys for any entity (Operation, CircuitOperation or Circuit) should mean "The classical bits that need to be True for this entity to run". So internal keys are not really part of this. We can have a separate protocol to get the internal keys but I don't see what purpose it can serve that cannot already be served by measurement_keys protocol.

The diagram can iterate through internal operations to figure out which of the measurement keys we need to draw a creg for.

95-martin-orion · 2021-09-09T17:30:40Z

cirq-core/cirq/protocols/control_key_protocol.py

+from cirq._doc import doc_private
+
+
+class SupportsControlKey(Protocol):


Clean up docstrings in this file - it looks like there was a copy & replace from measurement keys, but the meaning doesn't line up.

daxfohl · 2021-09-09T20:48:21Z

cirq-core/cirq/circuits/circuit_operation.py

            k = value.MeasurementKey.parse_serialized(k).name
            k_new = self.measurement_key_map.get(k, k)
            k_new = key_map.get(k_new, k_new)
            if k_new != k:
                new_map[k] = k_new
        new_op = self.replace(measurement_key_map=new_map)
-        if len(new_op._measurement_key_names_()) != len(self._measurement_key_names_()):
+        if len(new_op._measurement_key_names_()) != len(self._measurement_key_names_()) or len(


I think we also need a check that this doesn't cause any new conflicts between control keys and measurement keys. Maybe check len(measurement_keys & control_keys) is unchanged. I'm pretty sure we need all three checks. Write some unit tests that cover each case.

smitsanghavi · 2021-09-13T02:51:16Z

I think there is a conflation between some slightly different sets of classical data here that's causing confusion.

Measurement keys that control the operation (or Circuit or CircuitOperation) . This is the set of classical bits that all have to be True for the operation to run, else it doesn't run at all. This is analogous to the controls in ControlledOperation. This is case where the name control_keys fits best. In the current implementation, most higher-level entities (anything except regular Operations) do not return this.
Measurement keys that this operation (or Circuit or CircuitOperation) creates/produces. measurement_keys remains the accurate method to capture these.
Measurement keys that this operation (or Circuit or CircuitOperation) uses (but does not produce). These are the "external control keys" in the discussions above. This is what CircuitOperation.control_key_names currently returns.
A union of 2 and 3. Any measurement keys that this entity produces or uses. This is analogous to a classical version of qubits protocol. Currently this is what control_keys tries to return for Circuits. I think this should be a separate cbits protocol.

If we need to compute "external control keys" (type 3 above) anywhere, we can just resort to set_difference(cbits, measurement_keys).

cirq-core/cirq/circuits/circuit_operation.py

smitsanghavi · 2021-09-13T02:45:20Z

cirq-core/cirq/circuits/circuit.py

+    def _control_key_names_(self):
+        return {key for op in self.all_operations() for key in protocols.control_key_names(op)}


I don't think we should be exposing internal cbits as control_keys. This would really make the semantics confusing as you noted. control_keys for any entity (Operation, CircuitOperation or Circuit) should mean "The classical bits that need to be True for this entity to run". So internal keys are not really part of this. We can have a separate protocol to get the internal keys but I don't see what purpose it can serve that cannot already be served by measurement_keys protocol.

The diagram can iterate through internal operations to figure out which of the measurement keys we need to draw a creg for.

daxfohl · 2021-09-13T19:34:21Z

@smitsanghavi This is a good breakdown. Actually what I am shooting for in the control_keys protocol is a union of types 1 and 3, to be interpreted as "the control keys that need to be defined prior to running this operation". Or in other words, the classical inputs to this function/operation/subcircuit. This corresponds to measurement_keys protocol as being the classical outputs of the same.

I'm interested to understand more about the CBits augmentation. Are you proposing a new class to represent them or just the protocol? One big difference I see between qubits and cbits is that cbits can be dynamically allocated. They aren't yet, but as we start introducing flow control features like repeat-until, there may be no way to a-priori determine a specific number of cbits in a circuit.

So I was hoping to keep feedforward on the simple side; just base it on measurement keys until some of the other issues like flow control, IQ points, non-unique measurements were better fleshed out. Ultimately I think we'll need something like CValue that can be a range of different data types or lists, and have measurement operations like "xor", "and", "or", "sum", "concat", etc (#4274 (comment)) that operate on different subtypes of CValue. But that's all obviously very nebulous and hand wavy at this point.

95-martin-orion · 2021-09-14T16:59:08Z

Discussed in the classical control sync: control_keys should continue returning types 1 and 3, as a parallel to measurement_keys which has a similar behavior. A separate controlled_by method which returns only type 1 will be introduced as part of the ClassicallyControlledOperation class in a subsequent operation.

daxfohl · 2021-11-01T18:58:26Z

cirq-core/cirq/circuits/circuit.py

        while k > 0:
            k -= 1
-            if not self._can_commute_past(k, op):
+            moment = self._moments[k]
+            if moment.operates_on(op_qubits) or any(


any is not the correct thing here. Use len(...)==0, or bool(...), or just nothing.

daxfohl · 2021-11-01T19:27:51Z

Closing for now in favor of #4610. Will revisit extern control keys later.

Use control keys in circuit construction to block controlled gates "earliest" append method from falling back to before the measurement. Parts 5&7 of https://tinyurl.com/cirq-feedforward. (I grouped them together in a single PR so that circuit.append could be a POC that control_keys protocol design works as intended). Replaces/simplifies #4490 since we will handle "extern" control keys in subcircuits as a separate PR.

Use control keys in circuit construction to block controlled gates "earliest" append method from falling back to before the measurement. Parts 5&7 of https://tinyurl.com/cirq-feedforward. (I grouped them together in a single PR so that circuit.append could be a POC that control_keys protocol design works as intended). Replaces/simplifies quantumlib#4490 since we will handle "extern" control keys in subcircuits as a separate PR.

daxfohl added 13 commits September 8, 2021 10:28

Remove decomposition in measurement_key_protocol

e481ea3

Simplify, removing unused method.

6704ed0

mypy

a42e8dd

Add control key protocol

c561e7f

Use control keys in circuit construction to block controlled gates "earliest" append method from falling back to before the measurement.

repr spec

f13453a

Prevent infinite recursion

5a43839

infinite recur

7773669

Prevent infinite recursion, fix decompose check

8bfa633

Prevent infinite recursion in test

a481b2e

Fix tests

efa72ce

Fix tests

731a1de

remove unused method

e056ee7

remove merge error

07af7fe

CirqBot added the size: M 50< lines changed <250 label Sep 8, 2021

google-cla bot added the cla: yes Makes googlebot stop complaining. label Sep 8, 2021

daxfohl mentioned this pull request Sep 8, 2021

Add control_keys protocol #4466

Closed

daxfohl commented Sep 8, 2021

View reviewed changes

cirq-core/cirq/circuits/circuit_test.py Show resolved Hide resolved

daxfohl added 4 commits September 8, 2021 10:52

fix lint

641f838

subcircuits

afad4b0

new test

2bf65aa

remove unused

4dddc05

daxfohl marked this pull request as ready for review September 8, 2021 20:05

daxfohl requested review from cduck, vtomole and a team as code owners September 8, 2021 20:05

daxfohl requested a review from mpharrigan September 8, 2021 20:05

low level tests

fc652e3

tanujkhattar assigned 95-martin-orion Sep 9, 2021

95-martin-orion requested changes Sep 9, 2021

View reviewed changes

daxfohl commented Sep 9, 2021

View reviewed changes

smitsanghavi suggested changes Sep 13, 2021

View reviewed changes

daxfohl requested review from ColemanCollins, dabacon, gmauricio, HGSilveri, ma5x, mrwojtek, nakardo, pschindler and wcourtney as code owners October 28, 2021 19:57

daxfohl force-pushed the control_keys3 branch from e56b2fe to fc652e3 Compare October 28, 2021 19:58

Merge branch 'master' into ckmerge

fc9f93d

daxfohl commented Nov 1, 2021

View reviewed changes

daxfohl mentioned this pull request Nov 1, 2021

Add control_keys protocol (simplified) #4610

Merged

daxfohl closed this Nov 1, 2021

daxfohl deleted the control_keys3 branch November 4, 2021 17:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add control_keys protocol #4490

Add control_keys protocol #4490

daxfohl commented Sep 8, 2021

daxfohl commented Sep 8, 2021

95-martin-orion Sep 9, 2021

daxfohl Sep 9, 2021

daxfohl Sep 10, 2021

smitsanghavi Sep 13, 2021

95-martin-orion Sep 9, 2021

daxfohl Sep 9, 2021

smitsanghavi commented Sep 13, 2021 •

edited

Loading

smitsanghavi Sep 13, 2021

daxfohl commented Sep 13, 2021

95-martin-orion commented Sep 14, 2021

daxfohl Nov 1, 2021

daxfohl commented Nov 1, 2021

		def _control_key_names_(self):
		return {key for op in self.all_operations() for key in protocols.control_key_names(op)}

		from cirq._doc import doc_private


		class SupportsControlKey(Protocol):

Add control_keys protocol #4490

Add control_keys protocol #4490

Conversation

daxfohl commented Sep 8, 2021

daxfohl commented Sep 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smitsanghavi commented Sep 13, 2021 • edited Loading

Choose a reason for hiding this comment

daxfohl commented Sep 13, 2021

95-martin-orion commented Sep 14, 2021

Choose a reason for hiding this comment

daxfohl commented Nov 1, 2021

smitsanghavi commented Sep 13, 2021 •

edited

Loading