Skip to content
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

Classical control #4631

Merged
merged 76 commits into from
Dec 8, 2021
Merged

Classical control #4631

merged 76 commits into from
Dec 8, 2021

Conversation

daxfohl
Copy link
Contributor

@daxfohl daxfohl commented Nov 5, 2021

Sits on top of #4627

Creates ConditionalOperation class and executes operations conditionally upon the classical bits. Most of this is done in commit 06883d5.

Reimplements quantum teleportation example based off this class.

Parts 8, 9, 10 of https://tinyurl.com/cirq-feedforward.

Copy link
Collaborator

@95-martin-orion 95-martin-orion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. I think the only remaining items are the discussion on what to call this operation (to be covered at Cirq sync today) and the "controlled CircuitOp which contains a measurement" test case. After these are done, we can merge.

@daxfohl
Copy link
Contributor Author

daxfohl commented Dec 2, 2021

It looks like IfOperation won. Are we keeping with_conditions() and unconditionally()?

@95-martin-orion
Copy link
Collaborator

It looks like IfOperation won. Are we keeping with_conditions() and unconditionally()?

We still need to confirm with the internal error-correction team on the name, but either way I think those methods are still reasonable names.

def __init__(
self,
sub_operation: 'cirq.Operation',
conditions: Sequence[Union[str, 'cirq.MeasurementKey']],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the meaning of multiple conditions and measurement keys that correspond to more than one value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add some documentation here.

Multiple conditions are treated as and, so chaining op.with_conditions('a', 'b').with_conditions('c', 'd') is equivalent to op.with_conditions('a', 'b', 'c', 'd').

If a measurement was of multiple qubits, all zeros is considered False, anything else is considered True.

There's a subsequent PR that will allow sympy expressions to be conditions, like 'mkey1 | mkey2', or 'mkey1 > 20'. That will enable or conditions, and measurements-with-multiple-qubits to be used as ints,

@95-martin-orion
Copy link
Collaborator

Between the Cirq sync and internal review, ClassicallyControlledOperation appears to be the preferred name for this. Let's update to use that name and get this merged.

@daxfohl
Copy link
Contributor Author

daxfohl commented Dec 6, 2021

@95-martin-orion I went ahead and updated the function names to with(/out)_classical_controls to match. LMK if you prefer reverting to with_conditions.

I added tests for subcircuits with control keys based on external measurements as well. They work, but I wouldn't say they're supported yet. As you can see in the tests, they require special handling during construction, since the control_keys protocol doesn't yet expose unbound control keys of subcircuits, so the logic in append doesn't detect the key conflict and pushes that subcircuit to the first moment by default. That is all taken care of in the scope PR.

Also I promised Dave some pydocs above. I'll add those.

@daxfohl
Copy link
Contributor Author

daxfohl commented Dec 6, 2021

Pydocs, added. Ready for review.

Copy link
Collaborator

@95-martin-orion 95-martin-orion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM. Pinging @dabacon for his input on the docstrings, otherwise this is ready to go.

@95-martin-orion
Copy link
Collaborator

Going ahead with merge to keep these moving.

@95-martin-orion 95-martin-orion added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Dec 8, 2021
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Dec 8, 2021
@CirqBot CirqBot merged commit e97768c into quantumlib:master Dec 8, 2021
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Dec 8, 2021
@daxfohl daxfohl deleted the cbit branch December 8, 2021 23:27
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
Sits on top of quantumlib#4627

Creates `ConditionalOperation` class and executes operations conditionally upon the classical bits. Most of this is done in commit quantumlib@06883d5.

Reimplements quantum teleportation example based off this class.

Parts 8, 9, 10 of https://tinyurl.com/cirq-feedforward.
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
Sits on top of quantumlib#4627

Creates `ConditionalOperation` class and executes operations conditionally upon the classical bits. Most of this is done in commit quantumlib@06883d5.

Reimplements quantum teleportation example based off this class.

Parts 8, 9, 10 of https://tinyurl.com/cirq-feedforward.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining. size: L 250< lines changed <1000
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants