-
Notifications
You must be signed in to change notification settings - Fork 575
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 qml.matrix(op)
function
#2241
Conversation
Hello. You may have forgotten to update the changelog!
|
Codecov Report
@@ Coverage Diff @@
## master #2241 +/- ##
=======================================
Coverage 99.28% 99.28%
=======================================
Files 233 235 +2
Lines 18808 18831 +23
=======================================
+ Hits 18673 18696 +23
Misses 135 135
Continue to review full report at Codecov.
|
|
||
|
||
@qml.op_transform | ||
def matrix(op, *, wire_order=None): |
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.
If this can take tapes etc, would you not call it obj
?
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 point!
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.
Oh, because the wrapper makes sure that by the time it reaches the code in this function, it is an 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.
Yep exactly :) But the documentation will only show the docstring for the first one, hence why I've tried to keep all the information for op/tape/qfunc etc in a single docstring
Returns: | ||
tensor_like or function: Function which accepts the same arguments as the QNode or quantum | ||
function. When called, this function will return the unitary matrix in the appropriate | ||
autodiff framework (Autograd, TensorFlow, PyTorch, JAX) given its parameters. |
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.
These are two very different return options. Maybe at least describe the other one, since the explanation just talks about the function return type.
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.
Sorry, which one do you mean to describe here? Just the functional one?
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.
Hmm, it can return a tensor or a function, hey? It would be good to say "the tensor is the desired matrix; if a function then it accepts..." or so?
@pytest.mark.parametrize("op_class", one_qubit_no_parameter) | ||
def test_non_parametric_gates_qfunc(self, op_class): | ||
"""Verify that the matrices of non-parametric one qubit gates is correct | ||
when provided as a qfunc""" |
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 may be blind, but it's more provided as a class, not a qfunc, no?
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.
instantiating an operator is basically the same thing, from PL's point of view, as a qfunc that only queues a single operation 😆 Which is very useful way of thinking when writing the tests
|
||
@pytest.mark.xfail( | ||
reason="This test will fail because Hamiltonians are not queued to tapes yet!" | ||
) |
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.
Interesting!
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.
yes, I didn't realize this was the case either! something we should really fix.
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.
Very minor comments, I didn't find any objections :)
I don't really understand matrix.tape_transform
but it seems to work :)
Co-authored-by: Maria Schuld <mariaschuld@gmail.com>
…to matrix-function
Extra awesome that |
Context:
Currently, to get the matrix of an operator, you must do
op.get_matrix()
. Alternatively, to get the matrix of a tape/qfunc, you must doqml.transforms.get_unitary_matrix()
.However, there are many downsides to this:
op.get_matrix()
is temporary, and we want to limit users relying on operator methods!qml.transforms.get_unitary_matrix()
is hard to find, and also will fail if not all operations on the tape supportget_matrix()
.It would be nice if there was a single function that covered this singular functionality, while insulating the
op.get_matrix()
method from changes.Description of the Change:
Adds
qml.matrix()
This function works on operations, qfuncs, tapes, and QNodes.
It accepts wire ordering.
Benefits: As above.
Possible Drawbacks: n/a
Related GitHub Issues: n/a