Add Operator has_matrix property
#2331
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2331 +/- ##
=======================================
Coverage 99.39% 99.39%
=======================================
Files 229 229
Lines 17396 17399 +3
=======================================
+ Hits 17290 17293 +3
Misses 106 106
Continue to review full report at Codecov.
|
pennylane/operation.py
Outdated
|
|
||
| Note: Child classes may have this as an instance property instead of as a class property. | ||
| """ | ||
| return cls.compute_matrix != Operator.compute_matrix |
There was a problem hiding this comment.
Clever solution, I didn't know this would work?
There was a problem hiding this comment.
Quick thought: what if an op overwrites matrix. We realised at the moment none of them needs to do that, but thought of it as a potential future design, and a developer could do it without guessing the severe consequences. Shall we ignore this for now?
There was a problem hiding this comment.
I didn't know this would work till recently either.
For most operations, this default will be fine. If someone wants to overwrite matrix at some point in the future without overwriting compute_matrix, then I would trust them enough to know to overwrite has_matrix too. I don't think we should avoiding fixing an immediate problem for something that might theoretically crop up at some point way down the line.
For the operator wrappers, we will be able to do:
def has_matrix(self):
return self.base.has_matrix
There was a problem hiding this comment.
I guess we could have:
return (cls.compute_matrix != Operator.compute_matrix) or (cls.get_matrix != Operator.get_matrix)
There was a problem hiding this comment.
Nice idea, the latter one!
| @@ -21,6 +21,23 @@ | |||
| from gate_data import I | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize("op_str", qml.ops.qubit.ops) | |||
There was a problem hiding this comment.
We moved away from the design of testing something on all ops - if a new op is added that just doesn't work for the test logic, it is a major nightmare.
I personally prefer tests that do not rely on ops being there at all. I think it is better to test exactly what we use: make a dummy op that overwrites compute_matrix and check for the dummy op.
| @@ -20,6 +20,11 @@ | |||
| import pennylane as qml | |||
|
|
|||
|
|
|||
| def test_has_matrix(): | |||
| """Test has_matrix for DisplacementEmbedding is False.""" | |||
| assert not qml.DisplacementEmbedding.has_matrix | |||
There was a problem hiding this comment.
We were planning to define a default matrix property, using the decomposition...maybe these tests are not so important?
Relates to the above comment: do you want to test that the templates have no matrix defined, or do you want to test that your query function works? Here you are doing the former, but I'm not sure if this is something we have to test, since it is just a missing feature!
|
@mariaschuld I've changed the tests so it now checks a generic new op in |
| num_wires = 1 | ||
|
|
||
| assert not MyOp.has_matrix | ||
| assert not MyOp(wires=0).has_matrix |
Currently, our simulation devices have to maintain a list of supported class names. This list can be difficult to maintain as new classes are added, and doesn't allow user defined operations to be natively executed on the simulator.
All the simulator cares about is whether or not the operation defines a matrix. If an operator defines a matrix, then the simulator can use it.
Currently, we can't determine whether or not a given operation defines a matrix without requesting it and seeing if an error is raised. Requesting a matrix is a potentially expensive call that we don't want to always perform.
With the new operator modifiers being added to PennyLane (control, adjoint, pow, prod, etc.), they define a matrix only if their nested operations define matrices. Therefore we need a method or property that can wrap this nested logic.
In the base class, we use a
classpropertyas often enough thehas_matrixinformation will be available at the class level. Upcoming operator modifiers though will only be able to define this information at the instance level. So we should never rely on the information existing at the class level, but when it exists it may be convenient to use.A subsequent PR will modify the base
Deviceand our simulators to determine support based on thehas_matrixproperty.