FIX: ensure np.interp gets an increasing sequence in LUTs #1282
Conversation
pcdsdevices/pseudopos.py
Outdated
| fp = self._table_data_by_name[real_field] | ||
|
|
||
| # xp must be increasing | ||
| if not xp[1] > xp[0]: |
There was a problem hiding this comment.
This is very intolerant of noise in the signal, are we confident we won't jitter enough to trigger this by accident?
There was a problem hiding this comment.
These come from fixed values in a text file that can look something like this:
50 71
55 41.5
60 18.8
65 6
70 1.3
75 0.5
Since they are fixed values there won't be any jitter, but I think it's worth adding a more thorough check so we can throw a warning if the values aren't monotonic
|
|
||
| assert lut.real.limits == (0, 9) | ||
| assert lut.pseudo.limits == (40, 400) | ||
| np.testing.assert_allclose(lut.forward(60 * ps)[0], 2 * rs) |
There was a problem hiding this comment.
how did I not know np.testing existed...
There was a problem hiding this comment.
You know, I didn't actually read the functions I was adding the sign swaps to, I was only looking for numbers and figuring out whether they were the real positions or the pseudo positions, so I also didn't realize np.testing existed.
My test here is just using the pre-existing test from Ken but adding the cases where either side is decreasing instead of increasing.
| Returns True if axis 1 of arr is strictly increasing and False otherwise. | ||
| """ | ||
| # see numpy.interp docs https://numpy.org/doc/stable/reference/generated/numpy.interp.html | ||
| return np.all(np.diff(arr) > 0) |
There was a problem hiding this comment.
I guess monotonically increasing does mean strictly increasing (never decreasing or staying the same). We just had the wrong idea of "monotonically increasing" before.
There was a problem hiding this comment.
I actually re-read the docs and found that they specified strictly increasing instead of monotonically increasing, the difference being that a monotonic sequence is allowed to repeat adjacent numbers or have a zero slope.
Description
LookupTablePositionerwould fail silently if the lookup table was not strictly increasing in both axes.LookupTablePositionclass slightly to avoid code duplicationMotivation and Context
lxein XCS didn't work on master because the look up table was decreasing instead of increasing.np.interpexpects the first array to be strictly increasing.https://numpy.org/doc/stable/reference/generated/numpy.interp.html
How Has This Been Tested?
Where Has This Been Documented?
Pre-release notes and docstrings only
Pre-merge checklist