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

GridDevice: Exclude MeasurementGates in validation of qubit pairs #5654

Merged

Conversation

verult
Copy link
Collaborator

@verult verult commented Jun 30, 2022

I'm comfortable with special-casing MeasurementGate because it's the only gate today with the property that it can be applied to any subset of qubits.

Fixes #5652

@maffoo

@verult verult requested a review from maffoo June 30, 2022 00:27
@verult verult requested review from wcourtney, a team, vtomole and cduck as code owners June 30, 2022 00:27
@CirqBot CirqBot added the size: S 10< lines changed <50 label Jun 30, 2022
cirq.measure(
device_info.grid_qubits[2 * i + 1], device_info.grid_qubits[2 * (i + 1) + 1]
)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to add some test cases of measurement on more qubits, for example triplets of qubits or all qubits (cirq.measure(*device_info.grid_qubits)).

@verult verult added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 6, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 6, 2022
@CirqBot
Copy link
Collaborator

CirqBot commented Jul 6, 2022

Automerge cancelled: A required status check is not present.

Missing statuses: ['Coverage check', 'Doc test', 'Notebook formatting', 'Pytest MacOS (3.7)', 'Pytest MacOS (3.8)', 'Pytest MacOS (3.9)', 'Pytest Ubuntu (3.7)', 'Pytest Ubuntu (3.8)', 'Pytest Ubuntu (3.9)', 'Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Pytest Windows (3.9)', 'Typescript lint check', 'Typescript tests', 'Typescript tests coverage']

@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 Jul 6, 2022
@dabacon
Copy link
Collaborator

dabacon commented Jul 6, 2022

What about identity gate, does it also need to be allowed? (Yes odd to do identity across multiple qubits, but I think it is technically a multiqubit gate. Reset isn't)

@verult
Copy link
Collaborator Author

verult commented Jul 6, 2022

Ah right. I left identity gate as a TODO which will be addressed soon in a separate PR. Will change the check to something like not any(operation in gf for gf in SUBSET_GATE_FAMILIES) in a separate PR

@dstrain115
Copy link
Collaborator

What about wait gate? Does that need to be handled here?

@verult
Copy link
Collaborator Author

verult commented Jul 7, 2022

@dstrain115 does this mean only single-qubit WaitGates are allowed by the CircuitSerializer since both num_qubits and qid_shape are None in the WaitGate constructor?

op = cirq.WaitGate(duration=cirq.Duration(nanos=total_nanos or 0.0))(*qubits)

@dstrain115
Copy link
Collaborator

It's been a while since I dealt with this, but I think wait gates are similar to measurement gates in that they can be any permutation of qubits. I think they were the PERMUTATION_SET in the old device spec.

@verult verult force-pushed the cg-device-refactor/measure-two-qubits branch from 43701fb to bbbfecb Compare July 7, 2022 23:23
@verult
Copy link
Collaborator Author

verult commented Jul 7, 2022

Added WaitGate support

Copy link
Contributor

@maffoo maffoo left a comment

Choose a reason for hiding this comment

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

I'd suggest using the term "variadic" to describe these gates, otherwise this looks good to me.

@@ -37,6 +37,9 @@
MEASUREMENT_GATE_FAMILY = cirq.GateFamily(cirq.MeasurementGate)
WAIT_GATE_FAMILY = cirq.GateFamily(cirq.WaitGate)

# Families of gates which can be applied to any subset of valid qubits.
_SUBSET_PERMUTATION_GATE_FAMILIES = [MEASUREMENT_GATE_FAMILY, WAIT_GATE_FAMILY]
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we perhaps call this _VARIADIC_GATE_FAMILIES instead? I think the term "variadic" captures the property we want, that these are gates that can be applied to any number of qubits.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am going to remember this vocabulary word for when I take the Quantum SATs!

@verult verult added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 12, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 12, 2022
@CirqBot
Copy link
Collaborator

CirqBot commented Jul 12, 2022

Automerge cancelled: A required status check is not present.

Missing statuses: ['Build docs', 'Build protos', 'Changed files test', 'Coverage check', 'Doc test', 'Lint check', 'Notebook formatting', 'Pytest MacOS (3.7)', 'Pytest MacOS (3.8)', 'Pytest MacOS (3.9)', 'Pytest Ubuntu (3.7)', 'Pytest Ubuntu (3.8)', 'Pytest Ubuntu (3.9)', 'Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Pytest Windows (3.9)', 'Type check', 'Typescript lint check', 'Typescript tests', 'Typescript tests coverage']

@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 Jul 12, 2022
@MichaelBroughton MichaelBroughton added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 12, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 12, 2022
@CirqBot CirqBot merged commit 382e37b into quantumlib:master Jul 12, 2022
@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 Jul 12, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…antumlib#5654)

I'm comfortable with special-casing MeasurementGate because it's the only gate today with the property that it can be applied to any subset of qubits.

Fixes quantumlib#5652

@maffoo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: S 10< lines changed <50
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GridDevice should allow measuring any qubit
6 participants