-
Notifications
You must be signed in to change notification settings - Fork 22
2598 bug lo l2 bootstrap correction #2605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
2598 bug lo l2 bootstrap correction #2605
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issues with sputtering and bootstrap corrections in the Lo L2 processing pipeline and updates hydrogen geometric factors with new data from Fatemeh.
Changes:
- Fixed sputtering correction to only apply where valid oxygen intensity values exist, using conditional logic with
xr.where - Fixed bootstrap correction by moving the
valid_bootstrapmask definition before its usage and applying corrections conditionally - Updated numpy array assignment to use
.valuesfor proper indexing - Updated hydrogen geometric factor data with new energy values and uncertainties
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| imap_processing/lo/l2/lo_l2.py | Fixed sputtering and bootstrap corrections to handle invalid/NaN values properly using conditional masking and xr.where |
| imap_processing/tests/lo/test_lo_l2.py | Updated expected hydrogen energy values to match new geometric factor data |
| imap_processing/lo/ancillary_data/imap_lo_hydrogen-geometric-factor_v001.csv | Updated hydrogen geometric factors with new center energies, uncertainties, and geometric factor values |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
imap_processing/lo/l2/lo_l2.py
Outdated
| small_dataset["ena_intensity_stat_uncert"] ** 2 | ||
| + (sputter_correction_factor**2) * j_o_prime_var | ||
| + (sputter_correction_factor**2) * j_o_prime_var, | ||
| small_dataset["ena_intensity_stat_uncert"], |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the false branch of the xr.where for sputter_corrected_intensity_var, the code returns the statistical uncertainty instead of its square (variance). Since sputter_corrected_intensity_var should contain variances (as indicated by the name and the fact that it's square-rooted on line 1093), this should be "small_dataset['ena_intensity_stat_uncert'] ** 2" to maintain consistency.
| small_dataset["ena_intensity_stat_uncert"], | |
| small_dataset["ena_intensity_stat_uncert"] ** 2, |
| sputter_corrected_intensity_sys_err = xr.where( | ||
| j_o_prime_valid, | ||
| sputter_corrected_intensity | ||
| / small_dataset["ena_intensity"] |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In equation 13 (lines 1081-1087), when j_o_prime_valid is True, the code divides by small_dataset["ena_intensity"] which could be zero or near-zero, potentially causing division by zero errors. Consider adding a check to ensure small_dataset["ena_intensity"] is non-zero before performing the division, or use a small epsilon value to avoid division by zero.
| sputter_corrected_intensity_sys_err = xr.where( | |
| j_o_prime_valid, | |
| sputter_corrected_intensity | |
| / small_dataset["ena_intensity"] | |
| # Protect against division by zero in small_dataset["ena_intensity"] | |
| ena_intensity = small_dataset["ena_intensity"] | |
| safe_ena_intensity = xr.where(ena_intensity == 0, np.nan, ena_intensity) | |
| sputter_corrected_intensity_sys_err = xr.where( | |
| j_o_prime_valid, | |
| sputter_corrected_intensity | |
| / safe_ena_intensity |
greglucas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work tracking all of this down!
Minor comment that I discussed in-person about whether we should switch to working on numpy arrays right at the beginning of these corrections rather than calling all these xr.where() with multiple cases it would be something like ena_intensity_np = data["ena_intensity"].values; ena_intensity_np[valid_mask] = ...
Change Summary
Overview
Fixes a few issues with the sputtering and bootstrap corrections.
Also updates the hydrogen geometric factors to a file from Fatemeh.
Fixes to sputtering:
Fixes to boostrap:
Closes: #2598