Interpret grid_input as cell vertices and trace rays at cell centers - #187
Open
roytsmart wants to merge 1 commit into
Open
Interpret grid_input as cell vertices and trace rays at cell centers#187roytsmart wants to merge 1 commit into
grid_input as cell vertices and trace rays at cell centers#187roytsmart wants to merge 1 commit into
Conversation
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 Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Aug 27, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The components of
grid_input(and of the grids given todistortion()andvignetting()) are now interpreted as cell vertices, and the rays of the system are traced at the corresponding cell centers, so the inputs ofrayfunction_defaultand 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 viaFunctionArray.axes_vertex).Why:
image()(whosepupilparameter was already documented as vertices).Implementation:
_grid_input_physical/_grid_input_centerscached properties (denormalized vertices and their centers);_rayfunction_inputtraces the centers but attaches the vertex grid asinputs._rayfunction_and_axesresolves grids (components left asNoneinherit fromgrid_input), infers axes, denormalizes, centers, traces, and attaches the vertex inputs;distortion()/vignetting()compute their scene coordinates asrays.inputs.cell_centers(...)so calibration points align with the outputs.raytrace()/rayfunction()keep their exact-sample-point semantics, sinceimage()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 dropcenters=Trueand incrementnumby 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.inputsholds the vertex grid, outputs are traced at centers, andaxes_vertexreports all five grid axes.🤖 Generated with Claude Code
https://claude.ai/code/session_01Lm9SxSpyNg9hx8dNL9pUXc