diag: the float32 FFT probe that found #31's root cause

.github/workflows/fft-diagnostic.yml

@@ -0,0 +1,323 @@
+name: float32 FFT diagnostic (#31)
+
+# Temporary diagnostic for tap/MuTap#31 — DELETE THIS FILE when the issue
+# closes. It answers the three questions the investigation is blocked on, all
+# on the runner where the bifurcation actually occurs (macos-latest is an
+# Apple Silicon VM; the draw does NOT reproduce on Intel, so a local Mac
+# cannot substitute):
+#
+#   1a  Which FFT sizes are affected? Only N=512 and N=2048 have ever been
+#       probed. A consumer whose geometry is elsewhere may be unexposed, and
+#       a consumer whose geometry is 4096 may be exposed and unaware.
+#   1b  How far apart are the two draws? This is the fork in the road. A gap
+#       >= 1e-4 relative means the backend violates its own documented 4e-7
+#       contract outright and this is Apple's bug. A gap at ~1 ULP means the
+#       contract is honoured and OUR chain bifurcates on a legitimate backend
+#       difference — a materially more serious finding, since it would apply
+#       to any future toolchain or platform.
+#   1c  Is the draw selected by buffer address? If it is, over-aligning
+#       m_rp/m_ip in DspTap's wrapper fixes this while keeping vDSP and its
+#       ~3x speedup — the one outcome that costs nothing.
+#
+# Separate workflow rather than a job in ci.yml so it can be dispatched on
+# demand, and so deleting it later is one file rather than a surgical edit.
+
+on:
+  workflow_dispatch:
+  push:
+    branches: ['diag/**']
+
+jobs:
+  fft-determinism:
+    name: float32 FFT determinism (macOS)
+    runs-on: macos-latest
+    timeout-minutes: 60
+    steps:
+      - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
+        with:
+          submodules: recursive
+
+      # Every result here is only attributable with the chip and SDK recorded:
+      # the split is observed on M1 + macOS 26.5 + Xcode 26.6 and not on
+      # Intel + macOS 15.7 + AppleClang 17.
+      - name: Record host identity
+        run: |
+          sysctl -n machdep.cpu.brand_string
+          sysctl -n hw.model
+          sysctl -n hw.ncpu
+          sw_vers
+          cc --version | head -2
+
+      - name: Build the probe against both float32 backends
+        run: |
+          set -eu
+          cc -O2 -c submodules/dsptap/third_party/ooura/fftsg.c -o /tmp/fftsg.o
+          cc -O2 -c submodules/dsptap/third_party/ooura/fftsg_float.c -o /tmp/fftsg_float.o
+          c++ -std=c++20 -O2 -DTAP_DSP_FFT_ACCELERATE -Isubmodules/dsptap/include \
+            tests/fft_determinism_probe.cpp /tmp/fftsg.o /tmp/fftsg_float.o \
+            -framework Accelerate -o /tmp/probe_vdsp
+          c++ -std=c++20 -O2 -Isubmodules/dsptap/include \
+            tests/fft_determinism_probe.cpp /tmp/fftsg.o /tmp/fftsg_float.o -o /tmp/probe_ooura
+
+      # 1a. 200 processes per backend across every power of two 64..8192. One
+      # line per size per process, so "1 distinct hash per size" is
+      # deterministic and more is not. Ooura is the control: it must come back
+      # with exactly one hash per size, or the experiment proves nothing.
+      - name: 1a — size sweep, 200 processes per backend
+        run: |
+          set -eu
+          echo "## 1a — float32 FFT determinism by size (200 processes/backend)" >> "$GITHUB_STEP_SUMMARY"
+          for backend in vdsp ooura; do
+            rm -rf "/tmp/dump_$backend"; mkdir -p "/tmp/dump_$backend"
+            for _ in $(seq 200); do "/tmp/probe_$backend" sweep --dump "/tmp/dump_$backend"; done \
+              | sort | uniq -c | sort -k2,2 -k1,1rn > "/tmp/hist_$backend"
+            echo "### $backend" >> "$GITHUB_STEP_SUMMARY"
+            {
+              echo '```'
+              echo 'draws  N        hash              intra'
+              cat "/tmp/hist_$backend"
+              echo
+              echo 'distinct draws per size:'
+              awk '{print $2}' "/tmp/hist_$backend" | sort | uniq -c \
+                | awk '{printf "  %-9s %s draw(s)%s\n", $2, $1, ($1>1 ? "   <-- NONDETERMINISTIC" : "")}'
+              echo '```'
+            } >> "$GITHUB_STEP_SUMMARY"
+            echo "== $backend =="; cat "/tmp/hist_$backend"
+          done
+
+      # 1b. The dump directory now holds one file per distinct draw, named
+      # N<size>-<hash>.bin. Any size with two files is a bifurcating size:
+      # diff its draws and let the probe classify the gap.
+      - name: 1b — how far apart are the draws
+        run: |
+          set -eu
+          echo "## 1b — distance between draws" >> "$GITHUB_STEP_SUMMARY"
+          found=0
+          for n in 64 128 256 512 1024 2048 4096 8192; do
+            # shellcheck disable=SC2231
+            set -- /tmp/dump_vdsp/N$n-*.bin
+            if [ "$#" -ge 2 ]; then
+              found=1
+              echo "### N=$n — $# distinct draws" >> "$GITHUB_STEP_SUMMARY"
+              {
+                echo '```'
+                /tmp/probe_vdsp diff "$1" "$2"
+                echo '```'
+              } >> "$GITHUB_STEP_SUMMARY"
+              echo "== N=$n =="; /tmp/probe_vdsp diff "$1" "$2"
+            fi
+          done
+          if [ "$found" -eq 0 ]; then
+            echo "No size produced more than one draw on this host — nothing to diff." \
+              | tee -a "$GITHUB_STEP_SUMMARY"
+          fi
+
+      # 1b'. The draw-vs-draw comparison above only exists on a host that
+      # bifurcates. This one always exists, and on Intel it is where the
+      # finding turned out to be: vDSP vs Ooura sits INSIDE the documented
+      # contract measured as peak-normalized absolute error, and PAST the
+      # chain's measured breaking point measured as per-bin relative error.
+      # Same numbers, two metrics, ~1700x apart. Run it at every size so the
+      # metric gap is on the record for whichever geometry a consumer uses.
+      - name: 1b' — vDSP vs Ooura, per size, both metrics
+        run: |
+          set -eu
+          echo "## 1b' — vDSP vs Ooura (same host, same input)" >> "$GITHUB_STEP_SUMMARY"
+          for n in 64 128 256 512 1024 2048 4096 8192; do
+            v=$(ls /tmp/dump_vdsp/N$n-*.bin 2>/dev/null | head -1 || true)
+            o=$(ls /tmp/dump_ooura/N$n-*.bin 2>/dev/null | head -1 || true)
+            if [ -n "$v" ] && [ -n "$o" ]; then
+              echo "### N=$n" >> "$GITHUB_STEP_SUMMARY"
+              {
+                echo '```'
+                /tmp/probe_vdsp diff "$v" "$o"
+                echo '```'
+              } >> "$GITHUB_STEP_SUMMARY"
+              echo "== N=$n =="; /tmp/probe_vdsp diff "$v" "$o"
+            fi
+          done
+
+      # 1c. Within ONE process: same input, same FFTSetup, buffers moved to
+      # deliberate byte offsets. The cross-process sweep cannot separate "the
+      # draw is fixed at process start" from "the draw follows the heap
+      # address", because both are constant within a process; this can.
+      - name: 1c — is the draw selected by buffer address
+        run: |
+          set -eu
+          echo "## 1c — alignment discriminator" >> "$GITHUB_STEP_SUMMARY"
+          for n in 512 2048 4096; do
+            {
+              echo '```'
+              /tmp/probe_vdsp align "$n"
+              echo '```'
+            } >> "$GITHUB_STEP_SUMMARY"
+            /tmp/probe_vdsp align "$n"
+          done
+
+  # ---------------------------------------------------------------------------
+  # Round 2. The alignment fix made vDSP deterministic by forcing every process
+  # onto the 64-byte-aligned kernel. It was never checked whether that is the
+  # kernel on which the compliance rows PASS — the 140/60 split was measured on
+  # FFT output hashes, not on test outcomes. If the tone rows pass on the OTHER
+  # path, the fix picked the losing kernel and the story needs rewriting.
+  #
+  # Three builds of the same tree, same host, same run:
+  #   aligned    as shipped (m_rp/m_ip forced to 64-byte alignment)
+  #   unaligned  the fix deliberately defeated (+4 bytes past the boundary), so
+  #              every process takes the non-64-aligned kernel
+  #   ooura      backend control
+  #
+  # Each runs the two failing tonal rows plus the broadband row that passes, 5
+  # repetitions each, so within-variant stability is visible too. A row that
+  # fails 5/5 in one variant and passes 5/5 in another is decisive; a row that
+  # wanders inside a variant means a source of variation survives the fix and
+  # the diagnosis is incomplete.
+  tone-rows-vs-kernel-path:
+    name: tone rows vs FFT kernel path (macOS)
+    runs-on: macos-latest
+    timeout-minutes: 90
+    steps:
+      - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
+        with:
+          submodules: recursive
+
+      - name: Record host identity
+        run: |
+          sysctl -n machdep.cpu.brand_string
+          sysctl -n hw.model
+          sw_vers
+          cc --version | head -2
+
+      - name: Build and run all three variants
+        run: |
+          set -u
+          fft=submodules/dsptap/include/tap/dsp/fft.h
+          rows='g168_adapted\.ToneStability<float>|Float32Parity\.ToneRowWithNarrowbandGuard|itu_echo\.EchoStability<float>'
+          cp "$fft" /tmp/fft.h.orig
+          echo "## Round 2 — do the tone rows depend on which vDSP kernel runs?" >> "$GITHUB_STEP_SUMMARY"
+
+          run_variant() {
+            name="$1"; shift
+            echo "=============== $name ==============="
+            cmake -B "build-$name" -DCMAKE_BUILD_TYPE=Release "$@" > "/tmp/cfg-$name.log" 2>&1 || {
+              echo "configure FAILED"; tail -20 "/tmp/cfg-$name.log"; return 1; }
+            grep -E '^TAP_DSP_FFT_ACCELERATE' "build-$name/CMakeCache.txt" || echo "(accelerate default)"
+            cmake --build "build-$name" --parallel > "/tmp/build-$name.log" 2>&1 || {
+              echo "build FAILED"; tail -20 "/tmp/build-$name.log"; return 1; }
+            set +e
+            ctest --test-dir "build-$name" --output-on-failure --repeat until-fail:5 -R "$rows" \
+              > "/tmp/test-$name.log" 2>&1
+            rc=$?
+            set -e
+            echo "ctest rc=$rc"
+            grep -E "tests passed|Passed|\*\*\*Failed|actual:" "/tmp/test-$name.log" | head -40
+            {
+              echo "### $name (ctest rc=$rc)"
+              echo '```'
+              grep -E "tests passed|Passed|\*\*\*Failed|actual:" "/tmp/test-$name.log" | head -40
+              echo '```'
+            } >> "$GITHUB_STEP_SUMMARY"
+          }
+
+          # 1. As shipped.
+          run_variant aligned
+
+          # 2. Fix defeated: push the split buffers 4 bytes PAST the 64-byte
+          #    boundary so every process takes the other kernel. The assert
+          #    makes a silent no-op impossible — without it a failed patch
+          #    would quietly run the same build twice and look like a result.
+          python3 - <<'PY'
+          p = 'submodules/dsptap/include/tap/dsp/fft.h'
+          s = open(p).read()
+          old = 'return reinterpret_cast<float*>(up);'
+          assert s.count(old) == 1, f'patch site found {s.count(old)} times, expected 1'
+          s = s.replace(old, 'return reinterpret_cast<float*>(up + 4); // #31 round 2: force the NON-64-aligned kernel')
+          open(p, 'w').write(s)
+          print('patched: split buffers forced 4 bytes past the 64-byte boundary')
+          PY
+          run_variant unaligned
+          cp /tmp/fft.h.orig "$fft"
+
+          # 3. Backend control.
+          run_variant ooura -DTAP_DSP_FFT_ACCELERATE=OFF
+
+      # Independent of the rows: with the fix pinned, is the wrapper's output
+      # now actually invariant across processes ON THIS HOST? The sweep in the
+      # job above proved the bifurcation before the fix; this is the same
+      # measurement after it, and it is the in-situ check that the fix does
+      # what it claims rather than only passing DspTap's own tests.
+      - name: Post-fix determinism re-check (200 processes)
+        if: always()
+        run: |
+          set -eu
+          cc -O2 -c submodules/dsptap/third_party/ooura/fftsg.c -o /tmp/fftsg.o
+          cc -O2 -c submodules/dsptap/third_party/ooura/fftsg_float.c -o /tmp/fftsg_float.o
+          c++ -std=c++20 -O2 -DTAP_DSP_FFT_ACCELERATE -Isubmodules/dsptap/include \
+            tests/fft_determinism_probe.cpp /tmp/fftsg.o /tmp/fftsg_float.o \
+            -framework Accelerate -o /tmp/probe_vdsp
+          for _ in $(seq 200); do /tmp/probe_vdsp sweep; done | sort | uniq -c | sort -k2,2 -k1,1rn > /tmp/hist
+          {
+            echo "## Post-fix determinism (expect exactly 1 draw per size)"
+            echo '```'
+            cat /tmp/hist
+            echo
+            awk '{print $2}' /tmp/hist | sort | uniq -c \
+              | awk '{printf "  %-9s %s draw(s)%s\n", $2, $1, ($1>1 ? "   <-- STILL NONDETERMINISTIC" : "")}'
+            echo '```'
+          } >> "$GITHUB_STEP_SUMMARY"
+          cat /tmp/hist
+
+      # Which kernel is actually CORRECT? The A/B above shows the compliance
+      # rows pass on the non-64-aligned kernel and fail on the aligned one, but
+      # that alone does not say the aligned kernel is WRONG — it may simply be
+      # differently rounded, in which case selecting the other one is choosing
+      # a lucky coin rather than fixing anything. Measuring both against a
+      # double-precision reference decides which it is, and therefore whether
+      # changing the alignment constant is engineering or superstition.
+      - name: Which kernel is more accurate (vs a double reference)
+        if: always()
+        run: |
+          set -eu
+          fft=submodules/dsptap/include/tap/dsp/fft.h
+          cp "$fft" /tmp/fft.h.acc
+
+          # Stock header: the wrapper reports the 64-aligned kernel.
+          c++ -std=c++20 -O2 -DTAP_DSP_FFT_ACCELERATE -Isubmodules/dsptap/include \
+            tests/fft_determinism_probe.cpp /tmp/fftsg.o /tmp/fftsg_float.o \
+            -framework Accelerate -o /tmp/acc_aligned
+
+          # Patched header: the SAME wrapper, now landing off the boundary.
+          # Measuring through the wrapper is the point — it removes the probe's
+          # own buffer placement as an explanation for the earlier result.
+          python3 - <<'PY'
+          p = 'submodules/dsptap/include/tap/dsp/fft.h'
+          s = open(p).read()
+          old = 'return reinterpret_cast<float*>(up);'
+          assert s.count(old) == 1, f'patch site found {s.count(old)} times, expected 1'
+          open(p, 'w').write(s.replace(old, 'return reinterpret_cast<float*>(up + 4);'))
+          print('patched for the accuracy comparison')
+          PY
+          c++ -std=c++20 -O2 -DTAP_DSP_FFT_ACCELERATE -Isubmodules/dsptap/include \
+            tests/fft_determinism_probe.cpp /tmp/fftsg.o /tmp/fftsg_float.o \
+            -framework Accelerate -o /tmp/acc_unaligned
+          cp /tmp/fft.h.acc "$fft"
+
+          c++ -std=c++20 -O2 -Isubmodules/dsptap/include \
+            tests/fft_determinism_probe.cpp /tmp/fftsg.o /tmp/fftsg_float.o -o /tmp/acc_ooura
+
+          {
+            echo "## Accuracy against a double-precision reference"
+            echo "Read the p50 column: it is the median bin, so it says whether"
+            echo "MOST of the spectrum is right, which max_rel cannot on sparse"
+            echo "material and abs/peak cannot on any material."
+            echo '```'
+            for n in 512 2048 4096; do
+              echo "---- built 64-ALIGNED (as shipped) ----"
+              /tmp/acc_aligned truth "$n"
+              echo "---- built NOT-64-ALIGNED (fix defeated) ----"
+              /tmp/acc_unaligned truth "$n"
+              echo "---- built OOURA ----"
+              /tmp/acc_ooura truth "$n"
+            done
+            echo '```'
+          } | tee -a "$GITHUB_STEP_SUMMARY"