Add mark_overflow_packets to hi_goodtimes by subagonsouth · Pull Request #2703 · IMAP-Science-Operations-Center/imap_processing

subagonsouth · 2026-02-13T23:03:37Z

Change Summary

Overview

This adds a function that checks for full DE packets and removes that 8-spin period from the goodtimes dataset.

File changes

imap_processing/hi/hi_goodtimes.py
- Add a function that will search for full packets and remove them from Goodtimes if the final event is a qualified event.
imap_processing/tests/hi/test_hi_goodtimes.py
- Add test coverage for new goodtimes function

Testing

7 Unit tests added to get full coverage.

Closes: #2702

Copilot

Pull request overview

Adds a new Hi goodtimes culling routine to exclude 8-spin intervals impacted by full (overflow) Direct Event packets when the packet’s final event is “qualified” per calibration product configuration.

Changes:

Added mark_overflow_packets() to identify full DE packets (664 events) and cull corresponding goodtimes intervals when the final event is in a configured coincidence type set.
Added a new test suite (TestMarkOverflowPackets) covering overflow-packet culling scenarios and custom cull codes.
Updated imports/tests to support the new calibration config DataFrame fixture.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
`imap_processing/hi/hi_goodtimes.py`	Introduces `mark_overflow_packets()` and related logging/config parsing to cull overflow-affected 8-spin periods.
`imap_processing/tests/hi/test_hi_goodtimes.py`	Adds unit tests and fixtures for the new overflow packet culling behavior.

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

Copilot · 2026-02-13T23:07:58Z

imap_processing/tests/hi/test_hi_goodtimes.py

+        # The MET 1000 bin should have all spin bins culled
+        assert mock_goodtimes["cull_flags"].values[0, :].sum() > 0


This assertion only checks that some bins were culled, so the test would still pass if the implementation accidentally culled a single spin bin instead of the entire 8-spin period. Consider asserting that all 90 bins for the expected MET index are set to the expected cull code.

Suggested change

# The MET 1000 bin should have all spin bins culled

assert mock_goodtimes["cull_flags"].values[0, :].sum() > 0

# The MET 1000 bin should have all spin bins culled with the same nonzero code

flags_row = mock_goodtimes["cull_flags"].values[0, :]

assert np.all(flags_row == flags_row[0])

assert flags_row[0] != 0

imap_processing/tests/hi/test_hi_goodtimes.py

Copilot · 2026-02-13T23:07:59Z

imap_processing/tests/hi/test_hi_goodtimes.py

+        # Should be culled because the final event (last in list) is qualified
+        assert np.sum(mock_goodtimes["cull_flags"].values > 0) > 0


This assertion only checks that at least one bin was culled. To verify the algorithm requirement of excluding the entire 8-spin period, consider asserting all 90 bins for the expected MET index are culled (and, if applicable, that the cull code matches the default or provided cull_code).

Suggested change

# Should be culled because the final event (last in list) is qualified

assert np.sum(mock_goodtimes["cull_flags"].values > 0) > 0

# Should cull the entire 8-spin period (90 bins) because the final event (last in list) is qualified

culled_mask = mock_goodtimes["cull_flags"].values > 0

assert np.count_nonzero(culled_mask) == 90

Copilot · 2026-02-13T23:07:59Z

imap_processing/hi/hi_goodtimes.py

