-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add Qudit phase gate, Z_d #194
Conversation
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.
Thanks for the contribution! A few comments!
unitary/alpha/qudit_gates.py
Outdated
@@ -12,6 +12,14 @@ | |||
# See the License for the specific language governing permissions and | |||
# limitations under the License. | |||
# | |||
|
|||
|
|||
from typing import ( |
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.
nit: all on one line looks better to me, but I am not sure if the formatter is doing this.
unitary/alpha/qudit_gates.py
Outdated
@@ -51,6 +59,53 @@ def _circuit_diagram_info_(self, args): | |||
return f"X({self.source_state}_{self.destination_state})" | |||
|
|||
|
|||
class QuditRzGate(cirq.ops.eigen_gate.EigenGate): |
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.
I would just use cirq.EigenGate
in case package names change.
unitary/alpha/qudit_gates.py
Outdated
radians: The phase shift applied to basis d-1, measured in radians. | ||
""" | ||
|
||
_eigencomponents: Dict[int, List[Tuple[float, np.ndarray]]] = {} |
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.
Having _eignencomponents and _eigen_components seems confusing.
assert np.isclose(phase_rads / np.pi, rz._exponent) | ||
rz_unitary = cirq.unitary(rz) | ||
assert np.allclose(cirq.unitary(rz), expected_unitary) | ||
assert np.allclose(np.eye(len(rz_unitary)), rz_unitary.dot(rz_unitary.T.conj())) |
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.
I'd feel more comfortable if we had a test on the action of the QuditRzGate too. Can we test the qudit equivalent of HZH and make sure it is equivalent to X?
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.
Maybe let's make this a follow-up PR until the qudit Hadamard PR issue gets 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.
Some minor comments, then LGTM.
unitary/alpha/qudit_gates.py
Outdated
For a qudit of dimensionality d, shifts the phase of |phased_state> by radians. | ||
|
||
Args: | ||
dimension: Dimension of the qudits: for instance, a dimension of 3 |
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.
nit: would probably change the second colon to a period (then capitalize "For")
unitary/alpha/qudit_gates_test.py
Outdated
(np.pi * 4 / 3, np.pi * 2 / 3, 0, 1), | ||
], | ||
) | ||
def test_X_HZH_qudit_identity( |
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.
Why is this function repeated twice?
arxiv.org/abs/2008.00959 describes the Z_d gate as one of three gates that comprise a "universal gate set" for qudit computing.
The commit implements the Z_d gate using cirq.ops.eigen_gate.EigenGate as a base.