ULTRA l1b: Validation result updates#3136
Open
lacoak21 wants to merge 8 commits intoIMAP-Science-Operations-Center:devfrom
Open
ULTRA l1b: Validation result updates#3136lacoak21 wants to merge 8 commits intoIMAP-Science-Operations-Center:devfrom
lacoak21 wants to merge 8 commits intoIMAP-Science-Operations-Center:devfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns ULTRA L1B culling/validation behavior to eliminate subtle differences in how quality-flag masks are propagated through the extendedspin culling sequence (per #3132).
Changes:
- Adjusts how the cull “mask” is constructed/updated in
calculate_extendedspin()before running upstream-ion/spectral/statistical-outlier culls. - Updates the ULTRA45 Earth keepout angle threshold constant used in valid-event filtering.
- Updates the unit test expectations for
flag_statistical_outliers().
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
imap_processing/ultra/l1b/ultra_l1b_culling.py |
Minor whitespace-only change near valid-events helper logic. |
imap_processing/ultra/l1b/extendedspin.py |
Updates mask propagation/order for subsequent culls (upstream ion, spectral, stat outliers). |
imap_processing/ultra/constants.py |
Changes ULTRA45 Earth keepout angle threshold from 20° to 15°. |
imap_processing/tests/ultra/unit/test_ultra_l1b_culling.py |
Updates assertions around std_diff returned by flag_statistical_outliers(). |
Comments suppressed due to low confidence (1)
imap_processing/ultra/l1b/extendedspin.py:125
upstream_ion_qf_2is computed using the samemaskas the first upstream-ion pass. Sinceflag_upstream_ionexpectsmaskto represent bins flagged in previous steps, the second pass likely should also exclude bins flagged by the first pass (and/or high-energy). Otherwise the second channel set may be biased by bins already identified as upstream-ion contaminated.
upstream_ion_qf_2 = flag_upstream_ion(
de_dataset,
spin_tbin_edges,
energy_ranges,
mask,
UltraConstants.UPSTREAM_ION_ENERGY_CHANNELS_2,
instrument_id,
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change Summary
closes #3132
Overview
There were subtle differences in Bob's vs my code in how our masks were getting propagated and passed through. This PR aligns our code exactly.
File changes
Testing
Update stats test.