search

desihub / desimeter

DESI coordinates and transformations
BSD 3-Clause "New" or "Revised" License
2 stars 4 forks source link

Filter stationary positioner locations to eliminate outliers #193

Closed schlafly closed 2 years ago

schlafly commented 2 years ago

This PR filters the stationary positioners through a simple initial outlier rejection step to try to remove any major outliers. This mode is only turned on if clip_resid > 0.

correct_using_stationary also gains a new argument return_good, which gives a boolean mask indicating which stationary positioners were used in the turbulence fit. This is intended to let the ICS know when large numbers of positioners are being excluded and either track this somehow, potentially via grafana or alert if it gets too high. In recent data the number of clipped positioners was ~40, until 20220507, when it increased to ~230.

schlafly commented 2 years ago

Thanks Stephen. I'm not actually seeing your inline comment. Are there any changes to make, or should I go ahead and merge?

sbailey commented 2 years ago

I'm not sure what happened to my comment, but it was about this bit of code that pre-dates this PR:

mask = (np.isfinite(data['x']) & np.isfinite(data['y']) &
             np.isfinite(data['dx']) & np.isfinite(data['dy']))
     data = data[mask]

astropy 5 Tables (or something post 4.0.x), switched to treating NaN as masked values instead of propagating NaN such that np.isnan no longer works. It appears that np.isfinite still works ok as a mask, but if the data table arrives via astropy instead of fitsio, I suggest some extra testing specifically with astropy 5 and inf and nan input to make sure this mask does what you want.

Otherwise it's ok with me to merge this, and even ok to merge and test this more later if you want to get the fix in sooner than later.

schlafly commented 2 years ago

Great, thanks, I'll review with Klaus on that and merge now.