Process each CPI in two overlapped stages - #62
Closed
Purple10101 wants to merge 1 commit into
Closed
Purple10101 wants to merge 1 commit into
Purple10101 wants to merge 1 commit into
Conversation
The radar loop ran every stage of a CPI back to back, so throughput was the sum of the stages: 828 ms on owl-ded9 against a 500 ms CPI, and 688 ms on fairforest-b. Splitting the loop at the clutter filter makes throughput the slowest stage instead. The clutter filter holds no state across CPIs, so the split itself needs no numerical argument. Slots carry one CPI's channels and timing from the front stage to the back, two of them so the stages overlap fully, and everything that does hold state across CPIs (the tracker) stays inside the back stage, which processes CPIs one at a time in capture order. Three details the split has to get right: - The spectrum analyser moves from before the clutter filter to after it, so the slow half is extract plus filter rather than extract plus spectrum plus filter. It reads the reference channel, which the filter only reads, so its output does not change. TestStageOrder proves that on a signal the filter cancels by 58.8 dB, rather than by reading the source. - A live retune is latched against the CPI it arrives on instead of acted on where it is seen, because the tracker it resets now runs in the other stage. A CPI dropped by a failed filter hands the retune back rather than eating it. - FFTW bakes the thread count into the plan, so the stages plan with 2 threads each rather than both claiming all 4. The clutter transforms prefer 2 to 4 anyway (118.2 ms against 146.8 per CPI at 1e6 points), so the front stage gains; ambiguity gives up about 21 ms. Concurrent threaded executes on distinct plans were checked for deadlock and for identical results first. That last one is the only part that is not bit-identical: FFTW parallelises across a Cooley-Tukey factor, so a different thread count sums in a different order. Measured on aarch64, the disagreement is 2.4e-16 of peak at 1e6 points and 6.4e-21 at 200k, against the 6e-14 the FFT length work already accepted. On x86 the two thread counts agree exactly. TestStageOrder holds the bound. "cpi" still reports the work done on a CPI across both stages, excluding the queue wait, so it stays the quantity the serial loop reported. Throughput is now a separate "cpi_interval", and "pipeline_wait" says which stage is the bottleneck. Tests compile and pass on aarch64 as well as x86. Running the equivalence suite on ARM is what caught that IqData serialises its never-assigned min, max and mean, so whole-JSON comparison of two IqData objects is allocator luck; the test compares the spectrum and frequency arrays instead. That bug is pre-existing and still unfixed. Not yet measured on hardware. Expected throughput is the front stage: about 497 ms on owl-ded9 and 412 ms on fairforest-b, against 828 and 688 serial. That is not a reliable 500 ms on owl-ded9, whose clutter filter swings 13.6% between identical runs, so it needs the clutter filter work to follow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
|
Closing for now. The work is not lost: the branch stays on the remote and the measurements are recorded in the PR description above. Reopen or re-raise when there is a decision on how to take this forward. |
1 similar comment
Author
|
Closing for now. The work is not lost: the branch stays on the remote and the measurements are recorded in the PR description above. Reopen or re-raise when there is a decision on how to take this forward. |
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.
Stacked on #61, which is stacked on #60. Review those first; this PR shows only its own commit.
Splits the radar loop at the clutter filter so throughput becomes the slowest stage instead of the sum of the stages.
Full A-B-A against v0.4.3, run 2026-09-15, 240 s measurement window per phase, throughput taken from
nCpiadvance rather than polling:(ms per CPI.) Baseline drift A1 vs A2 was 15.1 ms and 2.3 ms, so both controls are tight.
fairforest-b's 1.52x is a floor, not the real figure. It landed on exactly 500.0 ms/CPI (480 CPIs in 240.0 s), which is the CPI duration itself. That node is no longer compute-limited but data-limited: it is keeping up with its receiver in real time and cannot go faster. The true compute headroom is larger but unobservable from throughput.
Two things reviewers will trip on
cpigoes UP and that is correct. It measures the latency of one CPI through the stages, and with two stages overlapped each CPI takes longer to pass through while more of them finish per second. On jn1 it read 1185 -> 1382 ms while throughput went 904 -> 561. The commit addscpi_intervalfor throughput andpipeline_wait(near zero, so the front stage is the bottleneck). Judging this change bycpisays it regressed.Continuous polling distorts the measurement. A tight
/api/timingloop costs ~200 ms/CPI on a saturated node: 762 ms/CPI polled against 561 ms clean, reproduced twice. All figures above are fromnCpiadvance with negligible polling.This is the only non-bit-identical commit in the stack
FFTW bakes the thread count into the plan, so the two stages plan with 2 threads each rather than both claiming 4. The clutter transforms prefer 2 to 4 anyway (118.2 ms against 146.8 per CPI at 1e6 points), so the front stage gains; ambiguity gives up. A different thread count sums in a different order, so results differ by 2.4e-16 of peak on aarch64 against the 6e-14 the FFT length work already accepts. On x86 the two thread counts agree exactly.
Everything else in the stack is byte- or value-identical.
Known issues
TestStageOrderfails in a bare container. It writes its FFT-length cache to/opt/blah2/save, and where that is not writable each filter instance re-measures and lands on a different length, so two instances cannot agree bit-for-bit. With a writable cache it passes, and production has one filter instance and a working host mount. The test should pinBLAH2_FFT_CACHEitself, andfastestFftLengthshould memoise in-process. Neither is done here.testjob is a false green (see Choose the clutter convolution FFT length by measurement #60)./dev/shmall ruled out. Judged a site issue and not attributable to this change, but worth knowing the node wedged twice in two days shortly after bursts of container recreates, while the other two absorbed the same six recreates without a single restart.🤖 Generated with Claude Code