Skip to content

3309 hi l2 add l2 map variables - #3365

Open
tmplummer wants to merge 4 commits into
IMAP-Science-Operations-Center:devfrom
tmplummer:3309-hi-l2---add-l2-map-variables
Open

3309 hi l2 add l2 map variables#3365
tmplummer wants to merge 4 commits into
IMAP-Science-Operations-Center:devfrom
tmplummer:3309-hi-l2---add-l2-map-variables

Conversation

@tmplummer

Copy link
Copy Markdown
Contributor

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:

  • ena_count — renamed from counts at the L1C→L2 rename step, fixing a bug where it had no CDF attribute definition and was being silently dropped from the final map.
  • bg_rate / bg_rate_sys_err — no longer discarded as "intermediate"; now combined across calibration_prod (inverse-variance weighted) and kept in the final product.
  • ena_intensity_background_systematic_err and ena_intensity_calibration_systematic_err — split out from what was previously an internal, discarded computation. ena_intensity_sys_err (unchanged, still present) is now their quadrature combination.
  • The background systematic term is rescaled by the same flux-correction ratio applied to ena_intensity, keeping it consistent with the corrected intensity (previously it wasn't corrected at all).
  • combine_maps() (ram/anti heliocentric-frame combination) extended to properly combine all the new/renamed variables instead of silently dropping the anti-map's contribution.
  • New CDF attribute entries added to the shared imap_enamaps_l2-common/imap_enamaps_l2-rectangular YAML configs for the two new systematic-error variables.
  • Test coverage updated/added in test_hi_l2.py and test_ena_maps.py accordingly.

Closes: #3309

…na_intensity_background_systematic_err, ena_intensity_calibration_systematic_err, bg_rate, bg_rate_sys_err

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 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 counts to ena_count and propagate the rename through Hi processing and ENA map utilities/tests.
  • Preserve and propagate bg_rate / bg_rate_sys_err, and add ena_intensity_background_systematic_err and ena_intensity_calibration_systematic_err (with ena_intensity_sys_err as 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 countsena_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.

Comment thread imap_processing/hi/hi_l2.py
Comment thread imap_processing/hi/hi_l2.py
Comment thread imap_processing/hi/hi_l2.py
Comment thread imap_processing/tests/hi/test_hi_l2.py
tmplummer and others added 2 commits August 4, 2026 11:41
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-averages ena_intensity_sys_err independently of the two component terms (bg_intensity_sys_err and ena_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. Recompute ena_intensity_sys_err from 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_err is defined in the common + rectangular ENA-map attribute YAMLs, but there is no corresponding override entry in imap_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 as bg_intensity_sys_err. If downstream consumers (or requirements) expect the ena_intensity_* naming, either rename the output variable (and update the CDF attribute configs accordingly) or update the PR description/issue expectations to match the chosen bg_intensity_sys_err name.
    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_ratio is computed as corrected/uncorrected intensity. If pre_correction_intensity is 0 (e.g., a pixel clipped to zero after background subtraction) this produces 0/0 -> NaN, which then turns bg_intensity_sys_err into 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
        )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Hi L2 - Add L2 Map variables

2 participants