Remove op validation when inserting into Circuit #4669
Conversation
cirq-core/cirq/ops/raw_types.py
Outdated
| @@ -853,17 +851,16 @@ def _validate_qid_shape(val: Any, qubits: Sequence['cirq.Qid']) -> None: | |||
| ValueError: The operation had qids that don't match it's qid shape. | |||
| """ | |||
| qid_shape = protocols.qid_shape(val) | |||
| if len(qubits) != len(qid_shape): | |||
| qubits_shape = protocols.qid_shape(qubits) | |||
There was a problem hiding this comment.
I have a slight preference to revert to the previous implementation here because the qid_shape protocol seems to be slow, and we know here that it's the same as tuple(q.dimension for q in qubits) but we don't actually need to construct the tuple.
cirq-core/cirq/ops/raw_types_test.py
Outdated
| @@ -120,7 +120,7 @@ def test_gate(): | |||
| _ = g(a, c) | |||
| with pytest.raises(ValueError, match='Wrong number'): | |||
| _ = g(c, b, a) | |||
| with pytest.raises(ValueError, match='Wrong shape'): | |||
| with pytest.raises(ValueError, match='g'): | |||
|
cc @MichaelBroughton re: devices api, which concerns itself with validation |
|
Updated per @maffoo's comments. |
|
Would this change mean a user gets no validation at all on a custom gate they write when putting it into a circuit ? |
There was a problem hiding this comment.
LGTM, though may want to get comments from @MichaelBroughton or other maintainers before merging.
|
We talked about this in the Cirq meeting today and I think we are fine to merge this in. |
MichaelBroughton
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
| @@ -4025,37 +4025,6 @@ def test_moments_property(circuit_cls): | |||
| assert c.moments[1] == cirq.Moment([cirq.Y(q)]) | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize('circuit_cls', [cirq.Circuit, cirq.FrozenCircuit]) | |||
| def test_operation_shape_validation(circuit_cls): | |||
| class BadOperation1(cirq.Operation): | |||
There was a problem hiding this comment.
Can we do validation in cirq.Operation base class ?
Proposal to remove validation on op insertion methods quantumlib#4667
Proposal to remove validation on op insertion methods quantumlib#4667
Proposal to remove validation on op insertion methods #4667