+        final_coin_type = coincidence_types[final_event_idx]
+
+        log_per_packet(
+            f"Packet {packet_idx} is full with qualified final event "


The per-packet log message says the final event is "qualified" before checking membership in all_valid_coin_types, so unqualified packets will be logged as qualified. Move this log line inside the if final_coin_type in all_valid_coin_types block or change the wording to reflect the actual qualification result.

Suggested change

f"Packet {packet_idx} is full with qualified final event "

f"Packet {packet_idx} is full with final event "

Copilot · 2026-02-13T23:08:00Z

imap_processing/hi/hi_goodtimes.py

+        goodtimes_ds.goodtimes.mark_bad_times(
+            met=np.array(mets_to_cull), cull=cull_code
+        )
+
+    logger.info(
+        f"Found {len(full_packet_indices)} full packet(s), "
+        f"dropped {len(mets_to_cull)} 8-spin period(s) due to overflow packets"


mets_to_cull can contain multiple event METs that map to the same goodtimes 8-spin interval (e.g., multiple DE packets share the same meta-event MET). Consider de-duplicating the times/interval indices before calling mark_bad_times and use the unique interval count in the summary log so "dropped N 8-spin period(s)" reflects actual periods culled.

Suggested change

goodtimes_ds.goodtimes.mark_bad_times(

met=np.array(mets_to_cull), cull=cull_code

)

logger.info(

f"Found {len(full_packet_indices)} full packet(s), "

f"dropped {len(mets_to_cull)} 8-spin period(s) due to overflow packets"

# De-duplicate METs so we mark each 8-spin interval only once

unique_mets_to_cull = np.unique(np.array(mets_to_cull, dtype=float))

goodtimes_ds.goodtimes.mark_bad_times(

met=unique_mets_to_cull, cull=cull_code

)

logger.info(

f"Found {len(full_packet_indices)} full packet(s), "

f"dropped {len(np.unique(np.array(mets_to_cull, dtype=float)))} 8-spin period(s) due to overflow packets"

tech3371

looks good to me. Code comments were help for someone like me who lack context.

tech3371 · 2026-02-17T15:44:56Z

imap_processing/hi/hi_goodtimes.py

+    Background:
+        Each DE packet can hold a maximum of 664 direct events. When a packet fills
+        completely, any additional events that occur are lost. If the final event
+        in a full packet has a coincidence type that is part of a defined calibration
+        product, the packet is considered to have potentially lost science-quality
+        events, and the entire 8-spin period should be excluded from analysis.


Thank you for these kinds of notes. I find them helpful!

tech3371 · 2026-02-17T15:47:57Z

imap_processing/hi/hi_goodtimes.py

+        return
+
+    # Use DEBUG level for per-packet logging if more than 10 full packets
+    log_per_packet = logger.info if len(full_packet_indices) <= 10 else logger.debug


interesting way of setting debug level

Add mark_overflow_packets to hi_goodtimes

b9d50a4

subagonsouth requested review from Copilot, lacoak21, sdhoyt and tech3371 February 13, 2026 23:03

subagonsouth self-assigned this Feb 13, 2026

subagonsouth added this to IMAP Feb 13, 2026

Copilot started reviewing on behalf of subagonsouth February 13, 2026 23:04 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

Vectorize overflow_packet algorithm

f495dea

tech3371 approved these changes Feb 17, 2026

View reviewed changes

Test that all spin-bins are culled

73f136d

subagonsouth merged commit 2b98ca7 into IMAP-Science-Operations-Center:dev Feb 18, 2026
14 checks passed

subagonsouth deleted the 2702-hi-goodtimes---mark-overflow-packets branch February 18, 2026 15:53

github-project-automation bot moved this to Done in IMAP Feb 18, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mark_overflow_packets to hi_goodtimes#2703

Add mark_overflow_packets to hi_goodtimes#2703
subagonsouth merged 3 commits intoIMAP-Science-Operations-Center:devfrom
subagonsouth:2702-hi-goodtimes---mark-overflow-packets

subagonsouth commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

Copilot AI Feb 13, 2026

Uh oh!

tech3371 left a comment

Uh oh!

tech3371 Feb 17, 2026

Uh oh!

tech3371 Feb 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		# The MET 1000 bin should have all spin bins culled
		assert mock_goodtimes["cull_flags"].values[0, :].sum() > 0

-        # The MET 1000 bin should have all spin bins culled
-        assert mock_goodtimes["cull_flags"].values[0, :].sum() > 0
+        # The MET 1000 bin should have all spin bins culled with the same nonzero code
+        flags_row = mock_goodtimes["cull_flags"].values[0, :]
+        assert np.all(flags_row == flags_row[0])
+        assert flags_row[0] != 0

		# Should be culled because the final event (last in list) is qualified
		assert np.sum(mock_goodtimes["cull_flags"].values > 0) > 0

-        # Should be culled because the final event (last in list) is qualified
-        assert np.sum(mock_goodtimes["cull_flags"].values > 0) > 0
+        # Should cull the entire 8-spin period (90 bins) because the final event (last in list) is qualified
+        culled_mask = mock_goodtimes["cull_flags"].values > 0
+        assert np.count_nonzero(culled_mask) == 90

	f"Packet {packet_idx} is full with qualified final event "
	f"Packet {packet_idx} is full with final event "

Conversation

subagonsouth commented Feb 13, 2026

Change Summary

Overview

File changes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

tech3371 left a comment

Choose a reason for hiding this comment

Uh oh!

tech3371 Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

tech3371 Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants