Skip to content

Interpret grid_input as cell vertices and trace rays at cell centers - #187

Open
roytsmart wants to merge 1 commit into
mainfrom
feature/grid-input-vertices
Open

Interpret grid_input as cell vertices and trace rays at cell centers#187
roytsmart wants to merge 1 commit into
mainfrom
feature/grid-input-vertices

Conversation

@roytsmart

Copy link
Copy Markdown
Collaborator

Summary

The components of grid_input (and of the grids given to distortion() and vignetting()) are now interpreted as cell vertices, and the rays of the system are traced at the corresponding cell centers, so the inputs of rayfunction_default and of the rayfunctions used by the fitting methods are defined on cell vertices while the outputs are defined on cell centers (a first-class named-arrays concept via FunctionArray.axes_vertex).

Why:

  • Makes the system grids consistent with the vertex convention already used by image() (whose pupil parameter was already documented as vertices).
  • Enables area-weighted methods, such as the upcoming effective area calculation, which need pupil cell areas from the vertices.
  • Rays are never traced exactly on aperture edges (cell centers are strictly interior), avoiding knife-edge vignetting comparisons.

Implementation:

  • _grid_input_physical / _grid_input_centers cached properties (denormalized vertices and their centers); _rayfunction_input traces the centers but attaches the vertex grid as inputs.
  • _rayfunction_and_axes resolves grids (components left as None inherit from grid_input), infers axes, denormalizes, centers, traces, and attaches the vertex inputs; distortion()/vignetting() compute their scene coordinates as rays.inputs.cell_centers(...) so calibration points align with the outputs.
  • raytrace()/rayfunction() keep their exact-sample-point semantics, since image() passes precomputed cell centers with stratified random offsets.
  • spot_diagram() draws its layout, wavelength colors, and field labels from the centered physical grid.

Migration note for downstream configurations

Grids built with na.*LinearSpace(..., centers=True) should drop centers=True and increment num by one, which reproduces the previously traced rays exactly (esis and cool-aid each have one or two such sites).

Tests

Full optika suite run locally: all passing (verified against the new named-arrays release together with #185). Verified directly that rayfunction_default.inputs holds the vertex grid, outputs are traced at centers, and axes_vertex reports all five grid axes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Lm9SxSpyNg9hx8dNL9pUXc

The components of grid_input (and of the grids given to distortion()
and vignetting()) are now interpreted as cell vertices, and the rays of
the system are traced at the corresponding cell centers, so that the
inputs of rayfunction_default and of the rayfunctions used by the
fitting methods are defined on cell vertices while the outputs are
defined on cell centers. This makes the system grids consistent with
the vertex convention already used by image(), enables area-weighted
methods such as the upcoming effective area calculation, and avoids
tracing rays exactly on the edges of the apertures.

raytrace() and rayfunction() keep their exact-sample-point semantics,
since image() passes precomputed cell centers with stratified random
offsets.

Migration note for downstream configurations: grids built with
na.*LinearSpace(..., centers=True) should drop centers=True and
increment num by one, which reproduces the previously traced rays
exactly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lm9SxSpyNg9hx8dNL9pUXc
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.33%. Comparing base (001b708) to head (6f31a3d).
⚠️ Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
optika/systems/_sequential.py 87.50% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #187      +/-   ##
==========================================
- Coverage   99.37%   99.33%   -0.05%     
==========================================
  Files         116      116              
  Lines        6282     6302      +20     
==========================================
+ Hits         6243     6260      +17     
- Misses         39       42       +3     
Flag Coverage Δ
unittests 99.33% <87.50%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

roytsmart added a commit that referenced this pull request Aug 28, 2026
* Solve for the stops once when linearizing, not three times

Denormalizing a grid does two unrelated things: it solves for where the
stops put the field and pupil, and it applies an affine map.  The solve
costs 0.13 s and depends on nothing but the wavelengths; the map is free.

`linearize` denormalizes three grids on the same wavelengths, one per
fit, so it solved three times for the same answer, and a fourth for
`rayfunction_default`.  Split the two halves apart, so that a caller
with several grids can solve once:

  _calc_rayfunction_stops_denormalize  the solve
  _denormalize_grid_from_rays          the affine map
  _denormalize_grid                    both, as before

and have `linearize` solve once, denormalize both of its grids, and pass
each fit a grid which is already physical, so their own denormalization
is a no-op.

  before: 4 stop solves, 0.59 s of 3.98 s
  after:  2 stop solves, 0.29 s of 3.59 s

The one which remains is `rayfunction_default`'s, on the default grid
rather than the one being linearized.

Since the fits can no longer be handed a grid of `None`, `linearize`
resolves the defaults itself, and the pupil grid `area_effective`
invents when given none becomes `_pupil_vertices_default` so that both
can reach it.  The two fits keep falling back to `grid_input.pupil` as
before: those grids differ because one wants cell vertices and the other
wants sample points, and reconciling them is #187's job, not this one's.

Every product of `linearize` is unchanged bit for bit: all twenty
distortion coefficients, all ten vignetting coefficients, the direction,
the scene and sensor coordinates the distortion is fit to, and the
illumination the vignetting is fit to.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Drop the wrapper by fixing the solver's defaults

`_calc_rayfunction_stops_denormalize` existed only to fill in four
arguments which every caller passes identically.  It is not needed: the
defaults on `_calc_rayfunction_stops` were simply wrong.  Its axes had
no defaults at all even though both callers pass the class constants,
and its sample counts defaulted to 101, which nothing has ever used.

Give it the values its callers actually want, and the wrapper and the
boilerplate in `rayfunction_stops` both go away.

Net 27 lines fewer, and the products of `linearize` remain identical to
main across all 37 arrays.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant