From b85e21d93ed662e115f0a66b1d7e56a8a1207c22 Mon Sep 17 00:00:00 2001 From: Tim Plummer Date: Wed, 28 Oct 2026 14:57:18 -0600 Subject: [PATCH 1/3] Fix swapped hae_latitude/hae_longitude in Hi L1B DE instrument_pointing returns (longitude, latitude), but compute_hae_coordinates assigned column 0 to latitude. Also wrap longitude into [0, 360) to match the CDF attributes and L1C PSET convention. Closes #3603 Co-Authored-By: Claude Opus 5.5 --- imap_processing/hi/hi_l1b.py | 6 ++++-- imap_processing/tests/hi/test_hi_l1b.py | 21 +++++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/imap_processing/hi/hi_l1b.py b/imap_processing/hi/hi_l1b.py index 9e1dd1c81d..21078d6e03 100644 --- a/imap_processing/hi/hi_l1b.py +++ b/imap_processing/hi/hi_l1b.py @@ -409,8 +409,10 @@ def compute_hae_coordinates(dataset: xr.Dataset) -> dict[str, xr.DataArray]: pointing_coordinates = instrument_pointing( et, SpiceFrame[f"IMAP_HI_{sensor_number}"], SpiceFrame.ECLIPJ2000 ) - new_vars["hae_latitude"].values = pointing_coordinates[:, 0] - new_vars["hae_longitude"].values = pointing_coordinates[:, 1] + # instrument_pointing returns (longitude, latitude) with longitude in + # (-180, 180]. Wrap longitude into [0, 360) to match the CDF attributes. + new_vars["hae_longitude"].values = np.mod(pointing_coordinates[:, 0], 360) + new_vars["hae_latitude"].values = pointing_coordinates[:, 1] return new_vars diff --git a/imap_processing/tests/hi/test_hi_l1b.py b/imap_processing/tests/hi/test_hi_l1b.py index 3800ec41d4..46f8090e0c 100644 --- a/imap_processing/tests/hi/test_hi_l1b.py +++ b/imap_processing/tests/hi/test_hi_l1b.py @@ -329,11 +329,19 @@ def test_de_nominal_bin_and_spin_phase( np.testing.assert_array_equal(new_vars["nominal_bin"].values, expected_nominal_bin) -@pytest.mark.parametrize("sensor_number", [45, 90]) +@pytest.mark.parametrize( + "sensor_number, pointing_lon, expected_lon, expected_lat", + [(45, -100.0, 260.0, -30.0), (90, 120.0, 120.0, 15.0)], +) @mock.patch("imap_processing.hi.hi_l1b.any_good_direct_events", return_value=True) @mock.patch("imap_processing.hi.hi_l1b.instrument_pointing") def test_compute_hae_coordinates( - mock_instrument_pointing, mock_any_good_de, sensor_number + mock_instrument_pointing, + mock_any_good_de, + sensor_number, + pointing_lon, + expected_lon, + expected_lat, ): """Test coverage for compute_hae_coordinates function.""" @@ -344,9 +352,10 @@ def side_effect_func(et, inst_frame: SpiceFrame, to_frame): The function signature matches the signature of `instrument_pointing`. The output is the same type and shape as what `instrument_pointing` returns - but is filled with values that match the instrument sensor number. + (longitude, latitude) with distinct values so a lat/lon swap is detected. """ - return np.full((et.size, 2), 45 if "45" in inst_frame.name else 90) + assert str(sensor_number) in inst_frame.name + return np.tile([pointing_lon, expected_lat], (et.size, 1)) mock_instrument_pointing.side_effect = side_effect_func @@ -363,10 +372,10 @@ def side_effect_func(et, inst_frame: SpiceFrame, to_frame): new_vars = compute_hae_coordinates(fake_dataset) assert "hae_latitude" in new_vars assert new_vars["hae_latitude"].shape == fake_dataset.event_met.shape - np.testing.assert_allclose(new_vars["hae_latitude"].values, sensor_number) + np.testing.assert_allclose(new_vars["hae_latitude"].values, expected_lat) assert "hae_longitude" in new_vars assert new_vars["hae_longitude"].shape == fake_dataset.event_met.shape - np.testing.assert_allclose(new_vars["hae_longitude"].values, sensor_number) + np.testing.assert_allclose(new_vars["hae_longitude"].values, expected_lon) @mock.patch("imap_processing.hi.hi_l1b.any_good_direct_events", return_value=True) From 472c01c0b736f3f19fa0a9dcd607d384b4bf4786 Mon Sep 17 00:00:00 2001 From: Tim Plummer Date: Wed, 28 Oct 2026 15:34:23 -0600 Subject: [PATCH 2/3] Return longitude in [0, 360) from instrument_pointing Replace the per-vector spiceypy.reclat loop with cartesian_to_spherical so (lon, lat) output uses the same [0, 360) longitude convention as IDEX and the Hi L1C PSET. Drop the now-redundant wrap in Hi L1B and add a mocked frame_transform test with known cartesian vectors. Co-Authored-By: Claude Opus 5.5 --- imap_processing/hi/hi_l1b.py | 5 +-- imap_processing/spice/geometry.py | 15 ++++--- imap_processing/tests/hi/test_hi_l1b.py | 7 ++-- imap_processing/tests/spice/test_geometry.py | 43 ++++++++++++++++++++ 4 files changed, 55 insertions(+), 15 deletions(-) diff --git a/imap_processing/hi/hi_l1b.py b/imap_processing/hi/hi_l1b.py index 21078d6e03..3ff7a36477 100644 --- a/imap_processing/hi/hi_l1b.py +++ b/imap_processing/hi/hi_l1b.py @@ -409,9 +409,8 @@ def compute_hae_coordinates(dataset: xr.Dataset) -> dict[str, xr.DataArray]: pointing_coordinates = instrument_pointing( et, SpiceFrame[f"IMAP_HI_{sensor_number}"], SpiceFrame.ECLIPJ2000 ) - # instrument_pointing returns (longitude, latitude) with longitude in - # (-180, 180]. Wrap longitude into [0, 360) to match the CDF attributes. - new_vars["hae_longitude"].values = np.mod(pointing_coordinates[:, 0], 360) + # instrument_pointing returns (longitude, latitude) + new_vars["hae_longitude"].values = pointing_coordinates[:, 0] new_vars["hae_latitude"].values = pointing_coordinates[:, 1] return new_vars diff --git a/imap_processing/spice/geometry.py b/imap_processing/spice/geometry.py index b465e08f81..53dec1d472 100644 --- a/imap_processing/spice/geometry.py +++ b/imap_processing/spice/geometry.py @@ -10,7 +10,6 @@ """ import logging -import typing from enum import IntEnum import numpy as np @@ -457,9 +456,10 @@ def instrument_pointing( """ Compute the instrument pointing at the specified times. - By default, the coordinates returned are (Longitude, Latitude) coordinates in - the reference frame `to_frame`. Cartesian coordinates can be returned if - desired by setting `cartesian=True`. + By default, the coordinates returned are (Longitude, Latitude) coordinates, in + degrees, in the reference frame `to_frame`. Longitude is in the range [0, 360) + and latitude is in the range [-90, 90]. Cartesian coordinates can be returned + if desired by setting `cartesian=True`. Parameters ---------- @@ -476,14 +476,13 @@ def instrument_pointing( Returns ------- pointing : np.ndarray - The instrument pointing at the specified times. + The instrument pointing at the specified times. Shape is (..., 2) for + (Longitude, Latitude) or (..., 3) if `cartesian=True`. """ pointing = frame_transform(et, BORESIGHT_LOOKUP[instrument], instrument, to_frame) if cartesian: return pointing - if isinstance(et, typing.Collection): - return np.rad2deg([spiceypy.reclat(vec)[1:] for vec in pointing]) - return np.rad2deg(spiceypy.reclat(pointing)[1:]) + return cartesian_to_spherical(pointing, degrees=True)[..., 1:3] def basis_vectors( diff --git a/imap_processing/tests/hi/test_hi_l1b.py b/imap_processing/tests/hi/test_hi_l1b.py index 46f8090e0c..5dfdf07c1b 100644 --- a/imap_processing/tests/hi/test_hi_l1b.py +++ b/imap_processing/tests/hi/test_hi_l1b.py @@ -330,8 +330,8 @@ def test_de_nominal_bin_and_spin_phase( @pytest.mark.parametrize( - "sensor_number, pointing_lon, expected_lon, expected_lat", - [(45, -100.0, 260.0, -30.0), (90, 120.0, 120.0, 15.0)], + "sensor_number, expected_lon, expected_lat", + [(45, 260.0, -30.0), (90, 120.0, 15.0)], ) @mock.patch("imap_processing.hi.hi_l1b.any_good_direct_events", return_value=True) @mock.patch("imap_processing.hi.hi_l1b.instrument_pointing") @@ -339,7 +339,6 @@ def test_compute_hae_coordinates( mock_instrument_pointing, mock_any_good_de, sensor_number, - pointing_lon, expected_lon, expected_lat, ): @@ -355,7 +354,7 @@ def side_effect_func(et, inst_frame: SpiceFrame, to_frame): (longitude, latitude) with distinct values so a lat/lon swap is detected. """ assert str(sensor_number) in inst_frame.name - return np.tile([pointing_lon, expected_lat], (et.size, 1)) + return np.tile([expected_lon, expected_lat], (et.size, 1)) mock_instrument_pointing.side_effect = side_effect_func diff --git a/imap_processing/tests/spice/test_geometry.py b/imap_processing/tests/spice/test_geometry.py index 2b71288d98..768c3bd3c7 100644 --- a/imap_processing/tests/spice/test_geometry.py +++ b/imap_processing/tests/spice/test_geometry.py @@ -484,6 +484,49 @@ def test_instrument_pointing(furnish_kernels): assert ins_pointing.shape == (3, 3) +@pytest.mark.parametrize( + "vectors, expected_lon_lat", + [ + # Single vector input: -y axis has a longitude of 270, not -90 + (np.array([0.0, -1.0, 0.0]), np.array([270.0, 0.0])), + # Multiple vector input covering all four quadrants and the poles + ( + np.array( + [ + [1.0, 0.0, 0.0], + [1.0, 1.0, 0.0], + [-1.0, 1.0, 0.0], + [-1.0, -1.0, 0.0], + [1.0, -1.0, np.sqrt(2)], + [0.0, 0.0, 1.0], + [0.0, 0.0, -1.0], + ] + ), + np.array( + [ + [0.0, 0.0], + [45.0, 0.0], + [135.0, 0.0], + [225.0, 0.0], + [315.0, 45.0], + [0.0, 90.0], + [0.0, -90.0], + ] + ), + ), + ], +) +@mock.patch("imap_processing.spice.geometry.frame_transform") +def test_instrument_pointing_lon_lat(mock_frame_transform, vectors, expected_lon_lat): + """Test instrument_pointing (lon, lat) output with known cartesian vectors.""" + mock_frame_transform.return_value = vectors + et = 0.0 if vectors.ndim == 1 else np.zeros(len(vectors)) + lon_lat = instrument_pointing(et, SpiceFrame.IMAP_HI_90, SpiceFrame.ECLIPJ2000) + assert lon_lat.shape == expected_lon_lat.shape + np.testing.assert_allclose(lon_lat, expected_lon_lat, atol=1e-12) + assert np.all((lon_lat[..., 0] >= 0) & (lon_lat[..., 0] < 360)) + + @pytest.mark.parametrize( "frame", [ From a31c617e10841d9947a194874e142e26665d6091 Mon Sep 17 00:00:00 2001 From: Tim Plummer Date: Wed, 4 Nov 2026 09:28:42 -0700 Subject: [PATCH 3/3] Apply suggestion from @tmplummer --- imap_processing/spice/geometry.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/imap_processing/spice/geometry.py b/imap_processing/spice/geometry.py index 53dec1d472..82d2ca2b3c 100644 --- a/imap_processing/spice/geometry.py +++ b/imap_processing/spice/geometry.py @@ -456,9 +456,9 @@ def instrument_pointing( """ Compute the instrument pointing at the specified times. - By default, the coordinates returned are (Longitude, Latitude) coordinates, in - degrees, in the reference frame `to_frame`. Longitude is in the range [0, 360) - and latitude is in the range [-90, 90]. Cartesian coordinates can be returned + By default, the coordinates returned are (Longitude, Latitude), in degrees, in + the reference frame `to_frame`. Longitude is in the range [0, 360) and + latitude is in the range [-90, 90]. Cartesian coordinates can be returned if desired by setting `cartesian=True`. Parameters