Add missing term to trotter error loose bounds#1359
Conversation
The loose error bound for trotter decomposition was missing a term that lead to underestimation of the error in some systems. The loose error bound unit test is also changed accordingly.
![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 updates the error_bound function in trotter_error.py by documenting the mathematical formulas for the tight and loose bounds in LaTeX and adding a missing term to the loose error bound calculation. It also updates the corresponding test in trotter_error_test.py. The reviewer pointed out that the loose error bound is overestimated by a factor of 12 because the coefficients used (4.0) do not account for the correct scaling factors from the second-order Trotter error expansion. The reviewer suggested scaling both terms by 1.0 / 3.0 instead of 4.0 and updating the test assertions accordingly.
mhucka
left a comment
mhucka
left a comment
There was a problem hiding this comment.
It looks like there may be some opportunities for efficiency improvements.
| if coefficient_a: | ||
| error_a = 0.0 | ||
|
|
||
| for beta in range(alpha + 1, len(terms)): | ||
| term_b = terms[beta] | ||
| (coefficient_b,) = term_b.terms.values() | ||
| if not ( | ||
| trivially_commutes(term_a, term_b) or commutator(term_a, term_b) == zero | ||
| ): | ||
| error_a += abs(coefficient_b) |
There was a problem hiding this comment.
Actually, upon further examination, it looks like the use of a doubly-nested loop is not necessary. The value computed by the inner loop does not depend on coefficient_a, which implies error_a could be computed in a separate pass, turning this O(N²) algorithm into O(N).
There was a problem hiding this comment.
I think we have to keep the nested loop here unfortunately. The commutation check in the inner loop depends on both alpha and beta.
Reduces the number of arithmetic operations when computing the Trotter loose error bounds.
|
@arettig I'll leave it to you to merge this when you're ready. |
2 participants
This fixes #819. The loose error bound for trotter decomposition is currently implemented as equation 16 from this paper:
In the paper they mention this is only an upper bound on the second term of equation 6, but we should be giving an upper bound of both terms. I've changed the loose error bound to account for both terms:
The loose error bound unit test is also changed accordingly.