Add nsteps for Trotter expansion based on propagator error#1371
Add nsteps for Trotter expansion based on propagator error#1371arettig wants to merge 2 commits into
Conversation
The current implementation computes the number of steps needed in a Trotter expansion to reach an error in the ground state energy. This commit adds functionality to compute the number of steps determined by the error in the propagator itself.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces trotter_steps_required_propagator to calculate the number of Trotter steps required for accurate state evolution, updates trotter_steps_required to handle negative times correctly, and adds corresponding unit tests. The reviewer suggested adding input validation to both functions to prevent division-by-zero errors when precision parameters are non-positive, and ensuring that at least one step is returned for any non-zero simulation time when the error bound is zero.
Added checks to ensure the parameters and therefore output of trotter_steps_required functions are physically meaningful. Added several tests for edge cases of these functions.
mhucka
left a comment
mhucka
left a comment
There was a problem hiding this comment.
IMHO it would benefit for just a couple of small additions to value testing. Otherwise it looks good!
| if energy_precision <= 0: | ||
| raise ValueError("energy_precision must be strictly positive.") | ||
| if time == 0: | ||
| return 0 | ||
| return max(1, int(ceil(abs(time) * sqrt(trotter_error_bound / energy_precision)))) |
There was a problem hiding this comment.
Should this also check that trotter_error_bound has reasonable values? E.g., that it's non-negative.
| if prop_precision <= 0: | ||
| raise ValueError("prop_precision must be strictly positive.") | ||
| if time == 0: | ||
| return 0 | ||
| atime = abs(time) | ||
| return max(1, int(ceil(atime * sqrt(atime * trotter_error_bound / prop_precision)))) |
There was a problem hiding this comment.
Similar to the comment above about trotter_error_bound
2 participants
Fixes #820. Currently we compute the number of steps in a Trotter expansion based on the desired error in the ground state energy. This PR adds a function to compute the number of steps based on the desired error in the propagator$U$ (which is different). This could be useful for certain use cases like dynamics.
New tests are also added to cover this new function.