-
Notifications
You must be signed in to change notification settings - Fork 61
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
Remove TrotterCircuit
cache
#1371
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1371 +/- ##
==========================================
- Coverage 99.84% 99.84% -0.01%
==========================================
Files 74 74
Lines 10806 10788 -18
==========================================
- Hits 10789 10771 -18
Misses 17 17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Thanks, I agree with your analysis, which is confirmed by the test you added.
|
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.
Thank you @stavros11 for this implementation of a solution.
When I was reading the code before I was wondering what was the use of TrotterCircuit
and removing it resolves that question too.
Thank you for reviewing. I updated the test in 784db2c. I will merge after the CI passes. |
Fixes #1357.
I believe the
TrotterCircuit
class is implementing a cache of the circuit implementing the Trotter decomposition returned byhamiltonian.circuit(dt)
, to avoid recreating it if the users decides to use a differentdt
. However, this creates a ("secret") link between the circuits returned byhamiltonian.circuit
that can confuse users, such as in #1357, and also complicates the code.If allocating a circuit is not considered very costly, this cache probably does not provide much in terms of performance, therefore I decided to remove it to have cleaner code (and behavior).
Checklist: