ULTRA add code for ultra90 efficiencies#2817
ULTRA add code for ultra90 efficiencies#2817lacoak21 wants to merge 1 commit intoIMAP-Science-Operations-Center:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates ULTRA processing to select sensor-specific (45 vs 90) efficiency lookup tables, so ULTRA90 no longer reuses ULTRA45 efficiencies.
Changes:
- Thread
sensor_idinto L1C spun efficiency/geometric factor computation to build a sensor name (ultra45/ultra90). - Update L1B efficiency lookup utilities to select the correct sensor-specific logistic interpolation CSV.
- Update unit tests for the new function signatures / parameters.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
imap_processing/ultra/l1c/ultra_l1c_pset_bins.py |
Adds sensor_id to efficiency/geometric factor calculation and passes sensor name through to efficiency interpolator/lookup. |
imap_processing/ultra/l1c/spacecraft_pset.py |
Passes sensor_id into the spun efficiency/geometric factor helper. |
imap_processing/ultra/l1c/helio_pset.py |
Passes sensor_id into the spun efficiency/geometric factor helper. |
imap_processing/ultra/l1b/ultra_l1b_extended.py |
Updates efficiency interpolator + get_efficiency API to require a sensor name. |
imap_processing/ultra/l1b/lookup_utils.py |
Updates get_energy_efficiencies to select ULTRA45 vs ULTRA90 logistic interpolation files. |
imap_processing/tests/ultra/unit/test_ultra_l1c_pset_bins.py |
Supplies sensor_id in the test call. |
imap_processing/tests/ultra/unit/test_ultra_l1b_extended.py |
Updates get_efficiency call to include sensor name. |
imap_processing/tests/ultra/unit/test_lookup_utils.py |
Updates get_energy_efficiencies call to include sensor name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| def get_efficiency( | ||
| energy: NDArray, | ||
| phi_inst: NDArray, | ||
| theta_inst: NDArray, | ||
| ancillary_files: dict, | ||
| sensor: str, | ||
| interpolator: RegularGridInterpolator = None, | ||
| ) -> np.ndarray: |
There was a problem hiding this comment.
get_efficiency now requires a sensor argument, but there are still call sites that invoke it with the old signature (e.g., imap_processing/ultra/l1b/de.py calls get_efficiency(..., ancillary_files) without the sensor). This will raise a TypeError at runtime. Update all callers to pass the sensor string (e.g., f"ultra{sensor}") or provide a backwards-compatible default.
| if sensor == "ultra45": | ||
| lookup_table = pd.read_csv( | ||
| ancillary_files["l1b-45sensor-logistic-interpolation"] | ||
| ) | ||
| else: | ||
| lookup_table = pd.read_csv( | ||
| ancillary_files["l1b-90sensor-logistic-interpolation"] | ||
| ) |
There was a problem hiding this comment.
This change introduces a hard dependency on ancillary_files["l1b-90sensor-logistic-interpolation"] for ULTRA90. The current test ancillary fixture/config only includes the 45-sensor file, so ULTRA90 processing will KeyError unless the 90-sensor logistic interpolation CSV is also provisioned. Either (1) ensure the ancillary bundling/downloader + test fixtures include the new 90-sensor file, or (2) implement a temporary fallback to the 45-sensor efficiencies with a clear warning if the 90-sensor file is missing (to preserve the previous behavior).
| eff, gf = get_efficiencies_and_geometric_function( | ||
| valid_spun_pixels, | ||
| boundary_sf, | ||
| mock_theta, | ||
| mock_phi, | ||
| npix=pix, | ||
| sensor_id=45, | ||
| ancillary_files=ancillary_files, |
There was a problem hiding this comment.
get_efficiencies_and_geometric_function returns (gf_averaged, eff_averaged) (and callers in spacecraft_pset.py / helio_pset.py unpack it as geometric_function, efficiencies). This test unpacks as eff, gf, which swaps the two outputs and makes the assertions semantically misleading. Swap the unpacking order (or rename variables) so the test matches the function’s return contract.
Change Summary
Overview
Very simple PR to use efficiencies for ULTRA90. Before this, the ULTRA team instructed us to use 45 efficiencies for both because they hadn't gotten around to making them for 90 yet. Nick sent me new efficiency ancillary files for both 90 and 45 yesterday. I will be uploading those as soon as the goodtimes/culling is set to get pushed to prod.
File changes
Testing
Fix tests.