-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve the performance of the ProductFormula
synthesizers
#12724
Conversation
This changes the structure of the `atomic_evolution` callable in the `ProductFormula` synthesis class. This is motivated by the significant performance improvements that can be obtained by appending to the existing circuit directly rather than building out individual evolution circuits and iteratively composing them.
This can be used to recover the previous behavior in which the single individually evolved Pauli terms get wrapped into gate objects.
One or more of the following people are relevant to this code:
|
ProductFormula
synthesizersProductFormula
synthesizers
Pull Request Test Coverage Report for Build 9796156398Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, both performance and visualization wise, thanks for the effort! 🙂 I just have a few minor comments below.
releasenotes/notes/product-formula-improvements-1bc40650151cf107.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/product-formula-improvements-1bc40650151cf107.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Julien Gacon <[email protected]>
Co-authored-by: Julien Gacon <[email protected]>
Pull Request Test Coverage Report for Build 9806942643Details
💛 - Coveralls |
This is slightly faster than the `.compose`-based operation done previously as it performs fewer checks. Thanks to @jakelishman for the suggestion offline.
Pull Request Test Coverage Report for Build 9842266493Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Co-authored-by: Julien Gacon <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvement, thanks a lot for the work! 🚤 💨
…12724) * [WIP] adds the output argument to the internal atomic evolution * meta: modernize type hints * refactor: change callable structure of atomic evolution This changes the structure of the `atomic_evolution` callable in the `ProductFormula` synthesis class. This is motivated by the significant performance improvements that can be obtained by appending to the existing circuit directly rather than building out individual evolution circuits and iteratively composing them. * refactor: deprecate the legacy atomic_evolution signature * refactor: add the wrap argument to ProductFormula This can be used to recover the previous behavior in which the single individually evolved Pauli terms get wrapped into gate objects. * fix: insert the missing barriers between LieTrotter repetitions * refactor: align SuzukiTrotter and LieTrotter * Propoagate deprecation notice * fix: the labels of wrapped Pauli evolutions * fix: respect the insert_barriers setting * docs: add a release note * Apply suggestions from code review Co-authored-by: Julien Gacon <[email protected]> * fix: missing `wrap` forward in SuzukiTrotter * docs: improve documentation of the `atomic_evolution` argument Co-authored-by: Julien Gacon <[email protected]> * docs: also document the deprecated form of `atomic_evolution` * docs: call out ProductFormula docs in release note * refactor: change to PendingDeprecationWarning * refactor: explicitly convert to Gate when wrapping This is slightly faster than the `.compose`-based operation done previously as it performs fewer checks. Thanks to @jakelishman for the suggestion offline. * Update qiskit/synthesis/evolution/lie_trotter.py Co-authored-by: Julien Gacon <[email protected]> * docs: update after pending deprecation --------- Co-authored-by: Julien Gacon <[email protected]>
Summary
This PR implements the suggestion made in #12021 (comment).
Details and comments
I ran a very simple benchmark to showcase the performance improvements of this PR. Below you can see that we get a ~30x performance improvement (note the logarithmic y axis).
Here is the code which builds a heavy-hex graph of a given size, constructs an XYZ model Hamiltonian on its connectivity, and builds a LieTrotter and SuzukiTrotter circuit.
As part of this improvement, I also aligned the implementations of
LieTrotter
andSuzukiTrotter
which partially diverged in #12021. I also handle the wrapping of the individually time-evolved Pauli terms in a new way, which results in a different (but imo improved) visualization of a circuit. Below you can see the same circuit drawn with Qiskit 1.1.1 and this PR, side by side:Here is the code to plot the circuit above