Hi Authors,
I've been going through the methods one by one and lineardiff behaved differently. With no solver argument, the smoothed signal comes back swinging to about ±11 on an input of amplitude 1. If I pass solver='CLARABEL' the same call is fine.
importnumpyasnpfrompynumdiff.linear_modelimportlineardiffdt=0.01t=np.arange(0, 3, dt)
x=np.sin(3*t) +0.05*np.random.default_rng(0).standard_normal(len(t))
x_hat, _=lineardiff(x, dt, order=3, gamma=0.01, window_size=11) # default solverx_hat2, _=lineardiff(x, dt, order=3, gamma=0.01, window_size=11, solver='CLARABEL')
print(np.nanmax(np.abs(x_hat)), np.nanmax(np.abs(x_hat2)))
I poked at it a bit: cvxpy picks OSQP here, and on most of the windows it stops at its iteration limit (prob.status == 'user_limit', I got 6 of 8 windows on a 40-sample run). _solve_for_A_and_C_given_X_and_Xdot just returns A.value, C.value afterwards without looking at the status, so the half-converged numbers go straight into the derivative. The only hint is cvxpy's generic "Solution may be inaccurate" warning, which is easy to miss.
I think the tests don't catch it because test_diff_methods.py passes solver='CLARABEL' for lineardiff and uses 31 samples.
Would it make sense to check prob.status after solve() and warn/raise if it isn't optimal, and maybe default to CLARABEL when it's installed (it comes with cvxpy anyway)? A test with solver=None on a few hundred samples can catch this.
If using the solver's last iterate is intentional, a note in the docstring would help set expectations, though checking the status seems like the safer default.
(This is separate from #210 I hit that one too and see it's fixed on master.)
Environment: pynumdiff 0.2.3 (PyPI wheel), cvxpy 1.9.2 (OSQP 1.1.3, Clarabel 0.11.1), numpy 2.5.2, scipy 1.18.1, Python 3.13.15, macOS arm64.
Review reference: openjournals/joss-reviews#11172
Hi Authors,
I've been going through the methods one by one and lineardiff behaved differently. With no
solverargument, the smoothed signal comes back swinging to about ±11 on an input of amplitude 1. If I passsolver='CLARABEL'the same call is fine.I poked at it a bit: cvxpy picks OSQP here, and on most of the windows it stops at its iteration limit (
prob.status == 'user_limit', I got 6 of 8 windows on a 40-sample run)._solve_for_A_and_C_given_X_and_Xdotjust returnsA.value, C.valueafterwards without looking at the status, so the half-converged numbers go straight into the derivative. The only hint is cvxpy's generic "Solution may be inaccurate" warning, which is easy to miss.I think the tests don't catch it because
test_diff_methods.pypassessolver='CLARABEL'for lineardiff and uses 31 samples.Would it make sense to check
prob.statusaftersolve()and warn/raise if it isn't optimal, and maybe default to CLARABEL when it's installed (it comes with cvxpy anyway)? A test withsolver=Noneon a few hundred samples can catch this.If using the solver's last iterate is intentional, a note in the docstring would help set expectations, though checking the status seems like the safer default.
(This is separate from #210 I hit that one too and see it's fixed on master.)
Environment: pynumdiff 0.2.3 (PyPI wheel), cvxpy 1.9.2 (OSQP 1.1.3, Clarabel 0.11.1), numpy 2.5.2, scipy 1.18.1, Python 3.13.15, macOS arm64.
Review reference: openjournals/joss-reviews#11172