Add Hi Goodtimes statistical filter 0 check#2716
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements Statistical Filter 0 from the Hi L1C Algorithm Document Section 2.3.2.3 to detect drastic changes in penetrating background rate that would compromise background subtraction accuracy. The filter analyzes ESA (Electrostatic Analyzer) sweeps across multiple Pointings, computing normalized AB coincidence counts per sweep, and marks sweeps where the count exceeds 150% of the median as bad times.
Changes:
- Added helper functions to detect and track ESA sweep boundaries based on ESA step transitions
- Implemented normalized count calculation per sweep with configurable TOF threshold filtering
- Added main culling function that compares current Pointing sweeps against median across surrounding Pointings
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| imap_processing/hi/hi_goodtimes.py | Implements Statistical Filter 0 with three helper functions (_get_sweep_indices, _add_sweep_indices, _compute_normalized_counts_per_sweep) and the main mark_statistical_filter_0 function that marks bad ESA sweeps based on anomalous count rates |
| imap_processing/tests/hi/test_hi_goodtimes.py | Adds comprehensive test coverage with 21 new tests across 4 test classes covering helper functions and integration scenarios including normal sweeps, anomalous sweeps, edge cases, and validation errors |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
subagonsouth
force-pushed
the
2704-hi-goodtimes---statistical-filter-0
branch
from
dede9c2 to
a0c50ac
Compare
| # Find sweep boundaries where ESA step transitions from high to low | ||
| esa_diff = np.diff(esa_step.astype(np.int32)) | ||
| # Negative diff indicates high-to-low transition (e.g., 9 -> 1 = -8) | ||
| sweep_boundaries = esa_diff < 0 |
There was a problem hiding this comment.
Some things we have noticed on other instruments are incomplete set that makes up a whole sweep. Does that happen on Hi too and do you need way to check for complete sweep set?
There was a problem hiding this comment.
This should handle incomplete sweeps just fine. It just locates where the ESA makes a negative step. Hi should always have complete sweeps though anyway.
| # Validate that we have the minimum number of datasets | ||
| if len(l1b_de_datasets) < min_pointings: | ||
| raise ValueError( | ||
| f"At least {min_pointings} valid Pointings required, " | ||
| f"got {len(l1b_de_datasets)}" | ||
| ) |
There was a problem hiding this comment.
curious why min_pointing needs to be greater than len if l1b dataset.
There was a problem hiding this comment.
I added this just as an error handler in case the batch starter were to start this job with fewer than the minimum required number of pointings. If the number of l1b_de datasets is less than the minimum required, we fail here.
| normalized_counts = counts_per_sweep / n_unique_esa_steps | ||
|
|
||
| # Remove all variables that depend on event_met dimension | ||
| ds = l1b_de.drop_dims("event_met", errors="ignore") |
There was a problem hiding this comment.
I reviewed up till here. I will review more later
| ) | ||
|
|
||
| # Validate that we have the minimum number of datasets | ||
| if len(l1b_de_datasets) < min_pointings: |
There was a problem hiding this comment.
To your point that you brought up on Monday, wouldn't this validation fail if for example we were in LOI and you didn't have one of these l1b datasets for a valid reason?
There was a problem hiding this comment.
I am still working details with Paul and the dependency handling is not going to be easy. My thought here was to ask for 3 past and three future pointings but require a minimum of 4 total. That way, we could process the final pointing before LOI with the data from it and the previous 3 pointings.
lacoak21
left a comment
lacoak21
left a comment
There was a problem hiding this comment.
This looks great Tim! I wish I had seen your code before I started adding to the ULTRA goodtimes code because you gave me a bunch of ideas :)
subagonsouth
merged commit be17eb7
into
IMAP-Science-Operations-Center:dev
Change Summary
Overview
Implements Statistical Filter 0 from Algorithm Document Section 2.3.2.3 to detect drastic changes in penetrating background rate that would compromise background
subtraction accuracy.
Algorithm
File changes
imap_processing/hi/hi_goodtimes.py
New functions:
imap_processing/tests/hi/test_hi_goodtimes.py
-imap_processing/tests/hi/test_hi_goodtimes.py
New test class TestStatisticalFilter0 with 5 tests:
Closes: #2704