Consistent sparse list format in sparse operators (backport #14067) #14110
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.
Summary
Building a
PauliEvolutionGate
from aSparsePauliOp
andSparseObservable
representing the same Hamiltonian (both using only Paulis) currently does not generate the same circuit. Both are correct, but they are inconsistent: the CX chains in the Pauli evolution are just reversed. @alexanderivrii ran some benchmarks based on HamLib and found that, due to all our surprise, one ordering seems to be almost always result in less CX gates than the other. Somehow one order allows for better CX cancellations. The ordering comes back to what index order theto_sparse_list
methods return, which we chose opposite inSparseObservable
thanSparsePauliOp
, but we also could've chosen it to be consistent. Since there's now a performance impact, this PR changes to the consistent order.Details and comments
Here's the results of @alexanderivrii's benchmarks:
Another way to fix this would be to sort the indices and Paulis in the
PauliEvolution
but this (a) comes at additional cost and (b) will not allow users to insert custom index orders (since they would get re-sorted). So changing the order which we picked for a more natural reading seems fine here to be more consistent (and get better gate decompositions).No reno since this was introduced in 2.0. I'm labelling this as bugfix/stable backport so we can fix this for 2.0.
This is an automatic backport of pull request #14067 done by Mergify.