-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Improve docstring for unitary() & other text in unitary_protocol.py #7582
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
Open
mhucka
wants to merge
3
commits into
quantumlib:main
Choose a base branch
from
mhucka:mh-improve-unitary-docstring
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -24,7 +24,7 @@ | |||
| from cirq.protocols.apply_unitary_protocol import apply_unitaries, ApplyUnitaryArgs | ||||
| from cirq.protocols.decompose_protocol import _try_decompose_into_operations_and_qubits | ||||
|
|
||||
| # This is a special indicator value used by the unitary method to determine | ||||
| # This is a special indicator value used by the function unitary() to determine | ||||
| # whether or not the caller provided a 'default' argument. It must be of type | ||||
| # np.ndarray to ensure the method has the correct type signature in that case. | ||||
| # It is checked for using `is`, so it won't have a false positive if the user | ||||
|
|
@@ -44,17 +44,17 @@ def _unitary_(self) -> np.ndarray | NotImplementedType | None: | |||
| This method is used by the global `cirq.unitary` method. If this method | ||||
| is not present, or returns NotImplemented, it is assumed that the | ||||
| receiving object doesn't have a unitary matrix (resulting in a TypeError | ||||
| or default result when calling `cirq.unitary` on it). (The ability to | ||||
| or default result when calling `cirq.unitary` on it). The ability to | ||||
| return NotImplemented is useful when a class cannot know if it has a | ||||
| matrix until runtime, e.g. cirq.X**c normally has a matrix but | ||||
| cirq.X**sympy.Symbol('a') doesn't.) | ||||
| matrix until runtime; e.g., `cirq.X**c` normally has a matrix but | ||||
| `cirq.X**sympy.Symbol('a')` doesn't. | ||||
|
|
||||
| The order of cells in the matrix is always implicit with respect to the | ||||
| object being called. For example, for gates the matrix must be ordered | ||||
| object being called. For example, for gates, the matrix must be ordered | ||||
| with respect to the list of qubits that the gate is applied to. For | ||||
| operations, the matrix is ordered to match the list returned by its | ||||
| `qubits` attribute. The qubit-to-amplitude order mapping matches the | ||||
| ordering of numpy.kron(A, B), where A is a qubit earlier in the list | ||||
| ordering of `numpy.kron(A, B)`, where A is a qubit earlier in the list | ||||
| than the qubit B. | ||||
|
|
||||
| Returns: | ||||
|
|
@@ -68,7 +68,7 @@ def _has_unitary_(self) -> bool: | |||
|
|
||||
| This method is used by the global `cirq.has_unitary` method. If this | ||||
| method is not present, or returns NotImplemented, it will fallback | ||||
| to using _unitary_ with a default value, or False if neither exist. | ||||
| to using `_unitary_()` with a default value, or False if neither exist. | ||||
|
|
||||
| Returns: | ||||
| True if the value has a unitary matrix representation, False | ||||
|
|
@@ -81,17 +81,19 @@ def unitary( | |||
| ) -> np.ndarray | TDefault: | ||||
| """Returns a unitary matrix describing the given value. | ||||
|
|
||||
| The matrix is determined by any one of the following techniques: | ||||
| The matrix is determined by the first of these strategies that succeeds: | ||||
|
|
||||
| - If the value is a numpy array, it is returned directly. | ||||
| - The value has a `_unitary_` method that returns something besides None or | ||||
| NotImplemented. The matrix is whatever the method returned. | ||||
| - If the value is a NumPy array, it is tested using `linalg.is_unitary()`. | ||||
| If the result is `True`, the array is returned directly; if `is_unitary()` | ||||
| returns `False`, a `ValueError` exception is raised. | ||||
| - The value has a `_unitary_` method that returns something besides `None` or | ||||
| `NotImplemented`. The matrix is whatever the method returned. | ||||
| - The value has an `_apply_unitary_` method, and it returns something | ||||
| besides `None` or `NotImplemented`. The matrix is created by applying | ||||
| `_apply_unitary_` to an identity matrix. | ||||
| - The value has a `_decompose_` method that returns a list of operations, | ||||
| and each operation in the list has a unitary effect. The matrix is | ||||
| created by aggregating the sub-operations' unitary effects. | ||||
| - The value has an `_apply_unitary_` method, and it returns something | ||||
| besides None or NotImplemented. The matrix is created by applying | ||||
| `_apply_unitary_` to an identity matrix. | ||||
|
|
||||
| If none of these techniques succeeds, it is assumed that `val` doesn't have | ||||
| a unitary effect. The order in which techniques are attempted is | ||||
|
|
@@ -100,8 +102,8 @@ def unitary( | |||
| Args: | ||||
| val: The value to describe with a unitary matrix. | ||||
| default: Determines the fallback behavior when `val` doesn't have | ||||
| a unitary effect. If `default` is not set, a TypeError is raised. If | ||||
| `default` is set to a value, that value is returned. | ||||
| a unitary effect. If `default` is not set, a `TypeError` is raised. | ||||
| If `default` is set to a value, that value is returned. | ||||
|
|
||||
| Returns: | ||||
| If `val` has a unitary effect, the corresponding unitary matrix. | ||||
|
|
@@ -135,13 +137,14 @@ def unitary( | |||
| f"type: {type(val)}\n" | ||||
| f"value: {val!r}\n" | ||||
| "\n" | ||||
| "The value failed to satisfy any of the following criteria:\n" | ||||
| "The given value failed to satisfy any of the following criteria:\n" | ||||
| "- A NumPy array for which `linalg.is_unitary()` returned `True`.\n" | ||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not checked anymore and thus not possible here.
Suggested change
|
||||
| "- A `_unitary_(self)` method that returned a value " | ||||
| "besides None or NotImplemented.\n" | ||||
| "besides `None` or `NotImplemented`.\n" | ||||
| "- A `_decompose_(self)` method that returned a " | ||||
| "list of unitary operations.\n" | ||||
| "- An `_apply_unitary_(self, args) method that returned a value " | ||||
| "besides None or NotImplemented." | ||||
| "- An `_apply_unitary_(self, args)` method that returned a value " | ||||
| "besides `None` or `NotImplemented`." | ||||
| ) | ||||
|
|
||||
|
|
||||
|
|
||||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.