3309 hi l2 add l2 map variables - #3365
Conversation
…na_intensity_background_systematic_err, ena_intensity_calibration_systematic_err, bg_rate, bg_rate_sys_err
There was a problem hiding this comment.
Pull request overview
This PR updates IMAP-Hi L2 ENA map production to retain additional diagnostic and uncertainty variables through to final CDF output, including a rename of the counts variable and new split systematic-error components, and extends map-combination logic to include these variables.
Changes:
- Rename map-level event counts from
countstoena_countand propagate the rename through Hi processing and ENA map utilities/tests. - Preserve and propagate
bg_rate/bg_rate_sys_err, and addena_intensity_background_systematic_errandena_intensity_calibration_systematic_err(withena_intensity_sys_erras their quadrature sum). - Add/update CDF attribute config entries and expand tests for the new/renamed variables.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| imap_processing/hi/hi_l2.py | Renames counts→ena_count, adds split systematic errors, combines new vars across calibration products, and extends ram/anti combine logic. |
| imap_processing/ena_maps/ena_maps.py | Maps PSET counts to ena_count in HiPointingSet renames. |
| imap_processing/ena_maps/utils/naming.py | Adds CATDESC mappings for the new systematic-error variables. |
| imap_processing/cdf/config/imap_enamaps_l2-common_variable_attrs.yaml | Adds common CDF attribute entries for the new systematic-error variables. |
| imap_processing/cdf/config/imap_enamaps_l2-rectangular_variable_attrs.yaml | Adds rectangular-map dependency pointers for the new systematic-error variables. |
| imap_processing/tests/hi/test_hi_l2.py | Updates/extends Hi L2 tests for renamed counts and new systematic-error variables; adds coverage for flux-ratio scaling. |
| imap_processing/tests/ena_maps/test_ena_maps.py | Updates ENA maps tests for the ena_count rename and expected variables. |
Suppressed comments (1)
imap_processing/tests/hi/test_hi_l2.py:1484
- combine_maps() now combines bg_rate, bg_rate_sys_err, and the two split systematic-error components, but this test only asserts ena_count/exposure_factor. Add explicit assertions for the exposure-weighted results of the newly combined variables (with differing ram/anti values from the fixture) to prevent regressions.
expected_counts = 100 + 120 # 100 + (100 + 20)
np.testing.assert_array_almost_equal(
result.data_1d["ena_count"].values,
np.ones_like(result.data_1d["ena_count"].values) * expected_counts,
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (4)
imap_processing/hi/hi_l2.py:703
combine_maps()currently exposure-averagesena_intensity_sys_errindependently of the two component terms (bg_intensity_sys_errandena_intensity_calibration_sys_err). That breaks the invariant established earlier in the pipeline (ena_intensity_sys_err = sqrt(bg^2 + calib^2)), so the combined map can become internally inconsistent. Recomputeena_intensity_sys_errfrom the combined component terms instead of averaging it separately.
for var in (
"ena_intensity_sys_err",
"bg_rate",
"bg_rate_sys_err",
"bg_intensity_sys_err",
imap_processing/cdf/config/imap_enamaps_l2-rectangular_variable_attrs.yaml:124
- The new support variable
ena_intensity_calibration_sys_erris defined in the common + rectangular ENA-map attribute YAMLs, but there is no corresponding override entry inimap_enamaps_l2-healpix_variable_attrs.yaml. Healpix ENA map generation drops variables without tiling-specific attribute overrides (or will produce incorrect DEPEND pointers), so this variable may be silently dropped from Healpix map CDFs.
ena_intensity_calibration_sys_err:
DEPEND_1: energy
DEPEND_2: longitude
DEPEND_3: latitude
LABL_PTR_1: energy_label
LABL_PTR_2: longitude_label
LABL_PTR_3: latitude_label
imap_processing/hi/hi_l2.py:464
- The PR description / linked issue describes adding
ena_intensity_background_systematic_err, but the implementation and tests expose this asbg_intensity_sys_err. If downstream consumers (or requirements) expect theena_intensity_*naming, either rename the output variable (and update the CDF attribute configs accordingly) or update the PR description/issue expectations to match the chosenbg_intensity_sys_errname.
map_ds : xarray.Dataset
Map dataset with new variables: ena_intensity, ena_intensity_stat_uncert,
bg_intensity_sys_err.
imap_processing/hi/hi_l2.py:516
flux_correction_ratiois computed as corrected/uncorrected intensity. Ifpre_correction_intensityis 0 (e.g., a pixel clipped to zero after background subtraction) this produces 0/0 -> NaN, which then turnsbg_intensity_sys_errinto NaN even though the flux correction itself didn’t introduce missing data. Consider normalizing non-finite ratios to 1.0 to avoid poisoning the systematic-error field.
with np.errstate(divide="ignore", invalid="ignore"):
flux_correction_ratio = map_ds["ena_intensity"] / pre_correction_intensity
map_ds["bg_intensity_sys_err"] = (
map_ds["bg_intensity_sys_err"] * flux_correction_ratio
)
Adds several new variables to IMAP-Hi L2 map products that were previously computed internally but dropped (or, in one case, silently lost) before final CDF output:
Closes: #3309