IDEX: algorithm updates to more accurately identify dust hits - #3446
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Some saturated peaks bypass lower-gain width measurement and can produce false dust-hit classifications.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates IDEX dust-hit detection to better handle complex TOF waveforms.
Changes:
- Scans all gain channels with prominence and width criteria.
- Adds saturation and truncated-baseline handling.
- Adds regression tests for new detection paths.
File summaries
| File | Description |
|---|---|
imap_processing/idex/idex_event_flags.py |
Implements revised dust-hit detection. |
imap_processing/tests/idex/test_idex_event_flags.py |
Adds regression coverage. |
Review details
Suppressed comments (2)
imap_processing/idex/idex_event_flags.py:316
- The truncated-baseline path measures every candidate directly on the possibly saturated waveform, so it loses the saturation-aware handling used by the standard path. Clipping lowers the effective half-height and can make narrow peaks satisfy the 20 ns requirement, producing false dust hits specifically when this fallback is needed. Match saturated peaks to a usable lower gain and measure there before counting them.
peaks = _collapse_saturated_peaks(peaks, waveform, corrected)
widths = [_fwhm(corrected, times, int(peak_index)) for peak_index in peaks]
if sum(width >= _MIN_PEAK_WIDTH_US for width in widths) >= _MIN_PEAK_COUNT:
imap_processing/idex/idex_event_flags.py:275
- The single-peak exception checks
direct_widths, bypassing the saturation-aware width calculated just above. A saturated 30 ns Gaussian can measure about 62 ns on clipped High while measuring 30 ns on Low, so this path incorrectly accepts it as a ≥50 ns broad event. Use the resolved width for the exception while still requiring exactly one detected High peak.
if (
channel_index == 0
and len(direct_widths) == 1
and direct_widths[0] >= _SINGLE_PEAK_WIDTH_US
):
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Just as an FYI this PR is also pretty big. Since the data release 2 is approaching its fine for just this time. |
|
is this suppressed comment relevant? imap_processing/idex/idex_event_flags.py:275 The single-peak exception checks direct_widths, bypassing the saturation-aware width calculated just above. A saturated 30 ns Gaussian can measure about 62 ns on clipped High while measuring 30 ns on Low, so this path incorrectly accepts it as a ≥50 ns broad event. Use the resolved width for the exception while still requiring exactly one detected High peak. |
|
Great work Alex!! Looks like this really hardens dust hit detection. I thought you just had a small bug 😆. I will let you address the comments before I approve. |
| waveform, times, _TOF_REFERENCE_BASELINES[channel_index] | ||
| ): | ||
| continue | ||
| corrected, sigma, peaks = _reference_qualifying_peaks( |
There was a problem hiding this comment.
I think it may be worth merging _reference_qualifying_peaks and _qualifying_peaks
or at least creating some helpers and a wrapper.
There was a problem hiding this comment.
Added new helper: _find_qualifying_peak_indices
There was a problem hiding this comment.
Is there a reason this was not addressed? Is there a plan to address it or no?
There was a problem hiding this comment.
🟡 Changes recommended
The fallback can misclassify high-noise waveforms, and the requested increased peak separation is not implemented.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Balanced
5c1c941
into
IMAP-Science-Operations-Center:dev
Change Summary
Overview
File changes
idex_event_flags.py
test_idex_event_flags.py
Testing
Closes #3443