HERA-Team / hera_qm

HERA Data Quality Metrics
MIT License
2 stars 2 forks source link

Update detrend_medfilt to use scipy.ndimage.median_filter #389

Closed jsdillon closed 3 years ago

jsdillon commented 3 years ago

In running H1C_IDR3 with the most recent pyuvdata I'm getting segfaults in XRFI that I couldn't really understand. I traced them down to the median filter and it's handling of infs. As the documentation for scipy.signal.medfilt says, the more general scipy.ndimage.median_filter. It doesn't appear to be having the same segfaults and I was able to rewrite detrend_medfilt to almost exactly reproduce the results.

There is a slight different in the implementation, which comes down to this line: https://github.com/HERA-Team/hera_qm/blob/88d42b02bc55e726dd8c60a4816af87ee24f99ba/hera_qm/xrfi.py#L416

In the old version, sig was computed for an array that was previously padded out. Here, d_sq is the size of the original data, so the reflecting pad happens again. This gives slightly different results, though I think this version is at least as justifiable in its handling of the edges. At any rate, I had to replace the "answer" files for the unit tests, and since I was doing that, I decided to write functions that have non-zero second derivatives to make the detrend_medfilt less likely to give 0s. I've verified that outside the edge regions the new and old code give the same result.

codecov[bot] commented 3 years ago

Codecov Report

Merging #389 (a45e5f5) into master (88d42b0) will decrease coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #389      +/-   ##
==========================================
- Coverage   97.06%   97.06%   -0.01%     
==========================================
  Files           8        8              
  Lines        3100     3098       -2     
==========================================
- Hits         3009     3007       -2     
  Misses         91       91              
Impacted Files Coverage Δ
hera_qm/xrfi.py 99.78% <100.00%> (-0.01%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 88d42b0...a45e5f5. Read the comment docs.

jsdillon commented 3 years ago

As a codicil to this PR, here's flags before this change and after on 2458041:

Before: image (4)

After: image (5)