docs: FFT audit and staged plan (revision 3.1) - #25
Merged
Merged
Conversation
…ed plan Records the findings behind the #define-double-float complaint (symbol leakage, preprocessor-selected backends with layout-by-define, lazy table init on the RT path, per-call allocation in the float-I/O overloads, packing arithmetic re-derived at every consumer) and the relatives found elsewhere in the repo (32-bit long sample counters, sample_traits with no double profile, fir_kernels' macro + is_same pattern, noexcept allocation in solve_dense, pvoc's double float-profile). Lays out a six-stage plan ending in one Ooura port over double/float/fixed types, and the design decisions to settle before the port. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
… plan Adds Part 4 (scope of the rdft-reachable subset, class shape, tables built in double at construction, const transforms, the two-pass port under one bit-exact gate, header-only with an explicit-instantiation escape hatch, and the cleanup the port enables) and Part 5 (why the engine should be named for what it is, split_radix_rdft, with Ooura kept in attribution, NOTICE and the parity test). Records the float-twiddle defect the #define double float build carries. Adds decisions D7, D8. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
Two independent hostile reviews (numerics/port and process/ecosystem) plus an author pass, every accepted item verified against source or a probe. Blockers resolved: float numerics no longer change inside the port (the port reproduces the C's table semantics for both precisions, so float stays bit-identical and no downstream float pin moves); the 'more accurate twiddles' claim was measured and dropped; fp-contraction becomes a written contract point with a -ffp-contract=off parity gate; unrunnable M55 gates removed and replaced by compile-only plus MuTap's QEMU legs; the MuTap CI flag typo that silenced the Ooura-on-M55 leg becomes a prerequisite; the packed-spectrum view is native-convention and DspTap-only; licensing text corrected to what Ooura's notice actually grants; the fixed-point design that failed arithmetic (int32 overflow in the twiddle multiply, runtime twiddle sums up to 2.0 in cftf1st, inverse structural gain N/2, ~25 dB per-bin SNR at -40 dBFS) moves to an appendix with its constraints as entry conditions. The port is split into add / flip / remove PRs, the counter bug ships alone first, D4 gains an ABI tag, the extern-template hatch is dropped (verified ineffective for in-class definitions), and F2/F4 in the audit are reworded to their actual exposure. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
…test/CI plans Fixed point returns as Stage 3 (fixed-point projects need FFT support): one int32 radix-4 kernel with Q1.30 twiddles and shift-before-multiply, Q15 as an I/O width with two guard bits, fixed and block-floating scaling policies with both directions' scales as header numbers, its own inverse scaling, Ooura's real post-pass formulas so the packing and sign convention are one contract. D1-D3 settled accordingly. Adds Part 8 (documentation plan: header contract with pinned-by test names, docs/fft-design.md, README profiles table, provenance, CLAUDE.md, capi + executed fft notebook), Part 9 (test plan: per-file batteries, parity target with its own fp-contract flags, independent oracle, fixed-point battery, RT guard, rules), and Part 10 (embedded CI: Cortex-M4 soft-float and M4F on mps2-an386, M33 on mps2-an505, M55 on mps3-an547, platform files with the MSPLIM write guarded for Armv7E-M, one-shot gtest harness, size and instruction-count gates). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
Stage 1b and Part 11: bench/icount scenarios per profile and size, one deterministic binary each, baselines seeded from the vendored C on every QEMU leg before the routing flip, QEMU TCG instruction counts ratcheted at +/-3% per target as a failing check, .text ceilings in the same job, host wall-clock informational only. Stage 2b is gated on port-vs-C, Stage 3b seeds the fixed-point scenarios, Stage 4 gets m55 and m55-ooura keys so a backend regression is visible. D11 records it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
…m rules) to the FFT plan Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
Part 13 records the findings that change the plan rather than a PR: negative on-target test filters, visible CI output for informational and fingerprint targets, an integer bench checksum with seeding only from main, SMMULR is not a bit-exact seam for mul_coeff, the arithmetic trait docstring as the 3b spec, the spectrum view not foreclosing integer types, fingerprint tooling in both repos, corrected licensing facts about the vendored Ooura file and WebRTC's broader notice, branch protection, and the capi defects' owning stage. D6 settles that readme.txt stays at third_party/ooura permanently. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
tap
added a commit
that referenced
this pull request
Sep 17, 2026
…ews (#18) NOTICE.md (canonical): the vendored fftsg.c is not "unmodified" — upstream carries no notice (it lives only in readme.txt), the banner on the vendored copy is Tap's, trailing whitespace is stripped, and DspTap ships 2 of the package's files; state that, soften "which the distribution grant covers" to the maintainer's reading, add the judgement-call caveat, drop the uncited "informally reported lack of objection", and describe WebRTC's precedent as what it is: a broader notice in their LICENSE that is not in fft.tgz and that DspTap does not rely on. Owned-path list now covers everything outside third_party/. LicenseRef-Ooura resolves to LICENSES/LicenseRef-Ooura.txt (readme.txt lines 140-145 verbatim). readme.txt stays at third_party/ooura/ permanently; only fftsg.c moves at 2c (D6 as amended). docs/fft-design.md: points at NOTICE as canonical and gives the reasoning; same fact corrections; two profiles today, Q15/Q31 planned; the 1.105e-7 / 1.19e-7 / 1.114e-7 values labelled as the audit's Part 6 N2 probe, pinned at Stage 2a; Oppenheim & Schafer cited with edition and section; Stage 3a -> 3b for the platform-independent table; Welch derivation (3b) split from the notebook numbers (3c); consumer follow-ups list (MuTap THIRD_PARTY_NOTICES at 2c, itu-compliance glossary at 2b, ci.yml header-only at 2c, MuTap-Max transitive); banner restructured with the four house lines first and tagged as a documented exception; email addressed to the author's current Kyoto address first, "Dr.", no bounce prediction, corrected description of what DspTap ships, and the question about WebRTC's broader wording; forward-link note for the plan PR (#25). README.md: Provenance/License aligned with NOTICE; VENDOR.md: the one stale MUTAP_FFT_CMSIS -> TAP_DSP_FFT_CMSIS (Tap-authored file, not vendored source). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
tap
added a commit
that referenced
this pull request
Sep 17, 2026
* docs: precise Ooura licensing statement, stale-name fixes, and the FFT design-note skeleton NOTICE.md: fix include/dsptap/ -> include/tap/dsp/ and DSPTAP_FFT_CMSIS -> TAP_DSP_FFT_CMSIS; replace "permissive/public" with Ooura's notice quoted verbatim and a statement of what it grants (use, copy, modify; distribution of the ORIGINAL package), what DspTap ships today (the original), and what the forthcoming C++ port relies on (the modification grant; it is a derivative work, not the original package); SPDX plan LicenseRef-Ooura AND MIT for the port header, MIT elsewhere; readme.txt stays in-tree permanently. docs/fft-design.md: the design-note skeleton per audit Part 8 item 2, with every measurement-dependent entry marked TODO(stage N): per-profile contract table (double/float filled from fft.h and its tests, Q15/Q31 from the Part 7 design), the split-radix vs radix-4-int32 rationale with the Welch 1969 and Oppenheim & Weinstein 1972 references, the fp-contraction policy, the transliteration rules, empty per-target size and icount tables, the provenance and licensing statement with the port header's banner ready to paste, a draft email to the address in Ooura's notice marked for the maintainer to send (nothing sent), and an empty contract-changelog table. README.md: Provenance and License sections aligned with NOTICE; one sentence pointing at docs/ for the port plan and design note. Nothing is claimed to have landed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy * docs: correct the Ooura licensing facts after the wave-1 hostile reviews (#18) NOTICE.md (canonical): the vendored fftsg.c is not "unmodified" — upstream carries no notice (it lives only in readme.txt), the banner on the vendored copy is Tap's, trailing whitespace is stripped, and DspTap ships 2 of the package's files; state that, soften "which the distribution grant covers" to the maintainer's reading, add the judgement-call caveat, drop the uncited "informally reported lack of objection", and describe WebRTC's precedent as what it is: a broader notice in their LICENSE that is not in fft.tgz and that DspTap does not rely on. Owned-path list now covers everything outside third_party/. LicenseRef-Ooura resolves to LICENSES/LicenseRef-Ooura.txt (readme.txt lines 140-145 verbatim). readme.txt stays at third_party/ooura/ permanently; only fftsg.c moves at 2c (D6 as amended). docs/fft-design.md: points at NOTICE as canonical and gives the reasoning; same fact corrections; two profiles today, Q15/Q31 planned; the 1.105e-7 / 1.19e-7 / 1.114e-7 values labelled as the audit's Part 6 N2 probe, pinned at Stage 2a; Oppenheim & Schafer cited with edition and section; Stage 3a -> 3b for the platform-independent table; Welch derivation (3b) split from the notebook numbers (3c); consumer follow-ups list (MuTap THIRD_PARTY_NOTICES at 2c, itu-compliance glossary at 2b, ci.yml header-only at 2c, MuTap-Max transitive); banner restructured with the four house lines first and tagged as a documented exception; email addressed to the author's current Kyoto address first, "Dr.", no bounce prediction, corrected description of what DspTap ships, and the question about WebRTC's broader wording; forward-link note for the plan PR (#25). README.md: Provenance/License aligned with NOTICE; VENDOR.md: the one stale MUTAP_FFT_CMSIS -> TAP_DSP_FFT_CMSIS (Tap-authored file, not vendored source). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy --------- Co-authored-by: Claude Fable 5.1 <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.
What this changes
Adds
docs/audit-fft-and-code-smells.md: the audit of the FFT layer and its relatives across the repo, and the staged plan (revision 3.1) whose end state is one real-FFT contract overdouble,float, Q15 and Q31, with the#define double floatbuild, the global Ooura symbols and the compiled static library gone. Documentation only; no code changes.Why
The trigger was the
#define double floatretargeting offftsg.c. The audit records what else belongs to the same family (preprocessor-selected backends with layout-by-define, lazy table initialisation on the RT path, packing arithmetic re-derived at every consumer, a 32-bit sample counter that overflows after ~12 h, a substrate with nodoubleprofile, and more), ranks it, and sequences the work so each stage is independently landable with a named gate. Two adversarial reviews of the plan (Part 6) and eighteen hostile reviews of the wave-1 PRs (Part 13) are folded in; revision 3.1 is the plan of record that the wave-1 PRs (#17, #18, #19, #20, #21, #22, #23, #24 and tap/MuTap#50) cite.docs/fft-design.md(#18) links this file, so it needs to be onmain.Verification
Prose only. The hosted build and test suite at the base commit
5ca3b1cwere run at the start of the audit: warning-free with-DTAP_DSP_WERROR=ON, 160/160 tests. Every source-level claim in Parts 1, 2 and 6 was verified against the file and line it cites at5ca3b1c; measured numbers in Parts 4, 6 and 7 came from reviewer probes and are labelled as such, with the stage at which a committed test pins them.Notes for the reviewer
include/moves. Decisions D1–D11 name the contract points that later stages move and the stage that moves each.git merge-treeclean against all of them).🤖 Generated with Claude Code
https://claude.ai/code/session_019ZPTzNxo5Fe4EtpXXKf7Sy
Generated by Claude Code