Repository navigation
fix: the energy axis refinement stops at an offset and gain bound and never reports converged there - #84
Merged
Conversation
…reports converged there A spectrum with one strong line fixes two numbers (its channel and its width in channels) against three refined parameters (offset, gain, FWHM). On a synthetic one-line spectrum (Al K-alpha, Mg and Si at 0, 4322 counts) the residual changes by under 5 % from gain 0 to +444 % along that valley, so the simplex walked: to gain +444 %, offset -6.59 keV and the 500 eV FWHM bound under the broadened edge step, to gain +32.7 %, offset -484 eV under the sharp one, both reported converged. Holding the continuum on the file axis left the broadened result bit-identical, so the edge step following the axis is not the cause. AxisRefinementSettings gains maximumOffsetShiftEV (200) and maximumGainShift (0.05); a trial outside is rejected as the width bounds are. A refinement that ends within 1e-4 of a bound span of any bound is not converged and names the parameter (AxisRefinementResult.parametersAtBound); the pooled fit then uses the file axis and width, with a warning and the footer clause NOT used (stopped at its bound: ...). A capped refinement that is used now says it did not converge. Stored policies without the new keys read the defaults. Every golden and parity case is bit-identical (they gain two empty lines for the new fields). Done: SpectroKit#83 fixed with red-first tests. Next: Paul reviews the before/after table; mac4DSTEM re-records its demo golden on 0.13.2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AxisRefinementResult gained parametersAtBound and boundText, so every golden with an unlocked refinement gains two lines (an empty list and an empty string). No existing line changed: every value is bit-identical. Re-recorded with SPECTRO_GOLDEN_DIR and testRecordBaseline. Done: goldens re-recorded for the new fields. Next: none. Co-Authored-By: Claude Opus 5.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.
Requested by Paul · project thread
Fixes #83
Numbers move. This PR waits for Paul (auto-merge off). Ships as 0.13.2.
Cause
A spectrum with one strong line fixes two numbers: the line's channel and its width in channels. The refinement has three parameters (offset, gain, FWHM), so they can trade along a valley. On the reported synthetic spectrum (Al K-alpha strong, Mg and Si planted weak and fitted at 0, 4322 counts in all), I held Al K-alpha at channel 148.6 and 9 channels wide. Along that valley the residual changes by under 5 % between gain 0 and gain +444 % (test
oneLineValleyIsFlat). Under the broadened step it keeps falling all the way to the 500 eV width bound. The edge step only decides where on the valley the simplex stops:The hypothesis that failed: "the edge step follows the axis and the FWHM, so there is a feedback". I tested it first. I held the continuum (edge positions and step width) on the file axis and file width during refinement. The broadened result came out bit-identical, so that feedback is not the cause. This would have refuted the valley diagnosis instead: a spectrum with two or more strong, well-separated lines running away. The 384 planted refinements of AxisGateTests E3 (lines spanning 1.25-1.84 keV) all stay converged, with gain at most 0.42 %.
Fix
AxisRefinementSettingsgets two new bounds,maximumOffsetShiftEV(default 200) andmaximumGainShift(default 0.05). A trial outside either is rejected, the same way the existing width bounds work. Inside the bounds the objective is unchanged, so any run that never crosses a bound takes exactly the same simplex path as before.AxisRefinementResult.parametersAtBound(public) and in a text clause.PooledQuantifieruses the file axis and the file width:FitPolicy.problemnow refuses an offset or gain bound that is not finite and positive.ProposalAxisRefinementskips a refinement that stopped at a bound.Before / after (the reported synthetic spectrum, 200 kV, Al + Mg + Si, axis unlocked)
The reference is the file axis: offset 0, gain 0. The lines were planted on it. There is no clean truth for the net counts: the generator truncates each pixel's channel value to an integer before summing, which removes most of the planted counts (about 10,500 Al K-alpha counts were planted before truncation).
After the fix, both steps give exactly the numbers of the locked-axis fit (the test asserts equality). An app that pins a golden of this spectrum will see these numbers move when it takes 0.13.2.
Tests (red first)
AxisRefinementRunawayTests:QuantificationPolicyTests.testAxisRefinementOptionsadds four checks:parametersAtBound == ["FWHM"];swift test -c release --parallel --skip SpectroKitParityTests): green; the 4 known issues were there before.--strictand SwiftFormat--lint: clean.🤖 Generated with Claude Code