ULTRA l1c fix exposure time calculation#2820
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes ULTRA L1C exposure-time scaling so flagged spins are excluded, aligning exposure factors with the quality-filtered spin selection produced in L1B “goodtimes”.
Changes:
- Update L1C exposure-time normalization to use
goodtimes_datasetspin periods and energy-dependent spin-quality flags. - Vectorize energy/spin-dependent event rejection masking in ULTRA L1B culling.
- Extend unit tests/fixtures to include energy-dependent spin-quality flags and validate the updated exposure-time behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
imap_processing/ultra/l1c/ultra_l1c_pset_bins.py |
Switch exposure-time scaling from universal spin table to goodtimes_dataset (quality-filtered) spins, with energy-dependent handling. |
imap_processing/ultra/l1c/spacecraft_pset.py |
Update callsite to pass energy bins and goodtimes_dataset into exposure-time calculation. |
imap_processing/ultra/l1c/helio_pset.py |
Same exposure-time callsite update for heliospheric pointing set generation. |
imap_processing/ultra/l1b/ultra_l1b_culling.py |
Replace per-energy-bin loop with vectorized event rejection mask based on spin/energy flags. |
imap_processing/tests/ultra/unit/test_ultra_l1c_pset_bins.py |
Add assertions around logging/behavior of good-spin counts per energy bin. |
imap_processing/tests/ultra/unit/test_ultra_l1b.py |
Update extendedspin mock data to include new energy-dependent flag fields. |
imap_processing/tests/ultra/unit/conftest.py |
Improve mock goodtimes dataset to include energy ranges/flags and spin-period info. |
💡 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.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 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.
| n_spins_in_pointing = ( | ||
| np.sum(spin_periods_2d, axis=1, where=good_spins_per_ebin) | ||
| / nominal_spin_seconds |
There was a problem hiding this comment.
n_spins_in_pointing can become a large negative (or NaN) if goodtimes_dataset contains the fill-value spin period used when no good spins exist (spin_period == -1e31 in calculate_goodtimes). Because np.sum(..., where=good_spins_per_ebin) will still include those fill periods for energy bins mapped to the padded "all True" rows, exposure can be scaled by an invalid spin count.
Consider masking out non-positive / non-finite spin_periods (or explicitly detecting the fill-value/no-good-spins case) when computing n_spins_in_pointing so exposure returns 0 (or another defined fill) when there are no valid spins.
subagonsouth
left a comment
There was a problem hiding this comment.
Just the one question about the comment vs. implementation.
Change Summary
Overview
When I produced the first l2 maps that included the new culling update, the ULTRA team noticed that the exposure looked too high and I realized that I had not been excluding spins that were flagged from the exposure time. I reproduced maps using this branch and they confirmed that this was the necessary fix.
File changes
Testing
Add a test to check that the correct spins were used in calculating the exposure factor.