Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #188 +/- ##
==========================================
+ Coverage 80.48% 80.89% +0.41%
==========================================
Files 102 102
Lines 15456 15758 +302
Branches 1488 1515 +27
==========================================
+ Hits 12440 12748 +308
+ Misses 2502 2500 -2
+ Partials 514 510 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The pixel_geometry docstring currently implies equivalence with surface_angles for aberration correction behavior, but the implementation applies perspective_correction to the Sun query whereas surface_angles does not, which should be clarified before release.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new per-pixel geometry computation path to curryer.compute.spatial so callers can compute ellipsoid intersections and surface/viewing/solar angles for very large focal planes using pure NumPy arrays (avoiding pandas (time, pixel) product indices), while keeping conventions consistent with existing DataFrame-based products.
Changes:
- Introduces
compute.spatial.pixel_geometry(...) -> PixelGeometryfor per-time/per-pixel intersection + angles + quality flags, and adds a publiccompute.spatial.relative_azimuth. - Refactors
calc_azimuth/calc_zenithto share local-frame helper functions, and adjustsray_intersect_ellipsoidto avoidsqrtwarnings on misses (NaN discriminants). - Adds extensive unit/integration tests plus version bump and changelog entry for 0.5.2.
File summaries
| File | Description |
|---|---|
curryer/compute/spatial.py |
Adds pixel_geometry, PixelGeometry, relative_azimuth, shared local-frame helpers, and warning-free miss handling in ray_intersect_ellipsoid. |
curryer/compute/geometry.py |
Switches boresight relative azimuth to the new shared spatial.relative_azimuth helper for convention consistency. |
tests/test_compute/test_compute_spatial.py |
Adds comprehensive unit and integration coverage for the new pixel-geometry path and helper refactors. |
docs/source/changelog.md |
Documents the new API and behavior changes under unreleased 0.5.2. |
pyproject.toml |
Bumps project version from 0.5.1 to 0.5.2. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| perspective_correction : str, optional | ||
| SPICE aberration correction applied to both the instrument and the Sun position | ||
| queries (e.g. ``"LT+S"``). Default None applies none, matching | ||
| `compute_ellipsoid_intersection` and `surface_angles` (which never corrects the Sun | ||
| query). |
| times SPICE could not resolve. | ||
| """ | ||
|
|
||
| lon: np.ndarray |
There was a problem hiding this comment.
We should handle this differently. libera_cam should specify the fields it wants, and curryer chooses how to compute and provide them. This is too mission specific, and we shouldn't have all of these libera variables be included in the default pixel imager geometry code... let's try to coordinat, mirror, and extend libera_rad functionality more closely, so that we have a consolidated and versatile geometry that can scale from 1 to n pixel vectors efficiently but not totally independently in methodology
…e geometry Vectorized per-pixel ellipsoid intersection and surface angles with ndarray in and out; calc_azimuth and calc_zenith share the local-frame math. ray_intersect_ellipsoid masks misses without a NumPy warning. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…does not Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pixel_geometry takes `fields=` and returns a {column: array} dict instead of a
fixed PixelGeometry NamedTuple, mirroring GeometryData: name the fields you want
and only the work behind them runs. A request with no solar field skips the Sun
ephemeris query outright; one with no angle skips the local-frame construction.
Measured 3.3x on the per-pixel numpy work for a geodetic-only request at one
million pixels.
The five surface-angle members share their selector and column names with the
GeometryField members of the same name, so a boresight product and a per-pixel
product address the same quantity by the same key. GeometryField and PixelField
share a _FieldEnum base rather than duplicating the (selector, columns,
description) machinery.
Spacecraft and Sun positions are no longer returned. They are per-time rather
than per-pixel, and GeometryData already serves them.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
399730d to
fef750c
Compare
Summary
curryer can now geolocate a focal plane one frame at a time: ndarrays in, ndarrays out, computing only the fields the caller asks for. The pandas
(time, pixel)product index does not scale to the Libera WFOV camera's 4.19 M pixels per frame (LIBSDC-808), and a Dask worker wants arrays anyway.The two new names
compute.spatial.pixel_geometry(ugps_times, instrument, pointing_vectors, *, fields, sun, perspective_correction, allow_nans, degrees, observer_id, fixed_frame_name) -> dict[str, np.ndarray]makes at most three SPICE calls per time (instrument rotation and position, Sun position). Everything per pixel is closed-form numpy already in the module:ray_intersect_ellipsoid, the on-surface geodetic conversion, and the azimuth and zenith of the spacecraft and the Sun at each intersection. Viewing angles reuse the spacecraft position the intersection queried, so there is no second ephemeris read.compute.geometry_fields.PixelFieldis its field registry, mirroringGeometryField: members are(selector, columns, description)triples, plain strings, selected withfields=[...]. Only the work behind the requested fields runs. A call with no solar field skips the Sun ephemeris query outright; one with no angle skips the local-frame construction.The five surface-angle members share their selector and column names with the
GeometryFieldmembers of the same name, because they are the same quantity evaluated at a pixel rather than at the boresight. A boresight product and a per-pixel product address it by the same key, andGeometryData'srelative_azimuthfield now calls the same public helper the pixel path does, so the two agree by construction rather than by convention.GeometryFieldandPixelFieldshare a_FieldEnumbase instead of duplicating the enum machinery;GeometryField's behaviour is unchanged, including the lunar members #186 added.What selection is worth
At one million pixels, one time, mocked SPICE, so this is the per-pixel numpy work alone. Skipping the Sun query comes on top of it.
SURFACE_GEODETIC+QUALITY_FLAGSSURFACE_GEODETICaloneAsking for everything is no slower than before, which was the other half of the requirement.
Return shape
A
{column: (n_times, n_pixels) ndarray}dict, keyed byPixelField.columnsthe wayGeometryData.get_geometrykeys its DataFrame, so a consumer readsresult["viewing_zenith"]whether the product is per-boresight or per-pixel.quality_flagsis the one integer column and is deliberately not cast to float the wayget_vectorscasts its output, so the bitmask stays exact.Spacecraft and Sun positions are not pixel fields. They are per-time rather than per-pixel, and
GeometryDataalready serves them.Other changes
compute.spatial.relative_azimuthbecomes public (CERES BDS R3V4 origin, Sun at 180, unfolded).calc_azimuthandcalc_zenithnow share one local-frame helper, andpixel_geometrybuilds that frame once per time from the intersection's own lon/lat instead of re-running the Ferrari conversion inside each angle call: intersection plus all angles goes from 0.53 to 0.22 s per million pixels, bitwise unchanged for float64 inputs.ray_intersect_ellipsoidmasks the negative discriminant rather than lettingsqrtwarn on every miss; zero-length vectors still warn, andpixel_geometryrejects them._surface_xyz_to_geodeticis extracted, behaviour-preserving, so the pixel path can reuse it. One docstring fix:compute_ellipsoid_intersectiontakes GPS microseconds, not seconds. Version 0.5.2 to 0.5.3.Rebased onto main
0.5.2 released while this branch was open, so the version moves to 0.5.3 and the changelog gains its own section above main's intact 0.5.2 entry.
Two conflicts, both from main and this branch appending to the same files. In
spatial.py, main'sboresight_offset_angles(#186) and this branch's local-frame helpers landed at the same point; both are kept. Ingeometry_fields.py, #186 added the threeMOON_*members alongside the__new__that this branch had lifted into_FieldEnum; the members are kept and the duplicate__new__dropped, since the shared base now provides it.GeometryData's moon provider is untouched.Notes
pixel_geometrydocuments a transient working set of about 250 bytes per pixel (tracemalloc at one million pixels), so callers with millions of pixels should pass a few times per call.The ancillary quality flags describe only the angles actually requested. A call that asks for no solar field never raises
CALC_ANCIL_INSUFF_DATA.perspective_correctionreaches both the instrument and the Sun query here, whereassurface_anglesalways queries the Sun uncorrected. The docstring says so. The two agree when no correction is given, which is the default and what Libera passes.Nothing outside curryer and LASP-Libera/libera_cam#19 references
pixel_geometryor the removedPixelGeometryNamedTuple, checked across libera_utils, libera_rad, libera_analysis and CSDS. The API has not been released, so replacing the NamedTuple with a dict breaks no consumer. libera_cam#19 is updated in step and pinned to this branch. No consumer referencesgeometry._relative_azimuthor relies on the removedRuntimeWarningeither.Tests
Unit tests with mocked SPICE cover nadir, off-nadir and miss pixels with pinned values, exact equivalence with
compute_ellipsoid_intersectionplussurface_angles, the radians path against a degrees run, theSpiceyErrorraise underallow_nans=False, every flag combination (CALC_ELLIPS_NO_INTERSECT,CALC_ELLIPS_INSUFF_DATAwith the SPICE cause,CALC_ANCIL_INSUFF_DATA,CALC_ANCIL_NOT_FINITE), invalid pointing vectors,perspective_correctionreaching both queries, the miss path emitting no warning, and thesignedandgeocentricbranches of the shared helpers.Field selection adds four. A subset returns only its own columns with values identical to a full call. A request with no solar field asserts
query_ephemerisis never called and noCALC_ANCIL_INSUFF_DATAis raised.RELATIVE_AZIMUTHrequested alone still computes the two azimuths it derives from. An unknown field name raisesKeyError.An integration test on the CPRS kernels checks the 480-pixel path against the DataFrame path and against SPICE
azlcpoat the middle pixel (atol 1e-5 deg; measured disagreement below 1e-6).tests/test_computeplustests/test_correctionafter the rebase: 559 passed, 21 skipped, no failures. The merge seams are covered —test_compute_geometry.py(81 tests,GeometryFieldand the lunar members) andtest_compute_spatial.py(70 tests, includingboresight_offset_angles) both pass clean. pre-commit clean. The--run-extrafailures in this checkout are a missing localdata/gmtedelevation dataset and a pre-existing pandas/spiceypy readonly-array incompatibility, both unrelated to this diff and both reproduced with it stashed.🤖 Generated with Claude Code