Closed ixfoduap closed 3 years ago
Thanks @ixfoduap! Note that this will be solved by moving tape expansion to the device. The logical flow will look as follows:
SingleExcitation
and DoubleExcitation
operations.default.qubit
is in control of expansion, and understands these operations natively, so does not decompose them for the forward pass/classical backpropagation.SingleExcitationPlus
and SingleExcitationMinus
(etc.) for gradient computations.(@trbromley)
Something to be careful of: expansion cannot happen within the interface black-box, it must happen outside.
This will be so clean! :)
Both excitation gates satisfy the four-term parameter shift rule, so we could use that one :) I talked to Josh and I am excited about the split up of the differentiation pass, but for these gates we might get away with the current structure.
@dwierichs we should probably add the 4-term rules sooner rather than later, as I am struggling to split up the forward and backward expansions 😆 It was not as straightforward as I thought it would be.
In particular: moving the forward expansion was easy, and worked well. The issue is that the backward expansion could result in additional classical processing, that we need to account for during the backward pass.
Closing this, as the 4-term shift rules have been added.
The new SingleExcitation and DoubleExcitation operations support analytical gradients through a decomposition into a product of two gates that satisfy the usual parameter-shift rule.
However, since these gates are actually supported in default PennyLane simulators, they are not in fact decomposed when computing gradients. Instead, by specifying that gradients are computed analytically, PennyLane assumes that the excitation gates themselves satisfy the usual parameter-shift rule. This leads to incorrect results when computing gradients using parameter-shift.
During development, gradients were tested across different methods, but unfortunately we have now identified that there was an bug in the tests themselves.
Since gradients are now calculated using backpropagation as default, this is not an issue in the majority of calculations. It will only appear for users that build circuits using these operations and explicitly set the differentiation method to parameter-shift.
This problem can be fixed by removing these operations from the list of supported ones, but this will lead to an unnecessary doubling of gates applied in the forward pass.