Skip to content

2800 hi l1c backgrounds#2937

Open
subagonsouth wants to merge 5 commits intoIMAP-Science-Operations-Center:devfrom
subagonsouth:2800-hi-l1c---backgrounds
Open

2800 hi l1c backgrounds#2937
subagonsouth wants to merge 5 commits intoIMAP-Science-Operations-Center:devfrom
subagonsouth:2800-hi-l1c---backgrounds

Conversation

@subagonsouth
Copy link
Copy Markdown
Contributor

@subagonsouth subagonsouth commented Apr 8, 2026

Summary

Implements background rate calculation for HI L1C PSET datasets. Adds _compute_background_counts() and updates pset_backgrounds() to compute background counts from direct events and convert them to isotropic background rates with uncertainties.

Key Changes

imap_processing/hi/hi_l1c.py

  • Added _compute_background_counts() function: Computes background counts by filtering direct events according to background configuration and binning into 3D array (epoch, calibration_prod, background_index)
  • Updated pset_backgrounds() function: Changed from accepting pre-computed background counts to computing them internally from L1B direct events, then averaging across spin bins to produce isotropic background rates
  • Updated generate_pset_dataset(): Loads background configuration CSV and passes exposure times to pset_backgrounds()

imap_processing/tests/hi/test_hi_l1c.py

  • Updated test_pset_counts() and test_pset_counts_empty_l1b(): Removed background_df parameter and background_counts assertions
  • Rewrote test_pset_backgrounds(): Updated to match new signature with L1B dataset and goodtimes, added required SPICE fixtures
  • Fixed test_generate_pset_dataset_uses_midpoint_time(): Updated mock to return exposure_times DataArray

Dependencies

Depends on #2935 (background utilities)

Related Issues

Closes: #2800

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Implements IMAP-Hi L1C PSET background-rate computation by deriving background counts directly from L1B direct events, applying background configuration scaling/uncertainties, and wiring the new ancillary input through the CLI and tests.

Changes:

  • Added background configuration DataFrame accessor/utilities and an event-filtering iterator for background selections.
  • Updated HI L1C PSET generation to load background config and compute isotropic background rates/uncertainties from L1B DE + exposure times.
  • Updated CLI ancillary handling and adjusted/rewrote affected tests to match new function signatures.

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/hi/hi_l1c.py Loads background config and computes background counts/rates/uncertainties during PSET generation.
imap_processing/hi/utils.py Adds BackgroundConfig accessor + shared filtering utilities and a background iterator.
imap_processing/cli.py Updates HI L1C PSET processing to require both cal-prod and backgrounds ancillary inputs.
imap_processing/tests/hi/test_hi_l1c.py Updates tests for new hi_l1c/generate_pset_dataset/pset_backgrounds signatures.
imap_processing/tests/hi/test_utils.py Adds coverage for the new background_config DataFrame accessor.
imap_processing/tests/hi/conftest.py Adds fixture path for the background configuration CSV.
imap_processing/tests/test_cli.py Updates CLI test inputs to provide both required HI L1C ancillary files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +599 to 614
# Call pset_backgrounds with the new signature
backgrounds_vars = hi_l1c.pset_backgrounds(
empty_pset.coords,
background_df,
hi_l1b_de_dataset,
hi_goodtimes_dataset,
exposure_times,
)

assert "background_rates" in backgrounds_vars
np.testing.assert_array_equal(
backgrounds_vars["background_rates"].data,
np.zeros((n_epoch, n_energy, n_cal_prod, n_spin_bins)),
assert backgrounds_vars["background_rates"].data.shape == (
len(empty_pset.coords["epoch"]),
len(empty_pset.coords["esa_energy_step"]),
len(empty_pset.coords["calibration_prod"]),
len(empty_pset.coords["spin_angle_bin"]),
)
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

test_pset_backgrounds now only asserts output variable presence and shapes. Since pset_backgrounds() was rewritten to compute counts, apply scaling factors, and propagate uncertainties, this test should include at least one numerical correctness/sanity assertion (e.g., rates constant across spin_angle_bin for isotropy, non-negative rates/uncertainties, and expected behavior when exposure is uniform/zero).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 8 out of 8 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

len(bg_coords["calibration_prod"]),
len(bg_coords["background_index"]),
),
dtype=np.int32,
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

