[ESSSANS] Apply the SANS beam center as a beam direction - #729
Draft
SimonHeybrock wants to merge 4 commits into
Draft
[ESSSANS] Apply the SANS beam center as a beam direction#729SimonHeybrock wants to merge 4 commits into
SimonHeybrock wants to merge 4 commits into
Conversation
The center-of-mass beam-center finder projected the center-of-mass position onto the plane normal to the incident beam without first subtracting the sample position. The resulting offset was therefore measured from the origin of the lab coordinate system rather than from the beam axis through the sample. This was invisible so far because all supported instruments place the sample on the beam axis at the origin.
… positions The beam center describes where the beam is, not where the detector is. Applying it as a detector position offset made it leak into everything derived from the pixel positions, in particular the solid angle, which is a property of the real geometry and must not depend on the beam. Callers also had to undo the shift to recover the real positions, see the SANS2D masks. Overriding `scattered_beam` in the coordinate transformation graph confines the beam center to the quantities that are actually measured relative to the beam: two_theta, phi and the cylindrical coordinates. Those are unchanged bit for bit; the solid angle, and hence I(Q), changes by up to 0.04% (0.2% for I(Qx,Qy)) for the Loki-at-Larmor reference data, so the reference results are regenerated. The beam center no longer needs to be added back to the result of the center-of-mass finder, since the pixel positions it works on are no longer shifted.
The beam center was applied as the same transverse shift to every pixel. On an instrument with banks at different distances, such as Loki, that is wrong: the beam center is set by the collimation, so it describes a beam direction, and a given angular deviation shows up as a proportionally smaller transverse offset on a bank closer to the sample. Applying the offset determined on the rear bank unchanged to the window frames overcorrects them by up to a factor of five. BeamCenter therefore becomes the position of the beam center relative to the sample: the transverse offset together with the distance at which it was determined. That distance is what turns an offset into a direction, so it has to be part of the value; a beam center given as an offset alone is now rejected rather than silently misapplied. A zero vector still means "no correction". The finders report the distance they used, so values obtained from them need no change beyond being carried around whole. Every pixel is then shifted in proportion to its own distance from the sample, which also covers the depth of a bank without any special treatment. Note that the distance reported by the center-of-mass finder is the intensity-weighted mean depth of the bank rather than its geometric center; see the docstring for why that is consistent, and for the limits of resolving anything within a bank. For the Loki-at-Larmor reference data the results move by less than 0.3 sigma of their own statistical uncertainty in almost every bin, so the reference results are regenerated.
SimonHeybrock
force-pushed
the
404-beam-center-direction
branch
from
August 26, 2026 08:05
4a18f07 to
ca6a774
Compare
The beam center is a point on the actual beam, measured from the sample, so it defines the beam direction. Passing that direction as `incident_beam` says so directly. It replaces the shear, which mapped the actual beam onto the nominal axis by displacing every pixel's scattered beam in proportion to its distance from the sample. The shear is exact along the beam and first order elsewhere, since it is a shear rather than a rotation. For the Loki-at-Larmor detector that costs a relative error in two_theta with a median of 3.8e-4 and a maximum of 1.2e-3, well above the 1e-5 the docstring claimed. Rotating the incident beam is exact everywhere, and it puts phi, the cylindrical coordinates and two_theta on the same footing instead of relying on the shear having mapped the transverse plane closely enough. The reference results move accordingly and are regenerated. Requires the beam-aligned coordinate system to follow the incident beam rather than its projection onto the horizontal plane; see scipp/scippneutron#725. Until that is released, phi and Qx/Qy would drop the vertical part of the beam center.
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.
Fixes #404.
Why
The beam center was applied as a fixed transverse shift of every detector pixel. That is wrong on an instrument with banks at different distances, which is what #404 is about, and it also leaked into places that have nothing to do with the beam.
The information in #404 is largely superseded. The current understanding is that the rear-detector rail calibration at Loki will be embedded in the positions in the file, that the sample is where it claims to be, and that the beam center therefore describes the direction of the beam, set by the collimation slits. A given angular deviation of the beam shows up as a proportionally smaller transverse offset on a bank closer to the sample, so a single offset cannot be applied unchanged to all banks.
What changed
Three independent steps, one commit each.
fix(sans): measure beam center relative to sample position— the center-of-mass finder projected the center-of-mass onto the plane normal to the beam without first subtracting the sample position, so the offset was measured from the origin of the lab frame rather than from the beam axis through the sample. Latent only: every currently supported instrument puts the sample on the beam axis at the origin, and the results are bit-for-bit unchanged on the Loki-at-Larmor and SANS2D test data. Fixed because the next commit makes the sample position load-bearing.refactor(sans): apply beam center to the scattered beam, not to pixel positions— the beam center describes where the beam is, not where the detector is, so it now overridesscattered_beamin the coordinate transformation graph instead of being turned into aDetectorPositionOffset. The pixel positions keep the values from the file. Callers no longer have to undo the shift to recover the real geometry, which removes theraw_pos = position + beam_centerworkarounds in the SANS2D masks.two_theta,phi,Qand the wavelength are unchanged bit for bit; I verified this by patching the pre-refactor code so that only its solid angle sees unshifted positions, which reproduces the new results exactly. The one thing that does change is the solid angle, which was computed from beam-center-shifted positions and so carried a smooth spurious tilt across every bank. Solid angle is a property of the real geometry and must not depend on the beam. On the real Loki geometry the size of that artefact is:loki_detector_0loki_detector_1-4loki_detector_5-8It is a tilt rather than scatter: each bank is a straight line through zero when plotted against the pixel position projected on the beam-center direction, with a slope set by its distance from the sample, so it biases I(Q) systematically rather than averaging out. The knock-on effect on the single-bank Loki-at-Larmor test data is only 0.041% in I(Q) and 0.22% in I(Qx,Qy), which is why the reference results move at all.
feat(sans): treat the beam center as a beam direction— every pixel is now shifted in proportion to its own distance from the sample, which maps the actual beam onto the nominal axis. This is the actual fix for #404, and it covers the depth of a bank without any special treatment.refactor(sans): apply the beam center as an incident beam direction— the shear above is replaced by pointingincident_beamalong the beam center, which is what a beam center is. Exact everywhere rather than exact along the beam and first order elsewhere, and it deletes the shear provider.This requires scipp/scippneutron#725 (stacked on scipp/scippneutron#724), which makes the beam-aligned coordinate system follow the incident beam instead of its projection onto the horizontal plane. Without it,
phiandQx/Qysilently drop the vertical component of the beam center. This PR therefore cannot be merged until that is released.For the real Loki geometry (
geometry-loki-2026-04-13.nxs, sample at the origin), taking a 34.3 mm offset determined on the rear bank:loki_detector_0(rear)loki_detector_1loki_detector_2loki_detector_3loki_detector_4loki_detector_5loki_detector_6loki_detector_7loki_detector_8The front window frames were over-corrected by more than a factor of four. Worth noting for the review: the banks are much deeper than one might assume, up to 327 mm for
loki_detector_5, which is 27% of its own distance from the sample. Doing the correction per pixel rather than per bank costs nothing here and removes the question entirely.How wrong was that? Geometry alone answers it, using the real pixel positions and comparing the scattering angle each pixel gets under the two treatments:
loki_detector_0(rear)loki_detector_1loki_detector_2loki_detector_3loki_detector_4loki_detector_5loki_detector_6loki_detector_7loki_detector_8A systematic, bank-wide 1.3% to 2.2% error in Q on the window frames, reaching 5.7% in the tail, against 0.02% on the bank the beam center was measured on. That is the size of the bug.
On the single-bank Loki-at-Larmor test data there is, correctly, almost nothing to see: with one bank the correction changes by at most 0.58 mm, far below the pixel size. What little change appears is binning jitter, pixels crossing Q-bin boundaries, rather than a shift of the curve. Rebinning shows this directly:
A smooth systematic survives rebinning; this collapses as the bins coarsen while the mean stays at zero, and adjacent bins disagree in sign more often than chance, which is what one bin losing what its neighbour gains looks like. So for existing single-bank reductions this commit is a no-op to within the noise, and the reference results move by 0.14 sigma median and 0.63 sigma at most.
The Loki-at-Larmor reference results are regenerated in the two commits that change them.
User-facing change
BeamCenteris now the position of the beam center relative to the sample,sc.vector([x, y, L_ref], unit='m'): the transverse offset together with the distance from the sample at which it was determined. That distance is what turns an offset into a direction, so it has to be part of the value.Only the direction enters the correction; the magnitude cancels. A value determined on one bank therefore applies to all banks, and, if the beam direction really is set by the collimation alone, also to a detector moved to a different distance along its rail — which was not true before, since the old code would have applied the wrong offset. Conversely the value is tied to the collimation that produced it, so it has to be re-determined when the slits change. Nothing in the files records which collimation a given value came from, so that one is documented rather than checked.
A zero vector still means "no correction", so existing
wf[BeamCenter] = sc.vector([0, 0, 0], unit='m')keeps working unchanged. A non-zero offset with no distance is rejected with a message explaining what is missing, rather than being silently misapplied as before. Both beam-center finders report the distance they used, so values obtained from them need no change beyond being carried around whole. The workflow GUI parameter moves fromVector2dParameterto the existingVector3dParameter.Accuracy and known limitation
The correction is now a rotation of the incident beam, so it is exact for every pixel and there is no approximation left to quote.
Earlier revisions of this PR applied it as a shear of the scattered beam, on the estimate that the resulting error in
two_thetastayed below 1e-5. That estimate was too optimistic: measured across the Loki-at-Larmor detector, with the beam center the finder returns, the relative error intwo_thetahas a median of 3.8e-4 and a maximum of 1.2e-3. It grows for pixels close to the sample, which the single-pixel estimate happened to miss. That is what moves the reference results in the last commit.The distance reported by the center-of-mass finder is the intensity-weighted mean depth of the bank, not its geometric center, because layers closer to the sample see more counts. That is consistent rather than wrong: the transverse position of the beam is linear in the distance from the sample, so any intensity-weighted average of points on the beam is again a point on the beam, and the reported distance says where it ended up. The premise holds only approximately, though, and on Loki-at-Larmor the per-layer estimates disagree by more than the geometry allows, so the uncertainty on the distance is of the order of the depth of the bank itself. That is irrelevant for extrapolating to other banks, a few centimetres out of several metres, but it means the variation of the correction within one bank is not resolved better than the noise on it. This is written up in the docstring of
beam_center_from_center_of_mass.Test plan
New
tests/beam_center_test.pycovers how the correction is applied: zero scattering angle at the beam center and everywhere else along the beam,phimeasured around the beam rather than around the nominal axis (both horizontally and vertically), correct behaviour with an off-axis sample and with gravity, and the rejection of a beam center without a distance. Newtests/beam_center_finder_test.pycovers what the finders return, on small synthetic detectors that run in under a second.tests/loki/workflow_test.pygains the two workflow-level guarantees that the beam center moves neither the pixel positions nor the solid angle; both fail on the previous code.The I(Q)-based finder is only exercised by tests that are currently skipped as too slow, so I ran it by hand on the SANS2D tutorial data: it reproduces the verified Mantid result to within the 2e-3 the skipped test asserts. The
beam-center-findernotebook needed updating, since the internal_iofq_in_quadrantsnow takes the plane in which the offsets are given, and I executed all of its cells to confirm.Note for anyone running the tests locally: if
esssansis installed non-editable, pytest will silently pick up the installed copy rather than the working tree.