[WIP] Avoid duplicate diagonalizing_gates #6288
Draft
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context:
Currently,
diagonalizing_gates
for a tape just iterates over all the observables and adds their diagonalizing gates to the list. This means that if the same observable occurs more than once, it's diagonalizing gates are included more than once.Description of the Change:
We get diagonalizing gates from a set of the base observables instead of
self.observables
.Benefits:
Open question:
One slightly unintuitive thing here is how to handle cases with
CompositeOp
observable where an observable is duplicated, but in an observable that can't be divided because its terms don't commute. For example, currently in this PR for:Here we include
X(0).diagonalizing_gates()
and theQubitUnitary
that diagonalizedqml.X(0)+qml.Y(0)
. Since the sum can't be diagonalized as a single observable we treat is as a completely different observable, the way we would:In contrast, if the sum doesn't have overlapping wires, and thus would be diagonalized one term at a time instead of as a single observable:
we end up with
X(0).diagonalizing_gates() + Y(1).diagonalizing_gates()
This feels more consistent now that we have a diagonalizing gate for X(0)+Y(0), but it doesn't reflect how we currently diagonalize (currently we would split the terms on to separate tapes and use
X(0).diagonalizing_gates()
on one andY(0).diagonalizing_gates()
on the other. So maybe we should remove the special treatment for CompositeOps with overlapping wires and just let the diagonalizing gates for[qml.X(0), qml.X(0)+qml.Y(0)]
beX(0).diagonalizing_gates() + Y(0).diagonalizing_gates()
.[sc-64695]