feat: new simultaneous inpainter #951
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #951 +/- ##
==========================================
+ Coverage 97.18% 97.20% +0.01%
==========================================
Files 30 30
Lines 10728 10793 +65
==========================================
+ Hits 10426 10491 +65
Misses 302 302
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Just some minor comments, but this generally looks good to me. Thanks @steven-murray!
hera_cal/lst_stack/averaging.py
Outdated
| stackf: np.ndarray, | ||
| stackn: np.ndarray, | ||
| base_noise_var: np.ndarray, | ||
| df: un.Quantity['frequency'], |
There was a problem hiding this comment.
I think this should just be a float for this function. At minimum, I don't want to force us to use .value... maybe we can make it flexible?
There was a problem hiding this comment.
Made it flexible (if a float, assume it's in Hz) and added docs.
hera_cal/lst_stack/averaging.py
Outdated
| max_convolved_flag_frac: float = 0.667, | ||
| use_unbiased_estimator: bool = False, | ||
| sample_cov_fraction: float = 0.0, | ||
| filter_half_widths: Sequence[float] = [0.1], |
There was a problem hiding this comment.
I don't think this is a reasonable default...
There was a problem hiding this comment.
I'm not sure if it should have a default, TBH.
There was a problem hiding this comment.
You're right -- I've removed the default.
hera_cal/lst_stack/averaging.py
Outdated
| sample_cov_fraction : float | ||
| A factor to use to down-weight off-diagonal elements of the sample covariance. | ||
| filter_half_widths : Sequence[float] | ||
| The half-widths of the DPSS filters. |
There was a problem hiding this comment.
Added that info to the docstring.
hera_cal/lst_stack/averaging.py
Outdated
|
|
||
| # Get median nsamples across the band | ||
| nsamples_by_night = np.median(stackn, axis=1, keepdims=True) | ||
| assert np.all(nsamples_by_night == stackn), 'This code assumes that nsamples is constant over frequency for a given night and baseline.' |
There was a problem hiding this comment.
We should probably turn this into a proper raised error
hera_cal/lst_stack/averaging.py
Outdated
| continue | ||
|
|
||
| # if there are too-large flag gaps even after a simple LST-stacking, continue | ||
| max_allowed_gap_size = max_gap_factor * filter_half_widths[0]**-1 / df.value |
This puts @tyler-a-cox's new simultaneous inpainter into
hera_cal.lst_stack.averaging.I refactored it a little so that the algorithm itself, that acts on a single baseline, is its own function. This makes testing it much easier, since you just have to create data arrays that are shape (nnights, nfreqs).
I've put a number of test cases in (as of now, not all of them pass...)