Repository navigation
TPC: fix the weight and grid scaling of the spline derivatives in addCorrections - #15994
Merged
alcaliva merged 2 commits intoOct 21, 2026
Merged
Conversation
…ections TPCFastSpaceChargeCorrectionHelper::addCorrections scaled the 4th parameter of each spline knot (the mixed derivative d2S/dudv) with the square of the weight: secMainScale * secMainScale for the main correction, ls * ls * ... for the added ones (ls is the weight times the z fade-out factor, 1 at the knots). A spline is linear in its parameters, so scaling the correction by w scales all of them by w. The correction was therefore right at the knots but wrong between them whenever a weight != 1 was used, e.g. the derivative map added with weight lumiScale in TPCScalerSpec::buildMap. The error is proportional to w - w^2. Measured with a production map and the derivative map of run 568041 (random points, vs the weighted point-by-point sum of getCorrectionLocal and of the inverse getters), for map + 0.13 x derivative: rms ~20 um in dx and dz, ~10 um without the first 10 pad rows; deviations above 100 um are rare (0.4 % of the points) and lie mostly in the first pad rows. At a weight of -0.5 the rms is ~150 um (~60 um without the first 10 pad rows). With the fix the sum is exact to float precision (< 0.1 um) for weights from -1 to 1.5, inverse corrections included; w = 1 is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A correction added on a different spline grid is sampled at the main grid's knots, and its derivatives must be converted with the
chain rule: with u = (y - y0) * yScale, d/du_main = (yScale_corr / yScale_main) d/du_corr, the same for v. addCorrections used the
inverse ratios, and applied the u factor to the dS/dv parameter and the v factor to dS/du (knot parameters {S, dS/dv, dS/du,
d2S/dudv}).
For identical grids (e.g. the derivative map) the factors are 1 and nothing changes. The M-shape map is created on a 4 x 4 grid in
TPCScalerSpec, with factors of 35-98 with respect to a production map: adding the M-shape map of run 544797 (up to 6 mm) to a map of
run 568041 gave errors of up to 89 cm in dx and up to the kMaxCorrection limit of 100 cm in dy (rms 4 cm and 7 cm), vs the
point-by-point sum. With the fix: rms 1-2 um, max 60 um (resampling of non-nested grids) inside the main map's grid. Outside of it
(|z| > ~1.37 x for production voxel maps) the merged map follows the edge and z fade-out of the main grid, as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
matthias-kleiner
requested review from
davidrohr,
shahor02 and
wiechula
as code owners
October 21, 2026 14:39
alcaliva
enabled auto-merge (squash)
October 21, 2026 18:54
alcaliva
approved these changes
Oct 21, 2026
wiechula
approved these changes
Oct 21, 2026
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
3 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.
No description provided.