Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Index Mapping Util for Rectangular grids #1258

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 168 additions & 0 deletions imap_processing/ena_maps/map_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
"""Utilities for generating ENA maps."""

from __future__ import annotations

import logging
import typing

import numpy as np
from numpy.typing import NDArray

from imap_processing.spice import geometry
from imap_processing.ultra.utils import spatial_utils

logger = logging.getLogger(__name__)


# Ignore linting rule to allow for 6 unrelated args
# Also need to manually specify allowed str literals for order parameter
def match_rect_indices_frame_to_frame( # noqa: PLR0913
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this function is doing too much...

  • building both az_el grids
  • flattening
  • doing transforms
  • generating indices for mapping both directions
  • reshaping back to 2D

To make it more general, flexible, and reusable, I would like to see the following function extracted.

def map_2d_coords_to_pixels(coords: NDArray, pixel_edges: Tuple(array_like), order: typing.Literal["C"] | typing.Literal["F"] = "F") -> NDArray:
    """
    Compute the flat indices that map the input coordinates to the pixels defined by pixel_edges.

    Parameters
    -----------
    coords : ndarray with shape (N, 2) where N is the number of input coordinate pairs.
    pixel_edges: Tuple[ndarray] where pixel_edges[0] defines the 0th axis pixel edges...
    order: order to use when converting from tuple of index arrays into an array of flat indices

    Returns
    --------
    indices : ndarray
        Flat array of indices with size = N that map the input coordinates into an array of pixels defined by the input pixel_edges that has been flattened with the same `order` as input.
    """

The whole push-pull idea is likely going to be specific to Ultra, so avoiding baking that in at this stage would be good. The function that builds the grids will likely want to be moved into this common map_utils module, though that can be in another PR.

Copy link
Contributor Author

@nkerman nkerman Jan 13, 2025

Choose a reason for hiding this comment

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

Okay, to confirm, you do see there being some overarching function that does basically this, with its components broken out a bit into sub-functions?

For Ultra, I think we want to be able to hand a single function input+output frames, and input+output spacings, and have it match up the indices, which does involve all these steps, with the exception of reshaping back to 2D, which doesn't currently happen here. I think you're referencing the ravel_multi_index, which is needed to convert from two small 1D indices (projected az bin, projected el bin) to a single 1D index of the 'unwrapped' sky.

The whole push-pull idea is likely going to be specific to Ultra, so avoiding baking that in at this stage would be good.

Agreed! Is there anywhere in this code currently that talks about push/pull, or did you mean this as more of a precaution?

The function that builds the grids will likely want to be moved into this common map_utils module, though that can be in another PR.

Got it - I made this ticket #1260 . Is it "cleaner" to do so in another PR? It seems messy to me to have all these PRs interacting with the same code, so I'd lean towards doing it here, but maybe that makes reviewing more difficult?

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, to confirm, you do see there being some overarching function that does basically this, with its components broken out a bit into sub-functions? For Ultra, I think we want to be able to hand a single function input+output frames, and input+output spacings, and have it match up the indices, which does involve all these steps (except reshaping back to 2D, which doesn't happen here. I think you're referencing the ravel_multi_index, which is needed to convert from two small 1D indices (projected az bin, projected el bin) to a single 1D index of the 'unwrapped' sky.

I'm not sure about an overarching function. For example, this function as written will regenerate the Map coordinates (azimuth, elevation, and pixel edges) for each PSET that you are binning. It would be good to avoid that by generating that once at the start of L2 processing and reuse throughout. Likewise, for the coordinate grid of each PSET, does Ultra store those in the PSET product? or do they really need to be regenerated in this function?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed! Is there anywhere in this code currently that talks about push/pull, or did you mean this as more of a precaution?
You're right. There is no push pull in this function. I wasn't careful enough reading the descriptions of the outputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it! I think I agree in principle with the goal of not re-generating the map coordinates for each PSET that gets added, but that relies on there already being something like a map object set up, and I wonder if we're edging closer to just implementing that, when we had been talking about a more focused set of self-contained utils for starting Ultra L2.

Likewise, for the coordinate grid of each PSET, does Ultra store those in the PSET product? or do they really need to be regenerated in this function?

The L1c files will have the separate coordinates (i.e the 1D grids of az, el), but the np.meshgrid would need to be constructed and unwrapped. I'm not sure I see the harm in regenerating these relatively lightweight grids.

Copy link
Contributor

@subagonsouth subagonsouth Jan 14, 2025

Choose a reason for hiding this comment

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

but that relies on there already being something like a map object set up, and I wonder if we're edging closer to just implementing that, when we had been talking about a more focused set of self-contained utils for starting Ultra L2.

Perhaps this is true. I think that things are going in the right direction here and common, reusable code is being identified. The reason I identify this re-generation of coordinates as an issue is not due to inefficiencies/performance reasons, it is because it becomes more difficult to disentangle down the road and is inflexible. Specifically, I have two concerns:

  • What happens if Ultra decides they need to change the PSET grid spacing at some point in time or need different spacings for different energy bands. L2 needs to get the grid spacing from the PSET product so that different resolution products can be combined. Looking at the code again I realize this could be handled with the current implementation.
  • Similarly, Hi and Lo have a 1D spatial array in the PSET products rather than 2D and don't, AFAIK, have any need for pull binning. With that in mind, there is talk of needing to combine Hi and Ultra data, so assumptions about the input PSET dimensionality need to be removed as much as possible.

input_frame: geometry.SpiceFrame,
projection_frame: geometry.SpiceFrame,
event_time: float,
input_frame_spacing_deg: float,
projection_frame_spacing_deg: float,
order: typing.Literal["C"] | typing.Literal["F"] = "F",
) -> tuple[NDArray, NDArray, NDArray, NDArray, NDArray, NDArray, NDArray, NDArray]:
"""
Match flattened rectangular grid indices between two reference frames.

Parameters
----------
input_frame : geometry.SpiceFrame
The frame in which the input grid is defined.
projection_frame : geometry.SpiceFrame
The frame to which the input grid will be projected.
event_time : float
The time at which to project the grid.
input_frame_spacing_deg : float, optional
The spacing of the input frame grid in degrees,
by default DEFAULT_SPACING_DEG.
projection_frame_spacing_deg : float, optional
The spacing of the projection frame grid in degrees,
by default DEFAULT_SPACING_DEG.
order : str, optional
The order of unraveling/re-raveling the grid points,
by default "F" for column-major Fortran ordering.
See numpy.ravel documentation for more information.

Returns
-------
tuple[NDArray]
Tuple of the following arrays:
- Ordered 1D array of pixel indices covering the entire input grid.
Guaranteed to have one index for each point on the input grid,
(meaning it will cover the entire input grid).
Size is number of pixels on input grid
`= (az_grid_input.size * el_grid_input.size)`.
- Ordered 1D array of indices in the projection frame
corresponding to each index in the input grid.
Not guaranteed to cover the entire projection grid.
Size is the same as the input indices
Copy link
Contributor

Choose a reason for hiding this comment

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

The use of "input indices" throughout this docstring is confusing. It had me re-reading the input parameter descriptions looking for indices that were input to the function.

= `(az_grid_input.size * el_grid_input.size)`.
- Azimuth values of the input grid points in the input frame, raveled.
Same size as the input indices.
- Elevation values of the input grid points in the input frame, raveled.
Same size as the input indices.
- Azimuth values of the input grid points in the projection frame, raveled.
Same size as the input indices.
- Elevation values of the input grid points in the projection frame, raveled.
Same size as the input indices.
- Azimuth indices of the input grid points in the projection frame az grid.
Same size as the input indices.
- Elevation indices of the input grid points in the projection frame el grid.
Same size as the input indices.
"""
if input_frame_spacing_deg > projection_frame_spacing_deg:
logger.warning(
"Input frame has a larger spacing than the projection frame."
f"\nReceived: input = {input_frame_spacing_deg} degrees "
f"and {projection_frame_spacing_deg} degrees."
)
# Build the azimuth, elevation grid in the inertial frame
az_grid_in, el_grid_in = spatial_utils.build_az_el_grid(
spacing=input_frame_spacing_deg,
input_degrees=True,
output_degrees=False,
centered_azimuth=False, # (0, 2pi rad = 0, 360 deg)
centered_elevation=True, # (-pi/2, pi/2 rad = -90, 90 deg)
)[2:4]

# Unwrap the grid to a 1D array for same interface as tessellation
az_grid_input_raveled = az_grid_in.ravel(order=order)
el_grid_input_raveled = el_grid_in.ravel(order=order)

# Get the flattened indices of the grid points in the input frame (Fortran order)
# TODO: Discuss with Nick/Tim/Laura: Should this just be an arange?
flat_indices_in = np.ravel(
np.arange(az_grid_input_raveled.size).reshape(az_grid_in.shape),
order=order,
)

radii_in = geometry.spherical_to_cartesian(
np.stack(
(
np.ones_like(az_grid_input_raveled),
az_grid_input_raveled,
el_grid_input_raveled,
),
axis=-1,
)
)

# Project the grid points from the input frame to the projection frame
# radii_proj are cartesian (x,y,z) radii vectors in the projection frame
# corresponding to the grid points in the input frame
radii_proj = geometry.frame_transform(
et=event_time,
position=radii_in,
from_frame=input_frame,
to_frame=projection_frame,
)

# Convert the (x,y,z) vectors to spherical coordinates in the projection frame
# Then extract the azimuth, elevation angles in the projection frame. Ignore radius.
input_grid_in_proj_spherical_coord = geometry.cartesian_to_spherical(
radii_proj, degrees=False
)

input_az_in_proj_az = input_grid_in_proj_spherical_coord[:, 1]
input_el_in_proj_el = input_grid_in_proj_spherical_coord[:, 2]

# Create bin edges for azimuth (0 to 2pi) and elevation (-pi/2 to pi/2)
proj_frame_az_bin_edges, proj_frame_el_bin_edges = spatial_utils.build_az_el_grid(
spacing=projection_frame_spacing_deg,
input_degrees=True,
output_degrees=False,
centered_azimuth=False, # (0, 2pi rad = 0, 360 deg)
centered_elevation=True, # (-pi/2, pi/2 rad = -90, 90 deg)
)[4:6]

# Use digitize to find indices (-1 since digitize returns 1-based indices)
input_az_in_proj_az_indices = (
np.digitize(input_az_in_proj_az, proj_frame_az_bin_edges) - 1
)
input_el_in_proj_el_indices = (
np.digitize(input_el_in_proj_el, proj_frame_el_bin_edges[::-1]) - 1
)

# NOTE: This method of matching indices 1:1 is rectangular grid-focused
# It will not necessarily work with a tessellation (e.g. Healpix)
flat_indices_proj = np.ravel_multi_index(
multi_index=(input_az_in_proj_az_indices, input_el_in_proj_el_indices),
dims=(
int(360 // projection_frame_spacing_deg),
int(180 // projection_frame_spacing_deg),
),
order=order,
)
return (
flat_indices_in,
flat_indices_proj,
az_grid_input_raveled,
el_grid_input_raveled,
input_az_in_proj_az,
input_el_in_proj_el,
input_az_in_proj_az_indices,
input_el_in_proj_el_indices,
Comment on lines +166 to +167
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these indices needed? I'm trying to understand if there is a way to simplify this function signature. Complex function signatures are generally a sign that the function is overly complex, or doing too many things. Also, staying away from 2D indices will be helpful for if/when HEALPix is added.

)
103 changes: 103 additions & 0 deletions imap_processing/tests/ena_maps/test_map_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
"""Tests coverage for ENA Map Mapping Util functions."""

from unittest import mock

import numpy as np
import pytest

from imap_processing.ena_maps import map_utils
from imap_processing.spice import geometry
from imap_processing.ultra.utils import spatial_utils


class TestENAMapMappingUtils:
@mock.patch("imap_processing.ena_maps.map_utils.geometry.frame_transform")
def test_match_rect_indices_frame_to_frame_same_frame(self, mock_frame_transform):
# Mock frame transform to return the input position vectors (no transform)
mock_frame_transform.side_effect = (
lambda et, position, from_frame, to_frame: position
)
et = -1
spacing_deg = 1
(
flat_indices_in,
flat_indices_proj,
az_grid_input_raveled,
el_grid_input_raveled,
input_az_in_proj_az,
input_el_in_proj_el,
) = map_utils.match_rect_indices_frame_to_frame(
input_frame=geometry.SpiceFrame.IMAP_DPS,
projection_frame=geometry.SpiceFrame.IMAP_DPS,
event_time=et,
input_frame_spacing_deg=spacing_deg,
projection_frame_spacing_deg=spacing_deg,
)[:6]

# The two arrays should be the same if the input and projection
# frames are the same and the spacing is the same.
np.testing.assert_array_equal(flat_indices_in, flat_indices_proj)

# The input and projection azimuth and elevation grids should be the same
np.testing.assert_allclose(az_grid_input_raveled, input_az_in_proj_az)
np.testing.assert_allclose(el_grid_input_raveled, input_el_in_proj_el)

# The projection azimuth and elevation, when reshaped to a 2D grid,
# should be the same as the input grid created with build_az_el_grid.
np.testing.assert_allclose(
input_az_in_proj_az.reshape(
(180 // spacing_deg, 360 // spacing_deg), order="F"
),
spatial_utils.build_az_el_grid(spacing=np.deg2rad(spacing_deg))[2],
)
np.testing.assert_allclose(
input_el_in_proj_el.reshape(
(180 // spacing_deg, 360 // spacing_deg), order="F"
),
spatial_utils.build_az_el_grid(spacing=np.deg2rad(spacing_deg))[3],
)

@mock.patch("imap_processing.ena_maps.map_utils.geometry.frame_transform")
@pytest.mark.parametrize("input_spacing_deg", [1 / 4, 1 / 3, 1 / 2, 1])
def test_match_rect_indices_frame_to_frame_different_spacings(
self, mock_frame_transform, input_spacing_deg
):
# Mock frame transform to return the input position vectors (no transform)
mock_frame_transform.side_effect = (
lambda et, position, from_frame, to_frame: position
)

et = -1
proj_spacing_deg = 1

(
flat_indices_in,
flat_indices_proj,
) = map_utils.match_rect_indices_frame_to_frame(
input_frame=geometry.SpiceFrame.IMAP_DPS,
projection_frame=geometry.SpiceFrame.IMAP_DPS,
event_time=et,
input_frame_spacing_deg=input_spacing_deg,
projection_frame_spacing_deg=proj_spacing_deg,
)[:2]

# The input indices should still be the same length as the projection indices
assert len(flat_indices_in) == len(flat_indices_proj)

# However, the min and max of the input indices should go from:
# 0 to (180/spacing_deg of the input)*(360/spacing_deg of the input)
# and the min and max of the projection indices should go from:
# 0 to (180/spacing_deg of the projection)*(360/spacing_deg of the projection)
assert flat_indices_in.min() == 0
assert flat_indices_in.max() == (
(180 / (input_spacing_deg)) * (360 / (input_spacing_deg)) - 1
)
assert flat_indices_proj.min() == 0
assert (
flat_indices_proj.max()
== ((180 / proj_spacing_deg) * (360 / proj_spacing_deg)) - 1
)

# There should be (proj_spacing_deg/input_spacing_deg)**2
# instances of each projection index, and proj_spacing_deg = 1.
assert np.all(np.bincount(flat_indices_proj) == 1 / (input_spacing_deg**2))
20 changes: 14 additions & 6 deletions imap_processing/tests/ultra/unit/test_spatial_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,14 @@ def test_build_solid_angle_map_invalid_spacing(spacing, match_str):
@pytest.mark.parametrize("spacing", valid_spacings)
def test_build_az_el_grid(spacing):
"""Test build_az_el_grid function."""
az_range, el_range, az_grid, el_grid = spatial_utils.build_az_el_grid(
spacing=spacing,
input_degrees=True,
output_degrees=True,
centered_azimuth=False,
centered_elevation=True,
(az_range, el_range, az_grid, el_grid, az_bin_edges, el_bin_edges) = (
spatial_utils.build_az_el_grid(
spacing=spacing,
input_degrees=True,
output_degrees=True,
centered_azimuth=False,
centered_elevation=True,
)
)

# Size checks
Expand All @@ -92,6 +94,12 @@ def test_build_az_el_grid(spacing):
npt.assert_allclose(az_range, expected_az_range, atol=1e-12)
npt.assert_allclose(el_range, expected_el_range, atol=1e-12)

# Check bin edges
expected_az_bin_edges = np.arange(0, 360 + spacing, spacing)
expected_el_bin_edges = np.arange(-90, 90 + spacing, spacing)
npt.assert_allclose(az_bin_edges, expected_az_bin_edges, atol=1e-11)
npt.assert_allclose(el_bin_edges, expected_el_bin_edges, atol=1e-11)


def test_rewrap_even_spaced_el_az_grid_1d():
"""Test rewrap_even_spaced_el_az_grid function, without extra axis."""
Expand Down
24 changes: 20 additions & 4 deletions imap_processing/ultra/utils/spatial_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ def build_az_el_grid(
output_degrees: bool = False,
centered_azimuth: bool = False,
centered_elevation: bool = True,
) -> tuple[NDArray, NDArray, NDArray, NDArray]:
) -> tuple[NDArray, NDArray, NDArray, NDArray, NDArray, NDArray]:
"""
Build a 2D grid of azimuth and elevation angles.
Build a 2D grid of azimuth and elevation angles, and their 1D bin edges.

Azimuth and Elevation values represent the center of each grid cell,
so the grid is offset by half the spacing.
Expand All @@ -123,7 +123,7 @@ def build_az_el_grid(

Returns
-------
tuple[NDArray, NDArray, NDArray, NDArray]
tuple[NDArray, NDArray, NDArray, NDArray, NDArray, NDArray]
- The evenly spaced, 1D range of azimuth angles
e.g.(0, 0.5, 1, ..., 359.5) deg.
- The evenly spaced, 1D range of elevation angles
Expand All @@ -132,6 +132,12 @@ def build_az_el_grid(
This grid will be constant along the elevation (0th) axis.
- The 2D grid of elevation angles (elevations for each azimuth).
This grid will be constant along the azimuth (1st) axis.
- The 1D bin edges for azimuth angles.
e.g. if spacing=1 deg:
az_bin_edges = [0, 1, 2, ..., 359, 360] deg.
- The 1D bin edges for elevation angles.
e.g. if spacing=1 deg:
el_bin_edges = [-90, -89, -88, ..., 89, 90] deg.

Raises
------
Expand All @@ -147,6 +153,14 @@ def build_az_el_grid(
if not np.isclose((np.pi / spacing) % 1, 0):
raise ValueError("Spacing must divide evenly into pi radians.")

# Create the bin edges for azimuth and elevation.
# E.g. for spacing=1, az_bin_edges = [0, 1, 2, ..., 359, 360] deg.
el_bin_edges = np.linspace(-np.pi / 2, np.pi / 2, int(np.pi / spacing) + 1)
az_bin_edges = np.linspace(0, 2 * np.pi, int(2 * np.pi / spacing) + 1)

# Create the 2D grid of azimuth and elevation angles at center of each bin.
# These ranges are offset by half the spacing and are
# one element shorter than the bin edges.
el_range = np.arange(spacing / 2, np.pi, spacing)
az_range = np.arange(spacing / 2, 2 * np.pi, spacing)
if centered_azimuth:
Expand All @@ -165,8 +179,10 @@ def build_az_el_grid(
el_range = np.rad2deg(el_range)
az_grid = np.rad2deg(az_grid)
el_grid = np.rad2deg(el_grid)
az_bin_edges = np.rad2deg(az_bin_edges)
el_bin_edges = np.rad2deg(el_bin_edges)

return az_range, el_range, az_grid, el_grid
return az_range, el_range, az_grid, el_grid, az_bin_edges, el_bin_edges


@typing.no_type_check
Expand Down
Loading