From 9b8ae57bcfc5edf24facc823bd0408345aec8a93 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 13 May 2026 21:54:59 +0000 Subject: [PATCH] docs(history): add torchfx library review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Library evaluation of matteospanio/torchfx against ASA's per-band filtering call sites. Conclusion: do not adopt — GPL v3 vs ASA's MIT license is a one-way door, the shipped filter inventory is too sparse to displace scipy, and Phase 1 bandpass cost is not on the critical path. The torchfx |/+ operator pattern and the (tensor, fs, device) carrier object are worth borrowing only if a future profile shows per-band filtering is a top-three Phase 1 cost on long tracks. https://claude.ai/code/session_01WJ4q6kzeEMscgnSa9gRqQk --- .../library-review-torchfx-2026-05-13.md | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 docs/history/library-review-torchfx-2026-05-13.md diff --git a/docs/history/library-review-torchfx-2026-05-13.md b/docs/history/library-review-torchfx-2026-05-13.md new file mode 100644 index 00000000..0327c2bf --- /dev/null +++ b/docs/history/library-review-torchfx-2026-05-13.md @@ -0,0 +1,69 @@ +# Library review — `matteospanio/torchfx` + +**Date:** 2026-05-13 +**Reviewer:** automated audit on `claude/review-torchfx-inspiration-LpSZq` +**Verdict:** **Do not adopt as a dependency.** Borrow the API ergonomics if a future GPU batched-filter pass is ever justified. + +--- + +## What torchfx is + +- PyTorch-native audio DSP. All filters subclass `torch.nn.Module`, so they participate in autograd and ride PyTorch's device placement. +- Composition via overloaded operators: `wave | f1 | f2` for sequential, `f1 + f2` for parallel-then-mix. +- `Wave` object carries a sample-rate (`.fs`) alongside the tensor, with `Wave.from_file()` / `.save()` (soundfile-backed). +- Filter inventory is **thin**: `LoButterworth`, `HiButterworth` (implied/example), `ParametricEQ`. No MIR, no loudness, no onset/key/BPM, no spectrogram primitives. +- Created March 2025, 12 releases, 205 commits, arXiv companion paper (2504.08624). Active but young. +- **License: GPL v3.** +- Deps: torch, numpy, scipy, soundfile. Python ≥ 3.10. + +## What ASA does in this neighborhood + +ASA's Phase 1 currently lives in Essentia + scipy + librosa. Specifically the spots that look like "filters as nn.Modules" would touch: + +| Site | What it does | How | +|---|---|---| +| `analyze_detection.py:38` `_bandpass_signal` | 4th-order Butterworth bandpass for per-band RT60 (`_REVERB_BANDS`, 4 octave bands). | `scipy.signal.butter(... output="sos")` + `sosfiltfilt`. | +| `analyze_detection.py:97-120` `analyze_per_band_transient_density` | 7-band bandpass → `librosa.onset.onset_strength` for hi-hat / kick density across spectralBalance bands. | Same scipy SOS pipeline, then librosa per band. | +| `analyze_core.py:894-903` | Per-band stereo bandpass via Essentia's `BandPass`. | Essentia's IIR bandpass, applied L/R. | +| `analyze_detection.py:481+, ~554` | Bandpass before envelope follower for effects detail. | Same scipy bandpass path. | + +Total ≈11 bandpasses per analysis run, each over a single mono (or stereo-paired) numpy array. The audio is bounded above by the 100 MiB upload cap. + +## Where torchfx's pattern *could* help + +If — and only if — these per-band passes ever become a visible cost driver: + +1. **Batched filter bank.** Today's 7-band transient-density pass is a serial Python loop of `sosfiltfilt`. A torchfx-style `BatchedFilterBank` (one `nn.Module` that stacks `(n_bands, sos)` and dispatches `torchaudio.functional.lfilter` across the batch dim) collapses 7 CPU passes into one GPU launch. The audio already lives on GPU during Demucs in `analyze_transcription.py` — re-using that device placement instead of round-tripping through numpy is the actual win, not the GPU itself. +2. **Stem-time co-location.** Demucs writes tensors on GPU. The current Layer-1 stem analysis pulls them back to CPU/numpy for Essentia+scipy. A torchaudio-based per-stem feature pass would keep the device. Borrowing torchfx's `Wave(tensor, fs, device)` carrier — *not* the class — is what makes that ergonomic. +3. **Readability.** `mono | hp(20) | lp(8000) | onset_env` reads better than four explicit numpy intermediates. The `|`/`+` overloading is the one piece of torchfx's API that's worth stealing wholesale if any of the above lands. + +## Why we should *not* depend on torchfx + +1. **License incompatibility.** ASA is MIT (`/LICENSE`). torchfx is GPL v3. Linking torchfx into the backend forces the combined work to GPL v3 on redistribution. This is a one-way door against any future hosted/commercial path and against permissive contributions back to ASA. **This is the blocking issue.** The next four are secondary. +2. **Filter inventory is too sparse to matter.** Two filter primitives shipped. ASA would write its own Butterworth bandpass anyway; the only thing borrowed would be `nn.Module` plumbing — which is a 30-line wrapper around `torchaudio.functional.lfilter`. +3. **Differentiability is unused.** ASA's filters are analysis tools, not training targets. Autograd through the filter is dead weight. +4. **No MIR/loudness offset.** torchfx does not replace any Essentia path. It is strictly upstream of measurement (signal conditioning), not a measurement tool itself. +5. **Quality invariant friction.** PURPOSE invariant #1: "Phase 1 measurements are ground truth." Switching the bandpass implementation under existing measurements (subBass, lowBass, …, brilliance) changes filter phase / transient response and would shift downstream onset counts and RT60 fits. Any swap needs a snapshot-diff regression test (`EXPECTED_TOP_LEVEL_KEYS` is not enough — values change, not keys). + +## Concrete recommendation + +**Do nothing in the dependency graph.** Keep Essentia + scipy as-is. The 11-bandpass-per-run cost is invisible next to Demucs and Phase 2 latency. Working the chain of custody is the higher-leverage path. + +**If — at some future point — a profile shows per-band filtering is a top-three Phase 1 cost on long tracks:** + +- Add `apps/backend/dsp_utils.py::TorchFilterBank` as a thin internal class. ~50 LOC. Stack SOS coefficients as `(n_bands, n_sections, 6)`, dispatch via `torchaudio.functional.lfilter`, return `(n_bands, n_samples)` tensor. +- Adopt the `|` operator for chains only if it lands in a hot loop. Avoid `+` (parallel-mix) — ASA does not mix bands back together; it analyzes each separately. +- Place the new module behind a feature flag so the scipy path remains the snapshot-test baseline. Diff old vs new outputs on a representative corpus before flipping the default. +- Do *not* import torchfx. Implement the pattern, not the library. + +## Borrowed-pattern checklist (for when/if we ever do this) + +1. Filters carry their own `fs` so callers don't pass it at every call site. (torchfx convention; correct.) +2. Filters are stateless once constructed; coefficients are buffers (`register_buffer`), not parameters. (Avoids accidental autograd attachment in eval contexts.) +3. The carrier object holds `(tensor, fs, device)` not `(np.ndarray, fs)`. The device field is what enables co-location with Demucs. +4. Operators wrap explicit method calls, not the other way around — `wave | filt` should be `filt.forward(wave)`. Keeps the call graph debuggable. +5. Round-trip parity test against scipy baseline before swapping any production code path. + +## Bottom line + +torchfx is a reasonable proof of concept for a pattern. It is not a dependency we should take, and the pattern itself only pays off if and when per-band CPU filtering becomes a measured bottleneck. Until then: file under "library to remember, not library to install."