Add MatrixGate.with_name method. #5941
Conversation
maffoo
force-pushed
the
u/maffoo/with-name
branch
from
de07e84 to
ecbdcc0
Compare
| @@ -66,6 +67,8 @@ def __init__( | |||
| qid_shape: The shape of state tensor that the matrix applies to. | |||
| If not specified, this value is inferred by assuming that the | |||
| matrix is supposed to apply to qubits. | |||
| unitary_check: If True, check that the supplied matrix is unitary up to the | |||
There was a problem hiding this comment.
Do we need to add this new argument to support the with_name method? Or Is it just a performance optimization ?
The docstring implies that the matrix should always be unitary, and enabling users to construct the gate with non-unitary matrices by passing unitary_check=False seems bad.
There was a problem hiding this comment.
It's just a performance optimization to allow skipping the check if you already know the matrix is unitary (as is the case in with_name). I agree it would be bad if a user uses this to construct matrix gates with non-unitary matrices, but they would have to do this explicitly and there's only so much we can do to protect users from themselves :-)
There was a problem hiding this comment.
Maybe add a note in the docstring saying that it should be set to false only for performance optimizations?
| @@ -66,6 +67,8 @@ def __init__( | |||
| qid_shape: The shape of state tensor that the matrix applies to. | |||
| If not specified, this value is inferred by assuming that the | |||
| matrix is supposed to apply to qubits. | |||
| unitary_check: If True, check that the supplied matrix is unitary up to the | |||
There was a problem hiding this comment.
Maybe add a note in the docstring saying that it should be set to false only for performance optimizations?
maffoo
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
the
automerge
CirqBot
removed
the
front_of_queue_automerge
Fixes #5938