-
Notifications
You must be signed in to change notification settings - Fork 989
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
Diagonal gate #3890
Diagonal gate #3890
Conversation
Call the file and classes |
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.
Looking great, I found a bunch of nits though. +1 to renaming the file to diagonal_gate.py
.
Co-authored-by: Balint Pato <balopat@users.noreply.github.com>
Co-authored-by: Balint Pato <balopat@users.noreply.github.com>
Co-authored-by: Balint Pato <balopat@users.noreply.github.com>
Co-authored-by: Balint Pato <balopat@users.noreply.github.com>
Co-authored-by: Balint Pato <balopat@users.noreply.github.com>
Co-authored-by: Balint Pato <balopat@users.noreply.github.com>
Co-authored-by: Balint Pato <balopat@users.noreply.github.com>
Co-authored-by: Balint Pato <balopat@users.noreply.github.com>
Thanks for reviewing! The file has been renamed to diagonal_gate. About parameterized DiagonalGate comment I have another question. Please take a look again. |
# We do not support the decomposition of parameterized case yet. | ||
# So cirq.decompose should do nothing. | ||
assert len(cirq.decompose(op)) == 1 | ||
assert cirq.decompose(op)[0] == op | ||
|
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.
you can simplify these two lines to assert cirq.decompose(op) == [op]
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.
Done.
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, great! One more tiny nit
Co-authored-by: Balint Pato <balopat@users.noreply.github.com>
One more thing: cirq.contrib.acquaintance.executor_test.py has a test DiagonalGate defined. Can you replace that with the new shiny DiagonalGate? Thanks! |
1 similar comment
One more thing: cirq.contrib.acquaintance.executor_test.py has a test DiagonalGate defined. Can you replace that with the new shiny DiagonalGate? Thanks! |
Sure. Done. |
Over in qiskit they mention Hidden Shift can be represented by a diagonal gate https://qiskit.org/documentation/stubs/qiskit.circuit.library.Diagonal.html, but I'm not grokking. We have a hidden shift example https://github.com/quantumlib/Cirq/blob/master/examples/hidden_shift_algorithm.py. Should it be updated to use this (or perhaps Hamiltonian Binary since you mention they have the same underlying math)? |
Hi Dax, Right, simulating any oracle problem, like
Note Qiskit Diagonal gate has more functionalities (we have dense unitary and 1, 2-qubits decomposite representation only now). We can also consider making it more generalized first. |
Add an N-qubit DiagonalGate, which is specified in #3866. The decomposition algorithm (2^n-1 Rz gates and 2^n -2 CNOT gates) is implemented based on the paper:
For the step-by-step explanation of the implemention, please refer this colab.
A current known issue is that the decomposition algorithm has a global phase shift between the diagonal unitary matrix and the decomposed circuit implementation. It can produce a surprising effect if the diagonal gates is used within a partial system, such as under the controlled gate. Hence, a GlobalPhaseOperation is added into the _decompose_ method. But GlobalPhaseOperation doesn't have as universal support as the single-/two-qubit gates. So adding GlobalPhaseOperation may limit the usage of DiagonalGate.