GLOWS L2 - Add ecliptic coordinates for histogram bin centers by vmartinez-cu · Pull Request #2871 · IMAP-Science-Operations-Center/imap_processing

vmartinez-cu · 2026-03-25T23:35:11Z

This PR adds a new method to compute the ecliptic longitude and latitude for each histogram bin center in the GLOWS L2 product using SPICE transformations. It also adds a unit test and mocking of these computations in existing tests where appropriate to remain independent of SPICE/time conversions.

Code Updates

Added the compute_ecliptic_coords_of_bin_centers static method to DailyLightcurve, which transforms instrument pointing directions from the IMAP spacecraft frame to the ECLIPJ2000 frame using SPICE, providing ecliptic longitude and latitude for each histogram bin center.
Updated the __post_init__ method of DailyLightcurve to compute and assign ecliptic_lon and ecliptic_lat using the new method, replacing the previous placeholder arrays.

Testing

Added a mock_ecliptic_bin_centers test fixture to mock ecliptic coordinate computations, ensuring unit tests for DailyLightcurve are independent of SPICE/time conversions.
Updated all affected tests in test_glows_l2_data.py and `test_glows_l2.py to use the new fixture, ensuring their reliability and isolation from
Added a unit test to test the output of the new method

Closes #2722

Copilot

Pull request overview

This PR adds computation of ecliptic longitude/latitude for GLOWS L2 histogram bin centers (via SPICE frame transforms) and updates the test suite to mock this computation in most unit tests while adding a kernel-backed unit test for the new method.

Changes:

Add DailyLightcurve.compute_ecliptic_coords_of_bin_centers() to compute bin-center ecliptic coordinates using SPICE transforms.
Update DailyLightcurve.__post_init__ to populate ecliptic_lon/ecliptic_lat instead of placeholder zeros.
Add a mocking fixture for ecliptic bin-center computation and update affected tests; add an external-kernel unit test for the new method.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
imap_processing/glows/l2/glows_l2_data.py	Compute and store ecliptic lon/lat for L2 daily lightcurve bin centers using SPICE transforms.
imap_processing/tests/glows/test_glows_l2_data.py	Add external-kernel test for ecliptic coordinate computation; update DailyLightcurve tests to use mocking fixture and include `imap_start_time`.
imap_processing/tests/glows/test_glows_l2.py	Inject new mocking fixture into L2 generation tests to avoid geometry-dependent SPICE calls.
imap_processing/tests/glows/conftest.py	Add `mock_ecliptic_bin_centers` fixture to monkeypatch the new SPICE-based computation.

Comments suppressed due to low confidence (1)

imap_processing/glows/l2/glows_l2_data.py:133

ecliptic_lon/ecliptic_lat are only assigned inside if self.number_of_bins:. When number_of_bins is 0 (e.g., no good data for a day), these attributes are left uninitialized and later access will raise AttributeError. Initialize them to empty arrays (or the correct fill values) before the conditional so the object is always in a valid state.

        if self.number_of_bins:
            # imap_spin_angle_bin_cntr is the raw IMAP spin angle ψ (0 - 360°,
            # bin midpoints).

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

imap_processing/glows/l2/glows_l2_data.py

Copilot · 2026-03-25T23:40:46Z

imap_processing/glows/l2/glows_l2_data.py

+        Histogram bin centers represent the center spin angle for each bin in the imap
+        frame, which corresponds to a specific pointing direction in space. This method
+        transforms the instrument pointing direction for each bin center from the IMAP
+        spacecraft frame to the ECLIPJ2000 frame.
+


The docstring says the pointing is transformed from the "IMAP spacecraft frame" to ECLIPJ2000, but the implementation builds az/el in SpiceFrame.IMAP_DPS and transforms from IMAP_DPS to ECLIPJ2000. Please update the docstring/parameter descriptions to match the actual frames to avoid confusion for future maintainers.

Agree. Perhaps this should read "This method transforms the instrument pointing direction for each bin center from the IMAP Pointing frame (IMAP_DPS) to the ECLIPJ2000 frame."

Your suggestions is much clearer, thanks! Are the IMAP spacecraft and DPS frames different?

Yes. The IMAP_SPACECRAFT frame is attached to the spacecraft and rotates with it. The IMAP_DPS frame is coincident with the s/c but does not rotate with it.

imap_processing/tests/glows/test_glows_l2_data.py

imap_processing/tests/glows/conftest.py

subagonsouth

Great! Everything looks good to me except for the one gotcha I mentioned about the time used for the transform_az_el call. Probably worth checking in with @maxinelasp about where that start time comes from.

subagonsouth · 2026-03-26T17:20:34Z

imap_processing/glows/l2/glows_l2_data.py

-            self.ecliptic_lon = np.roll(self.ecliptic_lon, roll)
-            self.ecliptic_lat = np.roll(self.ecliptic_lat, roll)
+            et_imap_start_time = sct_to_et(
+                met_to_sclkticks(l1b_data["imap_start_time"][0].data)


Something that recently came up to be aware of: If this start time is identically equal to the start time of the Pointing, it can sometimes have a type conversion error that causes the queried value to be outside the coverage window of the DPS kernel. If "imap_start_time" is a MET time that has been checked against the repoint table MET time to ensure it is in the Pointing, this implementation should be fine. If there is a chance of a rounding error, I suggest using the midpoint MET time instead of start time here.

That is not a safe assumption we can make here, so I think Tim's suggestion of midpoint timestamp makes the most sense. @subagonsouth, is this only for the sct_to_et conversion? Or are there other cases where this might be an issue?

The problem isn't really with the time conversion functions. Those functions will work for whatever time you give them. The problem arises when a certain series of time conversions happen to the Pointing start time that gets passed into a frame_transform call that uses the DPS kernel. I don't know how much detail you want but, here goes.

The DPS kernel has times represented in spacecraft clock format which is integer seconds and integer ticks (1 tick is 20us on IMAP). The repoint table also has times represented in sclk format. But, spice functions use double precision floating point TDB seconds (AKA ET). To complicate this even more, often in IMAP, we are storing the epoch as integer nanoseconds. So, a condition can arise where the conversion from sclk (MET) to epoch then back to TDB to pass into the frame_transform function which SPICE then converts back to sclk can result in an sclk value that is slightly before the sclk start time of the coverage provided by the DPS kernel. Since the DPS kernel is static during a pointing, the safest way to avoid this situation is to use the midpoint time of the Pointing.

subagonsouth · 2026-03-26T17:26:17Z

imap_processing/glows/l2/glows_l2_data.py

+        Histogram bin centers represent the center spin angle for each bin in the imap
+        frame, which corresponds to a specific pointing direction in space. This method
+        transforms the instrument pointing direction for each bin center from the IMAP
+        spacecraft frame to the ECLIPJ2000 frame.
+


Agree. Perhaps this should read "This method transforms the instrument pointing direction for each bin center from the IMAP Pointing frame (IMAP_DPS) to the ECLIPJ2000 frame."

imap_processing/glows/l2/glows_l2_data.py

imap_processing/tests/glows/test_glows_l2.py

tech3371 · 2026-03-27T14:53:48Z

Looks like Tim and Maxine is reviewing this PR. I will standby for now :)

maxinelasp

Nice work! Only thing is the change suggested by Tim - everything else looks good :) Once that is addressed, consider this approved by me

maxinelasp · 2026-03-27T15:20:37Z

imap_processing/glows/l2/glows_l2_data.py

-            self.ecliptic_lon = np.roll(self.ecliptic_lon, roll)
-            self.ecliptic_lat = np.roll(self.ecliptic_lat, roll)
+            et_imap_start_time = sct_to_et(
+                met_to_sclkticks(l1b_data["imap_start_time"][0].data)


That is not a safe assumption we can make here, so I think Tim's suggestion of midpoint timestamp makes the most sense. @subagonsouth, is this only for the sct_to_et conversion? Or are there other cases where this might be an issue?

subagonsouth

Other than the one nit-pick... Looks great!

subagonsouth · 2026-03-30T18:16:41Z

imap_processing/glows/l2/glows_l2_data.py

+            # Get the midpoint start time covered by repointing kernels
+            # needed to compute ecliptic coordinates
+            mid_idx = len(l1b_data["imap_start_time"]) // 2
+            et_imap_start_time = sct_to_et(


Nit pick: You might want to update this variable name to indicate that it is the Pointing midpoint time.

Good suggestion - renaming this variable will make this step more clear

…eclipitic coordinates of histogram bin centers to populate the DailyLightcurve ecliptic_lon and ecliptic_lat attributes.

…ts that were failing. Rebase and merge Maxine's spin angle work"

…o conftest. Modify method computing ecliptic coordinates to take two args rather than the l1b dataset

…s from a test that doesn't need it

…he repointing time coverage

…inting time

vmartinez-cu added this to the March 2026 milestone Mar 25, 2026

vmartinez-cu added this to IMAP Mar 25, 2026

vmartinez-cu added Ins: GLOWS Related to the GLOWS instrument Level: L2 Level 2 processing labels Mar 25, 2026

vmartinez-cu requested review from Copilot, maxinelasp, subagonsouth and tech3371 March 25, 2026 23:35

Copilot started reviewing on behalf of vmartinez-cu March 25, 2026 23:35 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

subagonsouth reviewed Mar 26, 2026

View reviewed changes

vmartinez-cu force-pushed the ecliptic-loc-cntr-bins branch from c086c01 to 42b524b Compare March 26, 2026 18:23

maxinelasp reviewed Mar 27, 2026

View reviewed changes

maxinelasp mentioned this pull request Mar 27, 2026

GLOWS L2 - Generate angular grids #917

Open

2 tasks

tech3371 removed this from the March 2026 milestone Mar 27, 2026

vmartinez-cu force-pushed the ecliptic-loc-cntr-bins branch from 42b524b to 8aadbc5 Compare March 30, 2026 17:13

subagonsouth approved these changes Mar 30, 2026

View reviewed changes

maxinelasp approved these changes Mar 30, 2026

View reviewed changes

vmartinez-cu added 8 commits March 30, 2026 13:16

Rebase dev to get recent glows updates needed. Add method to compute …

e573928

…eclipitic coordinates of histogram bin centers to populate the DailyLightcurve ecliptic_lon and ecliptic_lat attributes.

Mock method that computes ecliptic bin centers to update existing tes…

5d56a7f

…ts that were failing. Rebase and merge Maxine's spin angle work"

Add unit test for ecliptic coordinates (WIP)

dc4f7a5

Complete unit test for new method. Move mocked ecliptic coordinates t…

bd97ee2

…o conftest. Modify method computing ecliptic coordinates to take two args rather than the l1b dataset

Address PR comments - simplify docstrings and remove furnished kernel…

010387b

…s from a test that doesn't need it

Clarify docstring for mocked ecliptic coordinates

5a50f98

Address PR comments - use the midpoint time so that it falls within t…

1c7bfbd

…he repointing time coverage

Rename et time variable to clearly indicate that it's the midpoint po…

6015625

…inting time

vmartinez-cu force-pushed the ecliptic-loc-cntr-bins branch from 8aadbc5 to 6015625 Compare March 30, 2026 19:16

vmartinez-cu merged commit 48641ea into IMAP-Science-Operations-Center:dev Mar 30, 2026
14 checks passed

github-project-automation bot moved this to Done in IMAP Mar 30, 2026

vmartinez-cu deleted the ecliptic-loc-cntr-bins branch March 30, 2026 19:42

Conversation

vmartinez-cu commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

subagonsouth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tech3371 commented Mar 27, 2026

Uh oh!

maxinelasp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

subagonsouth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants