Skip to content

Add MatrixGate.with_name method. #5941

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 8, 2022
Merged

Add MatrixGate.with_name method. #5941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0a963f9
Add MatrixGate.with_name method.
maffoo Nov 4, 2022
6e898d9
Format
maffoo Nov 6, 2022
ecbdcc0
Reorder init method to fix types
maffoo Nov 6, 2022
dbe432a
Add note about when appropriate to disable unitary_check
maffoo Nov 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions cirq-core/cirq/ops/matrix_gates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def __init__(
*,
name: str = None,
qid_shape: Optional[Iterable[int]] = None,
unitary_check: bool = True,
unitary_check_rtol: float = 1e-5,
unitary_check_atol: float = 1e-8,
) -> None:
Expand All @@ -66,6 +67,10 @@ 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add a note in the docstring saying that it should be set to false only for performance optimizations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

given tolerances. This should only be disabled if the matrix has already been
checked for unitarity, in which case we get a slight performance improvement by
not checking again.
unitary_check_rtol: The relative tolerance for checking whether the supplied matrix
is unitary. See `cirq.is_unitary`.
unitary_check_atol: The absolute tolerance for checking whether the supplied matrix
Expand Down Expand Up @@ -99,8 +104,14 @@ def __init__(
f'qid_shape: {self._qid_shape}\n'
)

if not linalg.is_unitary(matrix, rtol=unitary_check_rtol, atol=unitary_check_atol):
raise ValueError(f'Not a unitary matrix: {self._matrix}')
if unitary_check and not linalg.is_unitary(
matrix, rtol=unitary_check_rtol, atol=unitary_check_atol
):
raise ValueError(f'Not a unitary matrix: {matrix}')

def with_name(self, name: str) -> 'MatrixGate':
"""Creates a new MatrixGate with the same matrix and a new name."""
return MatrixGate(self._matrix, name=name, qid_shape=self._qid_shape, unitary_check=False)

def _json_dict_(self) -> Dict[str, Any]:
return {'matrix': self._matrix.tolist(), 'qid_shape': self._qid_shape}
Expand Down
10 changes: 10 additions & 0 deletions cirq-core/cirq/ops/matrix_gates_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,16 @@ def test_named_two_qubit_diagram():
assert expected_vertical == c.to_text_diagram(transpose=True).strip()


def test_with_name():
gate = cirq.MatrixGate(cirq.unitary(cirq.Z**0.25))
T = gate.with_name('T')
S = (T**2).with_name('S')
assert T._name == 'T'
np.testing.assert_allclose(cirq.unitary(T), cirq.unitary(gate))
assert S._name == 'S'
np.testing.assert_allclose(cirq.unitary(S), cirq.unitary(T**2))


def test_str_executes():
assert '1' in str(cirq.MatrixGate(np.eye(2)))
assert '0' in str(cirq.MatrixGate(np.eye(4)))
Expand Down