-
Notifications
You must be signed in to change notification settings - Fork 586
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
Adds Single Excitation operations #1121
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.
Looking great @ixfoduap, especially updating all the jax/tf/autograd devices! I'm guessing this is just a pre-review, since there are no tests yet?
Co-authored-by: Josh Izaac <josh146@gmail.com>
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 @ixfoduap!. I have enjoyed reviewing this PR while learning how to add new operations to PL.
""" | ||
c = np.cos(phi / 2) | ||
s = np.sin(phi / 2) | ||
return np.array([[1, 0, 0, 0], [0, c, -s, 0], [0, s, c, 0], [0, 0, 0, 1]]) |
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.
Docstring declares that the function returns array[complex]
but the matrix elements s
and c
are real, right?
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.
Good catch!
""" | ||
c = jnp.cos(phi / 2) | ||
s = jnp.sin(phi / 2) | ||
return jnp.array([[1, 0, 0, 0], [0, c, -s, 0], [0, s, c, 0], [0, 0, 0, 1]]) |
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.
Same comment here regarding the type of the returned array, jnp.Tensor[float]
?
phi = tf.cast(phi, dtype=C_DTYPE) | ||
c = tf.cos(phi / 2) | ||
s = tf.sin(phi / 2) | ||
return tf.convert_to_tensor([[1, 0, 0, 0], [0, c, -s, 0], [0, s, c, 0], [0, 0, 0, 1]]) |
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.
probably the same here: tf.Tensor[float]
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.
Note: while true for JAX and autograd above, in this case the tensor will be complex, since phi
is cast to complex on line 208. So it should remain tf.Tensor[complex]
.
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.
Note: while true for JAX and autograd above, in this case the tensor will be complex, since
phi
is cast to complex on line 208. So it should remaintf.Tensor[complex]
.
Attention to details! 😄
tests/gate_data.py
Outdated
phi (float): rotation angle | ||
|
||
Returns: | ||
array: the two-qubit Givens rotation describing the single excitation |
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.
"describing the single excitation" or "describing the operation"
""" | ||
c = np.cos(phi / 2) | ||
s = np.sin(phi / 2) | ||
return np.array([[1, 0, 0, 0], [0, c, -s, 0], [0, s, c, 0], [0, 0, 0, 1]]) |
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.
Good catch!
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 @ixfoduap. Looks great to me!.
phi = tf.cast(phi, dtype=C_DTYPE) | ||
c = tf.cos(phi / 2) | ||
s = tf.sin(phi / 2) | ||
return tf.convert_to_tensor([[1, 0, 0, 0], [0, c, -s, 0], [0, s, c, 0], [0, 0, 0, 1]]) |
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.
Note: while true for JAX and autograd above, in this case the tensor will be complex, since
phi
is cast to complex on line 208. So it should remaintf.Tensor[complex]
.
Attention to details! 😄
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.
Looks good to me! 💯
pennylane/devices/autograd_ops.py
Outdated
r""" | ||
Single excitation rotation with positive phase-shift outside the rotation subspace. |
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 mentioned in the suggestion from the first iteration, this recurs here and in other files too for the newly added docstrings.
r""" | |
Single excitation rotation with positive phase-shift outside the rotation subspace. | |
r"""Single excitation rotation with positive phase-shift outside the rotation subspace. |
pennylane/devices/autograd_ops.py
Outdated
r""" | ||
Single excitation rotation with negative phase-shift outside the rotation subspace. |
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.
r""" | |
Single excitation rotation with negative phase-shift outside the rotation subspace. | |
r"""Single excitation rotation with negative phase-shift outside the rotation subspace. |
pennylane/devices/autograd_ops.py
Outdated
|
||
|
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 a suggestion for previous docstrings too here and in other files.
tests/ops/test_qubit_ops.py
Outdated
|
||
@pytest.mark.parametrize(("excitation", "phi"), [(qml.SingleExcitation, -0.1), |
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.
Two things:
- Could we parametrize for
diff_method
here too? - How about testing
default.qubit
for parameter-shift?
tests/ops/test_qubit_ops.py
Outdated
pytest.skip("JAX support for the parameter-shift method is still TBD") | ||
|
||
jax = pytest.importorskip("jax") | ||
from jax import numpy as jnp |
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.
How come this import is here and for tf we don't have one? 🤔 Was there an occurrence for TF already before in the file that imported it?
Co-authored-by: antalszava <antalszava@gmail.com>
Context:
Current tools for constructing quantum circuits for quantum chemistry are inefficient. By relying on fermionic-to-qubit mappings to define operations and circuit decompositions to compute gradients, the resulting circuits for creating single and double excitation operations lead to lengthy computations. Through research work, we've identified that Givens rotations perform the same role as fermionic excitation operations, while allowing for simple analytical gradients and faster implementation in hardware and simulators.
Description of the Change:
This PR adds the two-qubit
SingleExcitation
operation with support across all native devices. Its analytical gradient requires a decomposition into two gates,SingleExcitationPlus
andSingleExcitationMinus
, which are also added as part of this PR.A follow-up PR will also add the four-qubit
DoubleExcitation
operations.Benefits:
Much faster implementation of quantum circuits for quantum chemistry.
Note:
The
SingleExcitationPlus
andSingleExcitationMinus
have generators G that satisfy G^2=1 (self-inverse). This means they satisfy the standard parameter-shift rule and no custom gradient formula needs to be passed