MAG L1C: continue previous day's timeline across the day boundary (#2925) by sapols · Pull Request #10 · sapols/imap_processing

sapols · 2026-06-22T18:33:23Z

Summary

Rework of this draft following @maxinelasp's review and her new day-boundary unit tests (IMAP-Science-Operations-Center#3318): a much simpler implementation that continues the previous day's normal-mode timeline across the day boundary whenever the current day opens with a gap. The earlier neighbor_datasets / derive_inherited_timeline machinery, the L1C-neighbor (T018) path, and the synthetic validation fixtures are all removed.

What it does

mag_l1c() and process_mag_l1c() accept an optional previous_day_dataset (the previous day's norm L1B for the same sensor). When the processing day opens with a gap - including days with no normal-mode data at all - the timestamps generated for that gap continue the previous day's grid: anchored on its last sample within its own 24-hour day (trailing buffer samples past midnight are excluded), stepping at its ending cadence (snapped against the known MAG rates with _is_expected_rate). Only the clock is inherited; neighbor vectors never enter the output. Without a usable previous-day file, single-day processing still succeeds unchanged; the one behavioral difference that applies even on such days is the timestamp-precision fix described under Tests (gap-fill timestamps at 8+ vectors per second can move by up to 128 ns relative to previously generated products).

On the CLI side, the Mag L1C branch splits dependencies by start date using ProcessingInputCollection.get_valid_inputs_for_start_date: current-day L1B files are the one-or-two inputs exactly as before, and a previous-day norm L1B (delivered by a date_range entry in imap_mag_dependencies.yaml, see companion) routes to previous_day_dataset. Mode/sensor validation lives in mag_l1c, which ignores unusable neighbors with a warning.

Review feedback incorporated

All inline comments from the previous revision are addressed: no keyword-only marker and both optional datasets trail the signature, the no-neighbor behavior is documented, the redundant branch in mag_l1c() is gone with logging moved into process_mag_l1c(), _day_window_ns is renamed _expected_day_ns and the duplicate test helper _ttj2000_day_bounds is deleted in favor of it, cadence detection loops the known rates through _is_expected_rate, the anchor is the last timestamp inside the previous 24-hour day, inheritance triggers on any gap at the beginning of the day (not only no-NM days), and the CLI helper is gone in favor of get_valid_inputs_for_start_date with validation inside mag_l1c. Per the review summary, only the previous day is used and only L1B is accepted.

Tests

The two tests from IMAP-Science-Operations-Center#3318 are cherry-picked here with authorship preserved and their xfail markers removed; both pass. Two details they surfaced, flagged for @maxinelasp:

generate_missing_timestamps routed integer gap bounds through np.rint (float64), which shifts generated timestamps by up to 64 ns at 2025-era TTJ2000 magnitudes. Fixed here; without it the zero-jitter assertions cannot hold. Note this fix is shared machinery: for normal-mode rates of 8+ vectors per second (whose periods are not multiples of 128 ns), gap-fill timestamps can now differ from previously generated products by up to 128 ns and occasionally by one row at a gap edge - the new values are the exact intended grid. Rates of 1, 2, and 4 vectors per second are byte-identical to before.
The fixture's phase offset moved from 73_000_000 to 73_000_064 ns. The pipeline stores epochs in float64 columns whose spacing at these magnitudes is 128 ns, so a non-multiple-of-128 offset makes every grid point a rounding tie whose stored value jitters by 64 ns - the exact-equality assertions are then unsatisfiable for any implementation. If a tolerance-based assertion (atol=128 ns) is preferable to the nudged constant, happy to switch.

New unit tests cover: unusable neighbors ignored (burst mode, wrong sensor, L1C source, unknown cadence), whole-day inheritance when no NM data exists, buffer samples past midnight excluded from the anchor, and CLI routing with and without a previous-day dependency.

Scope deferred to a follow-up issue

T018 (inheriting from the previous day's L1C, which needs an L1C-on-L1C dependency design from the infra team), the T019 vs T024 spec conflict, MAG-approved validation data for T017-T019, and gap management beyond gaps at the beginning of the day.

Companion (infrastructure)

sapols/sds-data-manager#1 adds date_range: ["-1d", "0d"] to the MAG L1C norm L1B inputs in imap_mag_dependencies.yaml so the previous day's file is actually delivered in production. Until that merges and deploys, the inheritance path stays dormant and processing is unchanged.

Verification

Full non-external test suite passes (1578 passed, 4 skipped, 16 xfailed, plus the pre-existing IMAP-Science-Operations-Center#3215 xpass); pre-commit run --all-files is clean.

maxineofficial

The goal is generally to have one timeline that is continuous across modes and across days, so a complete set of MAG L1C files is a single, continuous timeline.

Obviously, this is pretty much impossible. Gaps, timeline drift, etc all move the timeline around a little. But it's good to remember that's the goal.

This should be okayed with the MAG team, but I think it is reasonable to only use timeline data from the previous day here. Trying to retrieve it from future days adds a lot of complexity, and I'm not sure it makes sense to do right now, since if day N-1 and day N+1 misalign, then the misalignment has to happen somewhere. That is, if N-1 is offset from exactly one second by 0.001, and N+1 is offset by 0.002, as the code currently is written, there has to be a jump from 0.001 to 0.002 somewhere. Putting that at the day boundary makes as much sense as anywhere else, and if future improvements smooth those jumps out then we can add future days at that point.

So, that simplifies this code somewhat, by making it so you only have an optional previous day rather than a list of days.

To simplify further - I think it is probably safe to always use L1B instead of L1C for the previous day. As you mention in the docstrings, L1C could have interpolated timestamps which would not be ideal, and it would require infrastructure updates, which is also annoying.

Again, this is something that should be discussed with the MAG team in more detail, but from an incremental improvement standpoint, I think it makes sense to limit the changes in this way.

maxineofficial · 2026-06-29T20:17:49Z

    first_input_dataset: xr.Dataset,
    day_to_process: np.datetime64,
    second_input_dataset: xr.Dataset = None,
+    *,


I'd probably slightly prefer not to use the asterisk param here, but if you want to keep it, I think both the optional, addiitonal datasets should be after it (ie move down second_input_dataset)

Dropped the asterisk. The signature is now
mag_l1c(first_input_dataset, day_to_process, second_input_dataset=None, previous_day_dataset=None), with both optional datasets at the end.

maxineofficial · 2026-06-29T20:19:24Z

+        the ``date_range`` entry in ``imap_mag_dependencies.yaml``. When the current day
+        has no usable normal mode data in the processing window, the L1C timeline
+        inherits its cadence and phase from a neighbor (the T017/T018 "time inheritance"
+        cases in the SDC Data Validation Document). See ``derive_inherited_timeline``.


Add a note about what happens if this is not provided

Added to both docstrings: without a (usable) previous-day dataset, gaps at the start of the day
fill on the current day's own grid, exactly as before

maxineofficial · 2026-06-29T20:23:34Z

+
    interp_function = InterpolationFunction[configuration.L1C_INTERPOLATION_METHOD]
-    if burst_mode_dataset is not None:
+    if inherited_timeline is not None and burst_mode_dataset is not None:


Is this if statement necessary? If inherited_timeline is None, that's the default in process_mag_l1c anyway.

So if this if statement is true, you pass in inherited_timeline. If it's false, you don't pass in inherited_timeline, meaning it defaults to None. I'd say remove this if statement and move the logger into process_mag_l1c.

You were right and the branch is gone entirely. mag_l1c() now just passes previous_day_dataset through to process_mag_l1c(), which logs when inheritance is actually
applied

maxineofficial · 2026-06-29T20:24:33Z

+    source: str
+
+
+def _day_window_ns(day_to_process: np.datetime64) -> tuple[int, int]:


Suggested change

def _day_window_ns(day_to_process: np.datetime64) -> tuple[int, int]:

def _expected_day_ns(day_to_process: np.datetime64) -> tuple[int, int]:

maxineofficial · 2026-06-29T20:32:42Z

+        True if at least one normal mode epoch falls within the window.
+    """
+    epoch = normal_mode_dataset["epoch"].data
+    return bool(np.any((epoch >= day_start_ns) & (epoch <= day_end_ns)))


This is such a nitpick, but since epoch is guaranteed monotonic increasing, you could just check the first and last item in the list

That helper (has_usable_norm_in_window) is gone now; the rework doesn't gate on "no usable NM in window" at all. Inheritance now hooks into the leading gap that find_all_gaps already detects

maxineofficial · 2026-06-29T21:17:37Z

    return normal_mode_dataset, burst_mode_dataset


+class InheritedTimeline(NamedTuple):


Can you document these values? in particular, what does 'anchor_ns' refer to?

The NamedTuple is gone now; _previous_day_grid's docstring now defines the anchor (the previous day's last normal-mode timestamp within its own 24-hour day) and the inherited grid (anchor + k * period)

maxineofficial · 2026-06-29T21:19:12Z

+    if abs(rate - observed_rate) > rate * L1C_TIMESTAMP_GAP_TOLERANCE:
+        return None
+
+    return InheritedTimeline(rate=rate, anchor_ns=int(epoch[-1]), source="l1c")


Sorry to make this more complicated... but I'm wondering if anchor_ns should actually be the last timestamp in the 24 hour day, not the last timestamp in the buffer.

Adopted! The anchor is now the last sample before the current day's midnight, so trailing buffer samples are excluded. There's a unit test for exactly this (test_process_mag_l1c_previous_day_anchor_ignores_buffer_samples)

maxineofficial · 2026-06-29T21:35:50Z

            normal_vecsec_dict = None

        gaps = find_all_gaps(norm_epoch, normal_vecsec_dict, day_start_ns, day_end_ns)
+    elif inherited_timeline is not None:


Why is this an elif? The inherited timeline should be used any time there is a gap at the beginning of the data.

This reshaped the whole design: inheritance now triggers whenever find_all_gaps reports a gap starting at the window start, whether the day has NM data (your new tests) or none at all (the whole window is then one leading gap and continues the previous day's grid)

maxineofficial · 2026-06-29T21:51:44Z

        return datasets


+def _collect_mag_l1c_inputs(


MAG specific functions should go inside the MAG class

maxineofficial · 2026-06-29T21:54:54Z

+    """
+    current_day_inputs: list[xr.Dataset] = []
+    neighbor_datasets: list[xr.Dataset] = []
+    for data_type in ("l1b", "l1c"):


Can't this just be divided by start_date? If the file is from the current day (as defined by self.start_time), it's a current_day_inputs, otherwise it's neighbor_datasets. Then you can move some of this validation checking into mag_l1c() itself instead of cluttering up the CLI, and use get_valid_inputs_for_start_date instead of making a new function here.

Done. Current-day inputs come from get_valid_inputs_for_start_date(start_date) and the neighbor from the same call with start_date - 1 day; mode/sensor validation moved into mag_l1c(), which warns and ignores rather than failing so single-day processing always succeeds

maxineofficial · 2026-06-30T18:01:15Z

+    source: str
+
+
+def _day_window_ns(day_to_process: np.datetime64) -> tuple[int, int]:


This is a duplicate of a function in test_mag_l1c (_ttj2000_day_bounds) so remove the test function and point usage here instead.

Done like you suggested: the test helper is deleted and the tests (including your new ones) import _expected_day_ns now

…AP-Science-Operations-Center#2925) When the processing day opens with a gap, generate the missing normal-mode timestamps on the previous day's grid - inheriting its ending cadence and phase from an optional previous-day norm L1B dataset - so the L1C timeline stays regular across the day boundary. With no (usable) previous-day file, behavior is unchanged and single-day processing still succeeds. - mag_l1c()/process_mag_l1c() gain an optional previous_day_dataset. The inherited grid anchors on the last sample within the previous 24-hour day and snaps its cadence against the known MAG rates via _is_expected_rate. With no normal-mode data at all, the whole window continues that grid. - Mag CLI splits L1C dependencies by start date with get_valid_inputs_for_start_date; the previous day's norm L1B (delivered via a date_range entry in imap_mag_dependencies.yaml) routes to previous_day_dataset. Mode/sensor validation lives in mag_l1c. - generate_missing_timestamps no longer routes integer nanosecond gap bounds through float64 (np.rint), which shifted generated timestamps by up to 64 ns at TTJ2000 scales. - Un-xfail the day-boundary tests from IMAP-Science-Operations-Center#3318; their phase offset moves to a multiple of 128 ns because the float64 epoch columns cannot represent other offsets exactly at 2025-era epochs. Shared test helper _ttj2000_day_bounds is replaced by the production _expected_day_ns.

sapols · 2026-07-07T18:30:57Z

@maxinelasp Thanks for the review and for suggesting this simplified approach. I reworked the PR to do everything you suggested (and updated the description accordingly): only one optional previous-day dataset (no list, no future days), and only norm L1B is accepted; an L1C (or burst, or wrong-sensor) neighbor is ignored with a warning so we still process with one day of data. I'll open a follow-up issue to track the L1C-inheritance variant (T018), the T019 vs T024 spec conflict, and everything else needing MAG input.

Additionally, your tests caught a real precision bug, and surfaced a storage limitation. Flagging both since it required a small change to your test code:

Real bug your tests caught: generate_missing_timestamps sent integer gap bounds through np.rint, which returns float64. At 2025-era TTJ2000 magnitudes, that shifted every generated timestamp by up to 64 ns. Fixed; your zero-jitter assertions can't hold without it. One product-side heads-up that comes with the fix: regenerated L1C files can show gap-fill timestamps shifted by up to 128 ns at 8+ vec/s rates compared to previously generated products (that's the corrected exact grid, not a regression, so don't be alarmed if a diff against older files flags it).
One constant changed in _build_cross_day_l1b(): I changed phase_ns from 73_000_000 to 73_000_064. The pipeline stores epochs in float64 columns, and at these magnitudes float64 can only represent multiples of 128 ns. With a 73 ms offset every grid point is a rounding tie, so stored timestamps jitter +/-64 ns and the exact-equality assertions are unsatisfiable for any implementation. A multiple-of-128 offset keeps the test's intent fully. If you'd rather keep 73 ms and use tolerance-based asserts (atol=128 ns), I'm happy to do that instead. Longer-term, if MAG cares about exact nanoseconds, carrying the timeline as int64 through the pipeline could go in the follow-up issue (I'll mention it in the issue description).

sapols · 2026-07-07T18:52:30Z

+        gap_end = np.rint(gap_end)
    output: np.ndarray = np.arange(
-        int(np.rint(gap[0])),
-        int(np.rint(gap[1])),


This was the precision problem in generate_missing_timestamps the new tests uncovered

sapols · 2026-07-07T18:54:17Z

+    # nanoseconds is 128 ns. A non-multiple offset makes every grid point a rounding
+    # tie, so the stored timestamps jitter +/- 64 ns and exact-equality assertions
+    # cannot hold for any implementation.
+    phase_ns = 73_000_064


This is the one constant value I changed

- Ignore (with a warning) a previous day dataset that has no epoch variable instead of raising KeyError. - Log skipped inheritance (too few pre-midnight samples, unknown cadence) at warning level to match the other unusable-neighbor paths. - Select the previous-day file by the job descriptor (norm + sensor) rather than "norm" alone, so a wrong-sensor file can never shadow the right one. - Drop the "exactly as before" claim from the mag_l1c docstring: the generate_missing_timestamps precision fix means gap fills at 8+ vec/s can differ from previously generated products by up to 128 ns even without a previous-day file.

sapols mentioned this pull request Jun 22, 2026

MAG L1C: depend on previous day's normal-mode L1B for time inheritance (#2925) sapols/sds-data-manager#1

Closed

sapols force-pushed the claude/issue-2925-v2 branch 4 times, most recently from 5aa6e0b to 0969b8f Compare June 26, 2026 02:26

maxineofficial reviewed Jun 29, 2026

View reviewed changes

maxineofficial reviewed Jun 30, 2026

View reviewed changes

maxineofficial and others added 2 commits July 6, 2026 11:35

Adding MAG L1C day boundary tests

485f56e

sapols force-pushed the claude/issue-2925-v2 branch from 0969b8f to ac9fdb5 Compare July 6, 2026 17:58

sapols changed the title ~~MAG L1C: support cross-day inherited timeline (T017/T018) (#2925)~~ MAG L1C: continue previous day's timeline across the day boundary (#2925) Jul 6, 2026

sapols commented Jul 7, 2026

View reviewed changes

sapols mentioned this pull request Jul 7, 2026

MAG L1C: continue previous day's timeline across the day boundary IMAP-Science-Operations-Center/imap_processing#3323

Open

2 tasks

sapols closed this Jul 7, 2026

		source: str


		def _day_window_ns(day_to_process: np.datetime64) -> tuple[int, int]:

	def _day_window_ns(day_to_process: np.datetime64) -> tuple[int, int]:
	def _expected_day_ns(day_to_process: np.datetime64) -> tuple[int, int]:

		return normal_mode_dataset, burst_mode_dataset


		class InheritedTimeline(NamedTuple):

Conversation

sapols commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What it does

Review feedback incorporated

Tests

Scope deferred to a follow-up issue

Companion (infrastructure)

Verification

Uh oh!

maxineofficial left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sapols Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sapols commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sapols commented Jun 22, 2026 •

edited

Loading

sapols Jul 7, 2026 •

edited

Loading

sapols commented Jul 7, 2026 •

edited

Loading