Skip to content

2822 hi l1b goodtimes implement bad tdc cal culling algorithm#2835

Merged
subagonsouth merged 7 commits intoIMAP-Science-Operations-Center:devfrom
subagonsouth:2822-hi-l1b-goodtimes---implement-bad-tdc-cal-culling-algorithm
Mar 24, 2026
Merged

2822 hi l1b goodtimes implement bad tdc cal culling algorithm#2835
subagonsouth merged 7 commits intoIMAP-Science-Operations-Center:devfrom
subagonsouth:2822-hi-l1b-goodtimes---implement-bad-tdc-cal-culling-algorithm

Conversation

@subagonsouth
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

# Based on sample code in culling_v2.c, skip this check if we have fewer
# than two diag_fee packets.
if len(diagfee.epoch) < 2:
logger.warning("No DIAG_FEE data to use for selecting good times")
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
# Check TDC calibration status (bit 1: 1=good, 0=bad)
any_tdc_failed = False

if check_tdc1 and (diagfee["tdc1_cal_ctrl_stat"].values[i] & 2) == 0:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why you are & with 2. Can you add comment about it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a binary and operation. I think that the comment "(bit 1: 1=good, 0=bad)" explains it?

@subagonsouth subagonsouth requested a review from tech3371 March 23, 2026 22:55
Copy link
Copy Markdown
Contributor

@tech3371 tech3371 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for refactoring. It's easier to follow!

subagonsouth and others added 2 commits March 24, 2026 10:02
Co-authored-by: Tenzin Choedon <36522642+tech3371@users.noreply.github.com>
@subagonsouth subagonsouth merged commit 8fc4fa8 into IMAP-Science-Operations-Center:dev Mar 24, 2026
14 checks passed
@github-project-automation github-project-automation bot moved this to Done in IMAP Mar 24, 2026
@subagonsouth subagonsouth deleted the 2822-hi-l1b-goodtimes---implement-bad-tdc-cal-culling-algorithm branch March 24, 2026 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Hi L1B Goodtimes - implement bad TDC cal culling algorithm

3 participants