Improvement for TEC correction - #369
seongsujeong wants to merge 18 commits into
Conversation
…pside TEC rejection logic
hfattahi
left a comment
hfattahi
left a comment
There was a problem hiding this comment.
some comments up to what we reviewed today.
bhawkins
left a comment
bhawkins
left a comment
There was a problem hiding this comment.
Polynomial fitting might not be robust to outliers, so another approach like median filter or RANSAC algorithm might be better.
@gshiroma suggests checking that the related output layers (az and rg corrections, but not TEC, which get geocoded) in the GCOV product still make sense in all cases.
In GCOV, disabling either correction (i.e., the azimuth or slant-range correction) causes the corresponding correction LUT to be omitted from the output. @gshiroma Please confirm if this is expected behavior. |
rad-eng-59
left a comment
rad-eng-59
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the updates. Just some minor comments.
|
|
||
| # Keep only inliers, but fall back to the plain fit if too few survive. | ||
| inliers = np.abs(resid) <= threshold | ||
| if np.count_nonzero(inliers) <= degree: |
There was a problem hiding this comment.
Shouldn't it be "degrees + 1" instead? A polynomial with degree-n should require at least n + 1 samples.
There was a problem hiding this comment.
<= will make trigger the log message below when the number of inliers are the same as the order. For example, if the order is 2 and we have 2 inliers, the if statement becomes True. If 3 or more, it satisfies the condition to perform 2nd order polynomial fitting.
| # Degenerate noise scale (near-perfect fit); nothing to reject. | ||
| if not np.isfinite(threshold) or threshold <= 0: | ||
| info_channel.log('TEC MAD Threshold is not valid. Keeping all TEC samples for polynomial fitting.') | ||
| return np.polyval(coeffs, x) |
There was a problem hiding this comment.
| return np.polyval(coeffs, x) | |
| raise RuntimeError() |
If we get here I'm not sure it's safe to evaluate the polynomial. So I'd suggest either return y or just throw an exception.
There was a problem hiding this comment.
I think it would make sense to return the original values when any smoothing attempt fails. Not only this one but also the one in the below (not enough inliers)
| inliers = np.abs(resid) <= threshold | ||
| if np.count_nonzero(inliers) <= degree: | ||
| info_channel.log('Not enough iners from TEC sample. Keeping all TEC samples for polynomial fitting.') | ||
| return np.polyval(coeffs, x) |
There was a problem hiding this comment.
| return np.polyval(coeffs, x) | |
| return y |
This condition suggests the fit is bad, so maybe it's better not to apply polynomial smoothing and just return the data. Should mention the behavior in the docstring.
There was a problem hiding this comment.
Now I'm having some doubts about what to do in this case.
There was a problem hiding this comment.
Not having enough inliers mean the TEC profile is messed up (which is rare). In that case, it would be hard to tell which one performs better (i.e. fitted or not fitted). In that case I think it's safe to leave the TEC values as they are.
| return delta_r | ||
|
|
||
|
|
||
| def _mad_polyfit(x: np.ndarray, y: np.ndarray, degree: int, |
There was a problem hiding this comment.
| def _mad_polyfit(x: np.ndarray, y: np.ndarray, degree: int, | |
| def _smooth_mad_polyfit(x: np.ndarray, y: np.ndarray, degree: int, |
The function doesn't return fit coefficients like numpy.polyfit, so I think it's clearer if the name is more different. The function performs smoothing, so _smooth_mad_polyfit makes more sense to me, at least.
5 participants
This PR adds several options to ionospheric TEC correction
EDIT
09/10/2026: RANSAC-based outlier detection is implemented and being applied before polynomial fitting. Test is ongoing internally.