-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Quantum Shannon Decomposition #6020
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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.
Thank you for opening the PR Uzair. Some high level comments:
- Please rename the file
quantum_shannon_decomposition
toquantum_shannon_decomposition.py
- Please add tests using pytest in a new file
quantum_shannon_decomposition_test.py
in the same directory. Make sure the test coverage checks pass. - Please sign the CLA.
Made these changes accordingly |
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.
Formatted file to address previous test failures
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.
Added testing file and formatted this file
Made edits for test coverage, formatting, lint |
Made changes with formatting new test functions and fixed error in test function for multiplexed cossin |
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.
Changes for QSD
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.
This is looking great. Left a round of comments mostly around styling and more robust testing.
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition_test.py
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition_test.py
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition_test.py
Outdated
Show resolved
Hide resolved
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.
Left another round of comments. This is looking good overall. We should be able to merge once the comments are resolved.
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition_test.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition_test.py
Outdated
Show resolved
Hide resolved
cirq-core/cirq/transformers/analytical_decompositions/quantum_shannon_decomposition_test.py
Outdated
Show resolved
Hide resolved
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.
Don't directly import the symbol names. Instead, import the sub-module which the symbol is a part of and then use <submodule_name>.<object_name>
.
For example:
from cirq import ops, protocols, linalg
if TYPE_CHECKING:
import cirq
Note that we have our import cirq
under a if TYPE_CHECKING
guard so that the entire module is available only for mypy checks. Now, for type checking you can use 'cirq.Qid'
instead of Qid
. For eg: def quantum_shannon_decomposition(qubits: List['cirq.Qid'], u: np.ndarray)
See other files in analytical decomposition for a reference.
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.
LGTM % nits
import cirq | ||
|
||
|
||
def quantum_shannon_decomposition(qubits: 'List[cirq.Qid]', u: np.ndarray) -> 'cirq.OP_TREE': |
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.
The arguments to a method should use the most general type (eg: Sequence instead of List) and the return type should be as specific as possible.
def quantum_shannon_decomposition(qubits: 'List[cirq.Qid]', u: np.ndarray) -> 'cirq.OP_TREE': | |
def quantum_shannon_decomposition(qubits: Sequence['cirq.Qid'], u: np.ndarray) -> 'cirq.OP_TREE': |
yield from _msb_demuxer(qubits, u1, u2) | ||
|
||
|
||
def _single_qubit_decomposition(qubit: 'cirq.Qid', u: np.ndarray) -> 'cirq.OP_TREE': |
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.
As discussed offline, consider updating the return type if internal helpers to cirq.ops.OpTree
instead, which should be enough to fix errors in yield from ()
statements.
Quantum Shannon Decomposition for cirq written by Uzair Faruqui, February 2023 Decomposes an arbitrary (2^n x 2^n) unitary matrix into a set of 1-qubit operations and 2-qubit CNOT gates. Implementation loosely based on this paper: https://arxiv.org/pdf/quant-ph/0406176.pdf
Update descriptions and tidy code
Remove extra whitespace
Changed name of file
Initial creation of test file. Will clean up and edit
Reformat test file
Reformatted file
Correction to argument name of 'test_random_qsd_n_qubit'
Missed a trailing whitespace
Correct argument for test_nth_gray
Added qsd to init
added qsd to init
Checks gates in gate set
…shannon_decomposition_test.py Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
…shannon_decomposition_test.py Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
…shannon_decomposition.py Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
…shannon_decomposition.py Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
Remove circuit_1 and circuit_2 from test_qsd_n_qubit
* Create quantum_shannon_decomposition Quantum Shannon Decomposition for cirq written by Uzair Faruqui, February 2023 Decomposes an arbitrary (2^n x 2^n) unitary matrix into a set of 1-qubit operations and 2-qubit CNOT gates. Implementation loosely based on this paper: https://arxiv.org/pdf/quant-ph/0406176.pdf --------- Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
Added Quantum Shannon Decomposition in file in Cirq / cirq-core / cirq / transformers / analytical_decompostions