2822 hi l1b goodtimes implement bad tdc cal culling algorithm

[subagonsouth]

Summary

• Implement mark_bad_tdc_cal culling algorithm - Add new goodtimes filter that removes times with failed TDC calibration based on DIAG_FEE packet data
• Integrate L1A DIAG_FEE dependency - Add l1a_diagfee parameter to hi_goodtimes() and _apply_goodtimes_filters() functions
• Update CLI to load DIAG_FEE data - Add file path retrieval and CDF loading for L1A DIAG_FEE in the Hi goodtimes processing path
• Reorder hi_goodtimes parameters - Move current_repointing to first positional argument for consistency

Details

mark_bad_tdc_cal Algorithm

Based on C reference (drop_bad_tdc_diagfee in culling.c):

• Scans DIAG_FEE packets chronologically and checks TDC1/TDC2/TDC3 calibration status (bit 1 = success)
• If any checked TDC has failed calibration, marks all times from that DIAG_FEE packet until the next as bad
• Skips duplicate DIAG_FEE packets that appear within 10 seconds (quirk when entering HVSCI mode)
• Requires at least 2 DIAG_FEE packets to perform checking

Filter Order Update

Goodtimes filters now apply in order:

1. mark_incomplete_spin_sets
2. mark_drf_times
3. mark_bad_tdc_cal (new)
4. mark_overflow_packets
5. mark_statistical_filter_0
6. mark_statistical_filter_1
7. mark_statistical_filter_2

Closes: #2822

[Copilot]

Pull request overview

Adds IMAP-Hi goodtimes culling based on TDC calibration status from L1A DIAG_FEE data, wiring the new dependency through the CLI processing path and expanding tests to cover the new filter and updated call signature.

Changes:

• Extend hi_goodtimes() / _apply_goodtimes_filters() to accept L1A DIAG_FEE and apply a new mark_bad_tdc_cal() filter.
• Update CLI Hi L1B goodtimes processing to require/load a DIAG_FEE CDF and pass it into hi_goodtimes().
• Add comprehensive unit tests for mark_bad_tdc_cal() and update existing CLI/goodtimes tests for the new dependency and argument order.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
imap_processing/hi/hi_goodtimes.py	Adds mark_bad_tdc_cal, threads DIAG_FEE through goodtimes orchestration, and updates filter ordering/docs.
imap_processing/cli.py	Fetches/validates a single L1A DIAG_FEE dependency for Hi goodtimes and passes the loaded dataset into hi_goodtimes().
imap_processing/tests/test_cli.py	Updates Hi goodtimes CLI test to include DIAG_FEE input and validate the new hi_goodtimes() call signature.
imap_processing/tests/hi/test_hi_goodtimes.py	Adds new test suite for mark_bad_tdc_cal and updates existing goodtimes tests to include the DIAG_FEE dependency.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

In _apply_goodtimes_filters()’s docstring, the text still says it applies filters 1–6, but the implementation now applies 7 filters (including mark_bad_tdc_cal). Please update that docstring sentence so the documented behavior matches the code.

[Copilot]

The early-return check uses len(diagfee.epoch) < 2 but the warning message says "No DIAG_FEE data...". When there is exactly 1 packet this message is misleading; consider updating it to indicate "insufficient DIAG_FEE packets" (and optionally include the count) to aid debugging.

Suggested change

	logger.warning("No DIAG_FEE data to use for selecting good times")
	logger.warning(
	"Insufficient DIAG_FEE packets to select good times (found %d, need at least 2)",
	len(diagfee.epoch),
	)

[Copilot]

test_loads_cal_config patches the other filter functions but not mark_bad_tdc_cal, even though _apply_goodtimes_filters now calls it. This makes the test depend on mark_bad_tdc_cal’s current early-return behavior with a MagicMock DIAG_FEE. Consider patching mark_bad_tdc_cal here as well to keep the test focused on calibration config loading and avoid future brittleness.