Add grid() interior aperture sampling (toward robust field/pupil sampling)#180
Open
jacobdparker wants to merge 2 commits into
Open
Add grid() interior aperture sampling (toward robust field/pupil sampling)#180jacobdparker wants to merge 2 commits into
grid() interior aperture sampling (toward robust field/pupil sampling)#180jacobdparker wants to merge 2 commits into
Conversation
…SectorAperture` `wire()` samples an aperture's boundary; `grid()` is its interior counterpart, mapping a grid of normalized coordinates in `[-1, 1]` onto points that fill the aperture. This is the primitive needed to sample a stop's interior with a grid of rays -- rather than inferring the field of view from the boundary extremes -- e.g. to robustly center a `SequentialSystem`'s field/pupil sampling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0157xmSAJzcePsXEpaHmMhY4
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #180 +/- ##
==========================================
- Coverage 99.34% 99.31% -0.03%
==========================================
Files 116 116
Lines 5983 6019 +36
==========================================
+ Hits 5944 5978 +34
- Misses 39 41 +2
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:
|
Adds test_grid_transformation exercising grid() on a transformed RectangularAperture and CircularSectorAperture, covering the two lines codecov flagged. Co-Authored-By: Claude Opus 4.8 <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
Adds a
grid()method toRectangularApertureandCircularSectorAperture— the interior counterpart towire()(which samples the boundary). It maps a grid of normalized coordinates in[-1, 1]onto points that fill the aperture, withtransformationrespected. This is the building block for sampling a stop's interior with a grid of rays instead of inferring its extent from the boundary.This is intentionally a small, self-contained primitive PR. The motivation — and the follow-up that consumes it — is below.
Motivation: the field/pupil sampling is fragile
SequentialSystem._denormalize_gridmaps the normalized[-1, 1]field/pupil grid onto each stop's field of view by tracing the stop perimeters (wire()) out to object space, taking the min/max to form a bounding box, and placing the element "center" at(min+max)/2:(min+max)/2is a 2-point estimator of the center: it hangs off two extreme rays, which are aberration-smeared marginal rays at the aperture corners. The corner that achieves the extreme hops as the element scans the field, so the assigned center jumps discontinuously.In a multi-slit spectrograph (MUSE), this puts a spurious ~0.3 mÅ kink into the slit-to-slit separation near specific slits — about ⅓ of the science requirement — that is not physical (it's wavelength- and grating-independent and doesn't shrink with denser
[-1, 1]sampling).The fix this enables
Replace the 2-point estimator with an N-point average: sample the stop interiors with a grid (this PR's
grid()), use the same secant/Newton solver already in_calc_rayfunction_stops_onlyto find the ray through each (field-point, pupil-point) pair, and let the center fall out as the centroid of the grid (= the chief ray through the element center, smooth by construction). Averaging over the whole aperture is insensitive to which corner is extreme.A prototype that swaps
wire → gridon the existing solver and takes the centroid drops the per-slit center error from 4.01 maas → 0.00 maas (the kink vanishes entirely, not just shrinks). The machinery is already there —_calc_rayfunction_stopsalready back-propagates to object space — so the consumer is a focused change; I've kept it out of this PR because routing the default rayfunction through it is a behavior change worth its own review (and touches value-asserting system tests).Credit to Roy Smart for the grid-sampling framing.
Tests
test_rectangular_aperture_gridandtest_circular_sector_aperture_grid(grid points fill the interior). Fulloptika/apertures/_apertures_test.pypasses (20486 tests),ruffclean.🤖 Generated with Claude Code