Fix expand_fn in fourier
#1720
Fix expand_fn in fourier
#1720
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1720 +/- ##
=======================================
Coverage 99.22% 99.22%
=======================================
Files 204 204
Lines 15395 15395
=======================================
Hits 15275 15275
Misses 120 120
Continue to review full report at Codecov.
|
doc/releases/changelog-dev.md
Outdated
| * Fixes a bug in the newly introduced expansion function `expand_multi_par_and_no_gen` in | ||
| `qml.fourier.qnode_spectrum`. | ||
| [(#1720)](https://github.com/PennyLaneAI/pennylane/pull/1720) |
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah yeah, that makes much more sense :)
| @@ -376,19 +376,19 @@ def decomposition(theta, wires): | |||
| raise NotImplementedError() | |||
|
|
|||
| @qml.qnode(dev) | |||
| def circuit(x, z1, y, z2): | |||
| def circuit(x, z0, y, z1): | |||
There was a problem hiding this comment.
Curious if there was a reason for changing this (aside from just preferring 0-indexing 😆 )
| @@ -400,20 +400,22 @@ def test_no_generator_expansion(self): | |||
| class _CRX(qml.CRX): | |||
| generator = [None, 1] | |||
|
|
|||
| @qml.qnode(dev) | |||
| def circuit(x, z1, y, z2): | |||
| @qml.qnode(dev, max_expansion=0) | |||
There was a problem hiding this comment.
So this is to avoid CRX being decomposed now that the stopping condition has changed?
There was a problem hiding this comment.
Yes, previously, CRX was already decomposed in circuit.construct, so that the tape expansion would not be triggered based on the missing generator. This was key to the bug not being seen by this test case.
|
The modified test case fails on master :) It did not before, because of the missing |
|
:D Have a good night :) |
While the
stopping_criterionfor the expansion functionexpand_multi_par_and_no_genwas adapted during the PR #1681,the trigger to start the decomposition does not include the condition of a generator missing.
Due to initial expansion of gates when creating a QNode together with an overlap between the two criteria for expansion (multiple gate parameters and missing
generator) in the test case, this was overlooked by the test that was designed to check this particular case.