This is associated with the handling of "Missing data" in Table 1 of the paper.
Isolated NaNs run fine, but a run of consecutive NaNs, which might resemble more on cases where sensor dropouts/disconnects, goes unexpected:
Note: This is a minor issue and just needs proper error handling/documentation.
importnumpyasnpfrompynumdiff.polynomial_fitimportpolydiffdt=0.01t=np.arange(0, 3, dt)
x=np.sin(3*t) +0.05*np.random.default_rng(0).standard_normal(len(t))
xg=x.copy(); xg[150:159] =np.nan# 9 consecutive samples missingpolydiff(xg, dt, degree=2, window_size=9)
UserWarning: Window of all NaNs encountered. `polyfit` will fail. Choose a wider `window_size`?
...
LinAlgError: SVD did not converge in Linear Least Squares
The code already knows this can happen, but then calls polyfit anyway and the user gets a linear-algebra error instead of a message.
Two more things I noticed while trying to find where the limit is:
- With a shorter run it doesn't crash but the values inside the gap are bad. 5 NaNs in a row with
window_size=9 gave a worst error of about 28 inside the gap, vs about 7 for the same call with no gap, and no warning. - The limit is a bit tighter than "gap smaller than window". A 15-sample run with
window_size=17 still crashes with the default friedrichs kernel. I think it's because the only valid points in the middle windows are the two edge samples, and the friedrichs weight there is ~e^-500, so polyfit basically sees nothing (lhs /= scale divides by ~0). With kernel='gaussian' the same case runs but the derivative is worse than plain finite differences (RMSE 5.8 vs 3.7 on my signal). window_size=33 is fine.
test_missing_data has this covered, but puts only four single NaNs into 31 samples, so none of this shows up in CI.
Maybe raise a ValueError with a clear message when a window has fewer than degree+1 usable points, and mention the rough limit in the docstring (with the default kernel it seems to be about gap ≤ window_size − 2·(degree+1))?
A test with a contiguous gap would be good too. For what it's worth rtsdiff, splinediff and robustdiff all handled the same 15-sample gap nicely.
rtsdiff bridged it with no visible artefact. so this is really just a polydiff thing, and the paper's "sensors that drop samples need no preprocessing" sentence might want to name those methods.
Same setup as in my lineardiff issue: 0.2.3 wheel, and still the same with master installed from source.
Review reference: openjournals/joss-reviews#11172
This is associated with the handling of "Missing data" in Table 1 of the paper.
Isolated NaNs run fine, but a run of consecutive NaNs, which might resemble more on cases where sensor dropouts/disconnects, goes unexpected:
Note: This is a minor issue and just needs proper error handling/documentation.
The code already knows this can happen, but then calls polyfit anyway and the user gets a linear-algebra error instead of a message.
Two more things I noticed while trying to find where the limit is:
window_size=9gave a worst error of about 28 inside the gap, vs about 7 for the same call with no gap, and no warning.window_size=17still crashes with the default friedrichs kernel. I think it's because the only valid points in the middle windows are the two edge samples, and the friedrichs weight there is ~e^-500, so polyfit basically sees nothing (lhs /= scaledivides by ~0). Withkernel='gaussian'the same case runs but the derivative is worse than plain finite differences (RMSE 5.8 vs 3.7 on my signal).window_size=33is fine.test_missing_datahas this covered, but puts only four single NaNs into 31 samples, so none of this shows up in CI.Maybe raise a ValueError with a clear message when a window has fewer than degree+1 usable points, and mention the rough limit in the docstring (with the default kernel it seems to be about gap ≤ window_size − 2·(degree+1))?
A test with a contiguous gap would be good too. For what it's worth rtsdiff, splinediff and robustdiff all handled the same 15-sample gap nicely.
rtsdiff bridged it with no visible artefact. so this is really just a polydiff thing, and the paper's "sensors that drop samples need no preprocessing" sentence might want to name those methods.
Same setup as in my lineardiff issue: 0.2.3 wheel, and still the same with master installed from source.
Review reference: openjournals/joss-reviews#11172