perf: pad inner FFT to a smooth size to remove sampling-frequency regression#78
Merged
Conversation
…ression The inner double-block correlation FFT ran at exactly 2*block_size, a size fixed by the sampling frequency (block_size = samples_per_code / num_blocks). When samples_per_code carries a large prime factor the inner FFT lands on a slow FFTW radix — e.g. 16.368 MHz (= 16 x 1.023 MHz, the canonical GNSS IF) gives block_size 1023 -> 2046 = 2*3*11*31, a radix-31 transform ~5x slower than a nearby smooth size. plan_acquire now zero-pads the inner FFT up to the next 2*3*5*7-smooth length (fft_size = nextprod((2,3,5,7), 2*block_size)), the same max_prime=7 "fast regime" recommend_sampling_freqs already uses. This is a no-op when the natural size is already smooth. Results are unchanged: any length >= 2*block_size preserves the first block_size linear-correlation lags, and the inverse FFT is normalised by the actual length, so the recovered correlation is mathematically identical. Verified end-to-end at 16.368/8.184 MHz vs the unpadded path: power surface matches to <1e-6 relative (Float32 rounding floor), Doppler and code phase bit-identical. Measured acquire!(1:32): 16.368 MHz 20.1 ms -> 4.2 ms, matching the smooth 16.384 MHz rate. Column-FFT-bound rates (large prime in num_blocks, e.g. 1.542 MHz -> 257) are unaffected: a DFT cannot be zero-padded losslessly. Full test suite passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #78 +/- ##
=======================================
Coverage 97.17% 97.18%
=======================================
Files 12 12
Lines 1170 1172 +2
=======================================
+ Hits 1137 1139 +2
Misses 33 33 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Inner-FFT padding removes the inner-FFT half of the sampling-frequency regression, so the old guidance (prefer a smooth samples_per_code for the inner FFT, the 16.368 MHz radix-31 "offender", the Fast/Slow inner-FFT timing table) is obsolete. Rewrite the "Sampling Frequency and FFT Performance" section to explain that the inner FFT is now auto-padded (bit-identical results) and that the remaining sensitivity is the column FFT (num_blocks / num_doppler_bins with a large prime, which a DFT cannot be zero-padded to avoid, e.g. 1.542 MHz -> 257). Also update the two spots that referenced the old framing: the index.md tip (16.368 MHz is no longer slow) and the num_blocks section's now-moot "second-order effect" paragraph. recommend_sampling_freqs docs are unchanged (still useful for the column FFT). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
The existing suite only benchmarks smooth sampling rates (2.048/5/10/15/16/25/12 MHz — all powers of two or 2^a*5^b), where inner-FFT padding is a no-op, so it neither shows the padding speedup nor guards its fast path. Add GPS L1CA at 16.368 MHz (samples_per_code 16368 = 2^4*3*11*31 → inner FFT lands on a large prime), which AirspeedVelocity runs against both master (slow, unpadded radix FFT) and this branch (padded to a 2*3*5*7-smooth size). Guards the regression fix and makes the improvement visible. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The rewrite dropped the old Fast/Slow timing table (it measured the now-fixed inner FFT). Add a small table quantifying the penalty that is still real after padding — a large prime in num_blocks / num_doppler_bins, which the column FFT cannot be zero-padded to avoid. Measured GPS L1CA, 32 PRNs: 1.542 MHz (num_doppler_bins = 257) is ~35x slower than the adjacent 1.500 MHz, and 3.069 MHz (31) ~3x slower than 3.000 MHz. Notes that rates slow only because of the inner FFT (e.g. 6.138, 16.368 MHz) are no longer affected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Since the inner double-block FFT is now zero-padded to a 2*3*5*7-smooth size, its factorization (and samples_per_code's) no longer affects speed — only the column FFT (num_doppler_bins), a DFT that cannot be padded, does. recommend_sampling_freqs was still filtering and ranking by inner-FFT / samples_per_code smoothness, which made it wrongly reject now-fast rates (e.g. 16.368 MHz: non-smooth samples_per_code 16368 = 2^4*3*11*31, but num_doppler_bins = 16). Changes: - Gate candidates only on num_doppler_bins max-prime (drop the samples_per_code and inner-FFT gates). - Report inner_fft_size as the padded length actually run (nextprod((2,3,5,7), 2*block_size)); inner_max_prime is therefore <= 7 by construction. - Rank :cost and :smoothness by num_doppler_bins_max_prime. - Cost model uses the padded inner size. 16.368 MHz is now recommended (num_doppler_bins = 16); 1.542 MHz (forced to num_blocks = 257) is still rejected. Tests updated + a new testset covering both; full suite passes. Co-Authored-By: Claude Opus 4.8 (1M context) <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
1 participant
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.
Summary
Removes the sampling-frequency performance regression documented in Sampling Frequency and FFT Performance, by zero-padding the inner double-block correlation FFT up to the next FFTW-fast (
2·3·5·7-smooth) length.The inner FFT ran at exactly
2 × block_size, a size fixed by the sampling frequency (block_size = samples_per_code ÷ num_blocks). Whensamples_per_codecarries a large prime factor, that lands on a slow FFTW radix. The flagship case is 16.368 MHz (= 16 × 1.023 MHz, the canonical GNSS IF):block_size 1023 → 2046 = 2·3·11·31, a radix-31 transform ~5× slower than a nearby smooth size.plan_acquirenow setsfft_size = nextprod((2, 3, 5, 7), 2 × block_size)— the samemax_prime = 7"fast regime" thatrecommend_sampling_freqsalready uses. No-op when the natural size is already smooth.Results are unchanged
Any length
≥ 2 × block_sizepreserves the firstblock_sizelinear-correlation lags (no aliasing into the zero tail), and the inverse FFT is normalised by the actual length, so the recovered correlation is mathematically identical. Verified end-to-end at 16.368 / 8.184 MHz, padded vs. forced-unpadded, on the same input:< 1e-6relative (Float32 rounding floor)~2e-6dBMeasured speedup
acquire!(GPSL1CA(), signal, 1:32), 24 threads:Padded 16.368 MHz (4.2 ms) now matches the smooth 16.384 MHz rate.
Scope / limitations
Padding fixes only the inner FFT. A large prime in
num_blocks(hencenum_doppler_bins) still slows the column FFT — a genuine DFT over the Doppler axis that cannot be zero-padded without changing its bins (e.g. 1.542 MHz →num_blocks = 257). Those rates still benefit fromrecommend_sampling_freqs. The doc section is updated to reflect this.Testing
🤖 Generated with Claude Code