Skip to content

Minimally Fix MAG L1C gap-fill bugs: pass validation tests T015 and T016#2885

Closed
sapols wants to merge 1 commit intodevfrom
claude/mag-l1c-fix-minimal
Closed

Minimally Fix MAG L1C gap-fill bugs: pass validation tests T015 and T016#2885
sapols wants to merge 1 commit intodevfrom
claude/mag-l1c-fix-minimal

Conversation

@sapols
Copy link
Copy Markdown

@sapols sapols commented Mar 30, 2026

Summary (From Claude)

Five surgical bug fixes to the existing MAG L1C gap-fill code to un-skip and pass validation tests T015 and T016, while keeping T013, T014, and T024 passing. No architectural changes, no new functions or classes, no rewrite of the pipeline flow.

This supersedes the earlier, heavier draft PR at sapols#2 (claude/mag-l1c-fix), which rewrote the gap-fill architecture with new dataclasses, helper functions, and a BM-only algorithm. After feedback from @alastairtree and @maxinelasp -- that the existing code was developed collaboratively with the MAG team and that deviations from the algorithm document may be intentional simplifications -- we went back to dev and identified the smallest set of changes needed to make the validation tests pass. The result is ~25 lines changed across 4 files instead of hundreds.

Closes #1993

Bug fixes

Fix A: 2D array indexing in interpolate_gaps() (mag_l1c.py)

filled_norm_timeline is an (n, 8) array, but the gap-timeline extraction was comparing the entire 2D array against scalar timestamps without specifying a column. The boolean mask broadcasted across all 8 columns, producing garbled results.

Fix: Index column 0 explicitly: filled_norm_timeline[:, 0][...]

Fix B: Off-by-one burst slice in interpolate_gaps() (mag_l1c.py)

burst_end was clamped to len(burst_epochs) - 1, but the subsequent Python slice [burst_start:burst_end] excludes the right endpoint, silently dropping the last burst sample from interpolation.

Fix: Use burst_start : burst_end + 1 in slices.

Fix C: CIC filter edge loss in linear_filtered() (interpolation_methods.py)

After the CIC filter truncates samples to compensate for group delay, the filtered timestamps are shorter than the originals. linear() then calls remove_invalid_output_timestamps(), which clips valid output timestamps to the now-shorter filtered range -- discarding data at the edges that should be extrapolated.

Fix: Pass extrapolate=True to linear(), matching the spec reference CIC code which uses IUS (extrapolates by default).

Fix D: Spurious micro-gaps at config mode transitions (mag_l1c.py)

When MAG switches vector rates, the declared rate-transition timestamp does not perfectly align with the actual cadence change in the data. find_all_gaps() splits the timeline at declared transitions, and find_gaps() misidentifies the boundary misalignment as 1-sample gaps. These phantom gaps cause downstream interpolation failures.

Fix: Filter out gaps whose duration spans at most 2 cadence periods (i.e., a single missing sample artifact) after find_all_gaps() returns.

Fix E: Hardcoded 0.5s cadence in generate_missing_timestamps() (mag_l1c.py)

Gap-fill timestamps were always generated at 0.5s (2 Hz) intervals regardless of the gap declared vector rate. For rate=4 (0.25s) or rate=1 (1.0s) gaps, this produced the wrong number of timestamps and misaligned with the MAG team expected validation outputs.

Fix: Read the rate from the gap descriptor third element (gap[2]) and compute cadence as int(1e9 / vectors_per_second). The int() cast also fixes nanosecond floating-point precision issues in np.arange().

Bonus: Dead code removal

Removed a no-op np.delete(filled_norm_timeline, timeline_index) call whose return value was never assigned.

Files changed

File Lines What
mag/l1c/mag_l1c.py ~15 Fixes A, B, D, E + dead code removal
mag/l1c/interpolation_methods.py 3 Fix C (extrapolate=True)
tests/mag/test_mag_validation.py -3 Remove pytest.skip() guard for T015/T016
tests/mag/test_mag_l1c.py ~10 Update unit test expectations for rate-aware cadence

Test results

All 102 MAG tests pass, including the previously-skipped T015 and T016 for both mago and magi:

102 passed, 32 warnings in 17.83s

Why this approach instead of the earlier PR

The earlier PR (sapols#2) took a comprehensive approach: rewriting the gap-fill pipeline with GapFillPlan dataclasses, build_gap_fill_plan(), _find_rate_segments(), and a BM-only interpolation algorithm. While technically sound, it replaced code that was developed in close collaboration with the MAG instrument team and made assumptions about which deviations from the algorithm document were bugs vs. intentional simplifications.

This PR takes the opposite approach: start from the existing code, identify the specific bugs preventing T015/T016 from passing, and fix only those. The 5 bugs found are all unambiguous correctness issues (wrong array dimensions, off-by-one slicing, clipped interpolation range, phantom gap detection, wrong cadence). No judgment calls about algorithm design were needed.

Five surgical bug fixes to the existing L1C gap-fill code, with no
architectural changes, to un-skip and pass validation tests T015/T016
while keeping T013, T014, and T024 passing.

Fixes: #1993

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sapols sapols changed the title Fix MAG L1C gap-fill bugs: pass validation tests T015 and T016 Minimally Fix MAG L1C gap-fill bugs: pass validation tests T015 and T016 Mar 30, 2026
@sapols sapols closed this Mar 30, 2026
@sapols sapols deleted the claude/mag-l1c-fix-minimal branch March 30, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG - MAG L1C validation tests

1 participant