_compute_background_counts currently allocates background_counts with dtype=np.int32. For long pointings / high event rates, background counts can plausibly exceed the 32-bit range and silently overflow. Prefer using an int64 accumulator (e.g., np.int64) for counts to make the rate/uncertainty computation safe.

Suggested change
dtype=np.int32,
dtype=np.int64,

Copilot uses AI. Check for mistakes.
@subagonsouth subagonsouth self-assigned this Apr 9, 2026
@subagonsouth subagonsouth added this to IMAP Apr 9, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 4 out of 4 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +894 to 910
# Create mapping from descriptor to path
anc_path_dict = {
dep.descriptor.split("-", 1)[1]: dep.imap_file_paths[
0
].construct_path()
for dep in anc_dependencies
}

# Verify we have both required ancillary files
if (
"cal-prod" not in anc_path_dict
or "backgrounds" not in anc_path_dict
):
raise ValueError(
f"Expected exactly one ancillary dependency. Got {anc_paths}"
f"Missing required ancillary files. Expected 'cal-prod' and "
f"'backgrounds', got {list(anc_path_dict.keys())}"
)
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

anc_path_dict derives keys via dep.descriptor.split("-", 1)[1], which will misbehave or raise IndexError if the descriptor format differs (e.g., missing sensor prefix or additional hyphens). To avoid fragile parsing, prefer a safer normalization (e.g., strip leading "45sensor-"/"90sensor-" when present, otherwise keep descriptor as-is) or map using the file’s parsed name fields, and raise a clear ValueError when required descriptors can’t be determined.

Copilot uses AI. Check for mistakes.
Comment on lines +635 to +663
# Convert background config DataFrame to xarray Dataset
config_ds = background_config_df.to_xarray()
scaling_factors_da = config_ds["scaling_factor"]
uncertainties_da = config_ds["uncertainty"]

# Compute scaled rates
scaled_rates = count_rates * scaling_factors_da

# Compute uncertainties (Poisson + scaling factor, combined in quadrature)
poisson_unc = (
np.sqrt(background_counts) / total_exposure_time
) * scaling_factors_da
scaling_unc = count_rates * uncertainties_da
combined_unc = np.sqrt(poisson_unc**2 + scaling_unc**2)

# Sum over background_index dimension to get final rates
total_rates = scaled_rates.sum(dim="background_index", skipna=True)
total_unc = np.sqrt((combined_unc**2).sum(dim="background_index", skipna=True))

# Broadcast to (epoch, esa_energy_step, calibration_prod, spin_angle_bin)
# Backgrounds are isotropic and independent of ESA step, so we
# broadcast across esa_energy_step and spin_angle_bin dimensions.
output_vars["background_rates"].values[:] = total_rates.values[
:, np.newaxis, :, np.newaxis
]
output_vars["background_rates_uncertainty"].values[:] = total_unc.values[
:, np.newaxis, :, np.newaxis
]

Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

background_config_df.to_xarray() can include calibration_prod values that are not present in pset_coords["calibration_prod"]. Because xarray aligns arithmetic on coordinate labels, count_rates * scaling_factors_da will expand to the union of calibration_prod coordinates; if the background config has extras, total_rates.values will no longer match the output array shape and assignment will fail. Consider subsetting/reindexing scaling_factors_da/uncertainties_da to exactly the PSET calibration_prod coordinates (or explicitly validating that the sets match) before doing the multiplication/sum.

Copilot uses AI. Check for mistakes.
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 L1C - backgrounds

2 participants