Fixed spurious Max iterations exceeded errors in the stop root-finding problem#168
Open
jacobdparker wants to merge 1 commit into
Open
Fixed spurious Max iterations exceeded errors in the stop root-finding problem#168jacobdparker wants to merge 1 commit into
Max iterations exceeded errors in the stop root-finding problem#168jacobdparker wants to merge 1 commit into
Conversation
…ing problem The Newton solve in `_calc_rayfunction_stops_only` used the default perturbation of `na.jacobian`, an absolute 1e-10 in the units of the variable, which is below the floating-point noise floor of the raytrace for variables measured in physical units, yielding a Jacobian made of noise and wild iterates. The default convergence tolerance of `na.optimize.root_newton` (1e-10 in the units of the residual) had the same scale-dependence problem: solves whose residuals had converged to machine precision were reported as "Max iterations exceeded". Both are now proportional to the size of the target aperture wire. Failures that remain are re-raised with the names of the stop surfaces involved and a hint about marking the object surface as the field stop for dispersive systems, instead of an anonymous "Max iterations exceeded" from deep inside named_arrays. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 11, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #168 +/- ##
==========================================
- Coverage 99.34% 99.29% -0.05%
==========================================
Files 116 116
Lines 5967 5978 +11
==========================================
+ Hits 5928 5936 +8
- 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:
|
roytsmart
reviewed
Jun 11, 2026
| # the size of the target aperture to be achievable in floating | ||
| # point for systems of any physical scale. | ||
| scale = np.maximum( | ||
| np.abs(grid_last.x).max(), |
Collaborator
There was a problem hiding this comment.
Shouldn't you use ptp() here instead of max?
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
2 participants
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.
First of a stack of three PRs fixing the normalized-coordinate stop solver for non-all-mirror systems (next: object-as-field-stop support, then transmissive stops). These bugs currently block the FZP tutorial (#153), the grazing-spectrograph tutorial (#166), and the FURST
Spectrographmodel (Kankelborg-Group/furst-optics#25).Problem
The Newton solve in
_calc_rayfunction_stops_onlyrelied on two scale-dependent defaults:na.jacobian's default perturbation is an absolute 1e-10 in the units of the variable. For variables measured in physical units (e.g. positions in mm), that is below the floating-point noise floor of the raytrace, so the Jacobian is computed from noise and the iterates step wildly.na.optimize.root_newton's default tolerance is 1e-10 in the units of the residual — 0.1 femtometers for mm residuals. Solves that had converged to machine precision (~1e-14 of the system scale) were reported asMax iterations exceededbecause a few elements oscillate in float noise just above the threshold.Changes
Max iterations exceededfrom inside named_arrays.No behavior change for systems that already converged; the existing test battery passes unchanged. Regression tests for the newly-solvable systems come with the next two PRs in the stack.
🤖 Generated with Claude Code