Fix Jacobi-Anger degree search stopping at a Bessel zero crossing - #1911
Fix Jacobi-Anger degree search stopping at a Bessel zero crossing#1911Sanjays2402 wants to merge 1 commit into
Conversation
degree_jacobi_anger_approximation searched for the smallest n with |J_n(t)| <= precision starting from n = 1. For n < t, |J_n(t)| oscillates and crosses zero repeatedly, so with a loose precision the doubling loop and the following bisection could land on a zero crossing rather than the asymptotic tail: for t = 50, precision = 1e-2 it returned degree 15 (|J_16(50)| = 0.005) instead of 56, and the resulting truncated expansion was not an approximation of exp(i t cos(theta)) at all. |J_n(t)| decays monotonically only for n >= t, so the search now starts at ceil(t) and treats every smaller n as not-yet-small, which keeps the bisection predicate monotonic. Fixes quantumlib#1848
![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 restricts the Jacobi-Anger approximation degree search to the decaying region (n >= t) to prevent premature truncation at zero crossings of the Bessel function, and adds a test case to verify this behavior. The review feedback correctly points out that negative values of t are not handled properly by this change, and suggests using abs(float(t)) for the minimum degree calculation along with adding negative test cases.
| # `|J_n(t)|` oscillates and crosses zero many times for `n < t`, and only decays | ||
| # monotonically for `n >= t`. Restricting the search to the decaying region keeps | ||
| # `term_too_small` monotonic, so we do not truncate at a zero crossing. | ||
| d_min = max(1, int(np.ceil(float(t)))) |
There was a problem hiding this comment.
The minimum degree d_min is calculated using float(t). However, if t is negative, float(t) will be negative, resulting in d_min = 1. Since |J_n(t)| = |J_n(|t|)|, the Bessel function magnitude is symmetric with respect to t, and the oscillatory region still extends up to |t|. For negative t, this would cause the search to start at 1 and potentially truncate at a zero-crossing, re-introducing the bug. Using abs(float(t)) ensures that the decaying region is correctly identified for both positive and negative values of t.
| d_min = max(1, int(np.ceil(float(t)))) | |
| d_min = max(1, int(np.ceil(abs(float(t))))) |
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("t", [20, 50]) |
There was a problem hiding this comment.
To ensure that negative values of t are also correctly handled and do not suffer from the same zero-crossing truncation issue, we should include negative test cases in the parameter list.
| @pytest.mark.parametrize("t", [20, 50]) | |
| @pytest.mark.parametrize("t", [-50, -20, 20, 50]) |
1 participant
Fixes #1848
degree_jacobi_anger_approximationsearched for the smallestnwith|J_n(t)| <= precisionstarting atn = 1, but|J_n(t)|oscillates through zero forn < t, so with a loose precision the doubling loop and bisection could stop at a zero crossing instead of the asymptotic tail. For the issue's case (t = 50,precision = 1e-2) it returned degree 15 because|J_16(50)| = 0.005, and the truncated expansion did not approximateexp(i t cos(theta))at all (max error 1.23). It now returns 56.|J_n(t)|decays monotonically only forn >= t, so the search starts atceil(t)and treats smallernas not-yet-small, keeping the bisection predicate monotonic. The added test coverstin {20, 50} at loose precisions and fails on the current code.