Repository navigation
feat: Bragg disk detection .single runs on every detector size - #61
Merged
Merged
Conversation
The Float32 path built FloatFFT2D(rows:columns:), which is nil for lengths vDSP DFT cannot take, so .single refused them with invalidOption. It now builds FloatFFT2D(anyRows:columns:): the same vDSP calls and bits on sizes vDSP takes, Bluestein on the others. BraggDetector.supportsSinglePrecision is true for every positive size; docs follow. Tests: nonVDSPSizesAgreeWithDouble (22x21, 165x40, 40x56; map 4.3e-7 and Fourier 4.0e-7 of max against 1e-5 and 3e-5; peaks within 2e-4 px and 2e-5; whole-cube equals per-pattern), everySizeIsSupportedAndDoubleStaysDefault, and the S2 fixture (40x56) now in singleAgreesWithDouble and singleMatchesPy4DSTEM at the strict tier. Red before (invalidOption), green after. Done: .single Bragg detection on any size (CuboKit#60). Next: CI and the consumer check on the PR; a docs catch-up can drop the open-items mention of the refusal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paullo9
added a commit
that referenced
this pull request
Nov 1, 2026
…ctor size (#69) Since #61 the Float32 Bragg path builds FloatFFT2D(anyRows:columns:), so .single runs on every positive detector size and supportsSinglePrecision is true for all of them. The consumer check still expected a 40 x 56 .single run to throw invalidOption, so it failed 1 of 152 on main and on the Release PR. The check now runs .single and .double on a 22 x 21 detector (a size vDSP DFT cannot take, FloatFFT2D.supports is false) with one planted disk: both find one peak near it and agree within 1e-3 px. supportsSinglePrecision also covers 22 x 21 (true) and a negative column count (false), next to the existing zero-row case. No kit source or number changes. consumer-check: 152 of 152 pass; the fast gate is green. Done: the consumer check matches the #61 behaviour. Next: the Release PR CI goes green after this merges. 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 #60
What changes
SingleState(the.singleBragg path) buildsFloatFFT2D(anyRows:columns:)instead ofFloatFFT2D(rows:columns:). On sizes vDSP DFT takes, that is the same calls and the same bits. On any other size it runs Bluestein through a longer vDSP DFT. Before this change.singlethrewinvalidOptionon those sizes.BraggDetector.supportsSinglePrecisionnow returns true for every positive size. The signature is unchanged and no consumer has to change code, so this isfeat:, notfeat!.BraggPrecision.singledoc comment, the two DocC pages,docs/bragg.md(Sizes) and the consumer-check follow.FloatFFT2D(anyRows:columns:)is already on main. This PR does not touch the files of docs: CPU mean no longer behind NumPy, close CuboKit#7 in open items #58 or feat: single-slice ptychography .single takes every detector size #59.Tests (red first)
nonVDSPSizesAgreeWithDoublecovers 22 x 21, 165 x 40 and 40 x 56, with and without the pattern Gaussian, under.parabolicand.multicorr. Results:SingleTolerance.correlation, 1e-5).SingleTolerance.fourier, 3e-5)..singlepass equals the per-pattern pass.singleAgreesWithDoubleandsingleMatchesPy4DSTEMat the strict tier (5e-5 px, 5e-6 on peaks above 1e-3 of the brightest). The minimum case counts rose to 39 and 29 so that S2 has to stay in.everySizeIsSupportedAndDoubleStaysDefaultreplaces the old refusal test.invalidOption("22 × 21 is not one"); after the fix they pass.swift test -c release --parallel --num-workers 1 -j 1 --skip CuboKitParityTestspasses all 429 tests in 8 targets. The CuboBragg tests run in 0.13 s, the slowest target in 0.62 s, and the incremental build took 8 s.tools/consumer-check/run.sh) was not run locally because it builds with 2 jobs. CI runs it.Review
A reviewer read the diff before the commit because it touches numerics. It found no correctness problem: every
.singleentry goes throughSingleState, the Bluestein forward and inverse are right, the transposed layout andnaturalSpectrumstill work, and sharing a Bluestein line across workers is safe. Two stale comments and the peak bound were fixed after the review.For a later docs catch-up
docs/open-items.mdis in two open PRs, so this PR leaves it alone. Any line there that says.singleBragg refuses non-vDSP sizes is now resolved by this PR.🤖 Generated with Claude Code