Minor changes towards issue 3135 (IMAP-Lo goodtimes)#3170
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes small IMAP-Lo L1B “goodtimes”/background-rate processing updates to (1) increase MET precision for cross-checking against the IMAP-Lo team’s offline pipeline and (2) reduce brittleness when additional ion species are added in the future by replacing hard-coded dimensions and making some lookups more tolerant.
Changes:
- Replace hard-coded ESA/spin-bin dimensions in selected arrays with shared LO constants (and introduce
N_SPIN_ANGLE_BINS). - Make histogram species handling more tolerant when a species’
*_countsvariable is absent (fallback to zeros). - Increase
gt_start_met/gt_end_metprecision tofloat64and relax attribute schema checking for background-rate variables.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
imap_processing/lo/l1b/lo_l1b.py |
Updates array shapes to use constants, adds missing-species tolerance, increases MET precision, and changes how bgrates vars/attrs are collected. |
imap_processing/lo/constants.py |
Adds N_SPIN_ANGLE_BINS constant (60) to centralize spin-angle bin sizing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| background_rate_fields = sorted( | ||
| [ | ||
| data_var | ||
| for data_var in l1b_backgrounds_and_goodtimes_ds.data_vars | ||
| if any( | ||
| data_var.endswith(suffix) for suffix in background_rate_field_suffixes | ||
| ) | ||
| ] | ||
| ) |
There was a problem hiding this comment.
I didn't catch this before, but no epoch variable in the bgrates dataset? I'm pretty sure that will not be tolerated by SPDF.
| lib_bgrates_ds = l1b_backgrounds_and_goodtimes_ds[background_rate_fields] | ||
| lib_bgrates_ds.attrs = attr_mgr_l1b.get_global_attributes("imap_lo_l1b_bgrates") | ||
|
|
||
| return lib_bgrates_ds, l1b_goodtimes_ds |
There was a problem hiding this comment.
| lib_bgrates_ds = l1b_backgrounds_and_goodtimes_ds[background_rate_fields] | |
| lib_bgrates_ds.attrs = attr_mgr_l1b.get_global_attributes("imap_lo_l1b_bgrates") | |
| return lib_bgrates_ds, l1b_goodtimes_ds | |
| l1b_bgrates_ds = l1b_backgrounds_and_goodtimes_ds[background_rate_fields] | |
| l1b_bgrates_ds.attrs = attr_mgr_l1b.get_global_attributes("imap_lo_l1b_bgrates") | |
| return l1b_bgrates_ds, l1b_goodtimes_ds |
There was a problem hiding this comment.
@tmplummer - not sure what I was smoking before, but fixed now.
|
It looks like a few of the changes inside of if statements are not getting exercised in unit testing. Is there any way you can add coverage for those? |
tmplummer
left a comment
There was a problem hiding this comment.
Thanks for adding the test coverage
9c1c905
into
IMAP-Science-Operations-Center:dev
Change Summary
Overview
Minor changes in support for issue #3135
File changes
imap_processing/lo/l1b/lo_l1b.py- increased precision for MET data - was causing issues when cross-checking against IMAP-Lo team's offline pipeline algorithm. Also made the code tolerant if we end up adding new species (besides 'O') in the future. Using existing constants when we can (added one new one -N_SPIN_ANGLE_BINS.Testing