Skip to content

Toolbox audit: lazy import, merged duplicates, consistent DSS API - #255

Merged
SebastianJiroSchlecht merged 5 commits into
mainfrom
claude/affectionate-mccarthy-k91r9v
Sep 27, 2026
Merged

SebastianJiroSchlecht merged 5 commits into
mainfrom
claude/affectionate-mccarthy-k91r9v

Conversation

@SebastianJiroSchlecht

@SebastianJiroSchlecht SebastianJiroSchlecht commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Implements the toolbox audit. It removes duplicated implementations, dead fallback code and one-function modules, fixes an RNG reproducibility bug, makes import pyFDN light, makes the delay state-space API consistent, and replaces Plotly with Matplotlib throughout.

As agreed, these two things are unchanged:

  • Every non-Plotly top-level export stays. One name is added (plot_fdn_build). The two Plotly-only helpers downsampled_scatter and downsample_plotly_trace are removed.
  • The nonlinear td operators stay in the library.

Changes

Bug fixes

  • construct_cascaded_paraunitary_matrix (and the velvet/random_dense gallery types), shift_matrix_distribute and random_matrix_shift ignored np.random.seed, because they drew from an unseeded default_rng(); the tests had been patching around this. They now use NumPy's global state like the rest of generate/. A regression test is added.
  • rt_to_gain_per_sample docstring: the level after RT seconds is −60 dB, not −30 dB.
  • matrix_sqrt always returned float32. It now keeps the input's dtype.

Import time and dead code

  • import pyFDN no longer loads torch or flamo: about 2.9 s → 0.9 s. The training API and flamo_to_pr load on first attribute access. tests/test_import.py guards this.
  • flamo and scipy are required dependencies, so these are removed:
    • about a dozen "is flamo installed?" checks
    • the placeholder AttenuationFilter/OutputEQ classes
    • the scipy-less sqrtm fallback
  • Removed auxiliary/coupled_rooms.py, a hand-wired FLAMO FDN used only by its own test; example_coupled_rooms covers the model.
  • Removed the unused flareverb dependency, the dead docs/references.py, the setup.py shim and stale mypy overrides.

Merged duplicates (outputs checked against the old code)

  • One time-domain engine. process_dss now renders through dss_to_td. dss_to_td runs post_delay directly on the delay output, so results are bit-identical to the old process_dss for LTI, time-varying and nonlinear hooks (checked for SOS, PitchShift, RingModulator and time-varying Kronecker hooks). dss_to_impz and build_to_impz share one impulse loop.
  • One FLAMO builder. dss_to_flamo and trainable_from_build share one fdn_core().
  • Energy decay curve. edc and energy_decay_curve share one Schroeder integral that works on NumPy arrays and torch tensors. The loss octave edges are derived from octave_bands() (same values).
  • Residues. The residue formula and multipole handling exist once, used by both dss_to_pr and flamo_to_pr.
  • Small helpers now written once: the random-orthogonal draw (was 3 copies), seed handling (3), default device (5), tensor-to-NumPy conversion (4), the losses' channels check (5), the RT-to-dB formula, and the filter target validation.

Consistency

  • Breaking: delay state-space arguments are (delays, A, B, C, D) everywhere:

    • dss_to_flamo(delays, A, B, C, D, fs, ...)
    • is_allpass(delays, A, B, C, D)
    • dss_to_impz(delays, A, B, C, D, ir_len)

    All call sites in src/, tests/ and examples/ were rewritten mechanically with an AST script.

  • Breaking: pole_boundaries takes the absorption as an (sections, 6, N) SOS bank (the FDNBuild.post_delay format) instead of a .b/.a object. Output is identical to the old version.

  • Breaking: tiny_rotation_matrix and rotation_matrix_from_angles now return NumPy arrays; every caller already converted the torch result.

  • Gallery names: all three galleries use snake_case type names ("nested_allpass", "velvet", …). The old fdnToolbox spellings are still accepted.

  • Renames and defaults: plot_fdn_build is the new name (plot_FDN_build stays as an alias), and flamo_to_pr no longer prints progress by default.

Plotting: Matplotlib only

  • Breaking: every plot function now returns a Matplotlib figure, built without pyplot so figures don't accumulate. Arguments and look are unchanged: zero-centred RdBu heatmaps, proportional block sizes, delay bars aligned with the columns of A, and curves coloured by delay length.
  • Breaking: animate returns a Matplotlib FuncAnimation; in a notebook, show it with mo.Html(anim.to_jshtml()). The Plotly-only transition_ms argument is removed.
  • SDN.visualize draws a Matplotlib 3-D axes.
  • Breaking: downsampled_scatter and downsample_plotly_trace are removed.
  • plotly is removed from every extra, and all example notebooks use Matplotlib. Ruff allows a bare fig as a cell's last expression in examples/, because that is how marimo displays a figure.

Less verbosity and structure

  • In generate/, fifteen one-function modules become orthogonal.py, paraunitary.py and structures.py. The allpass structure builders move there and are still re-exported from pyFDN.allpass.
  • fdn_build_gallery loses its two @overload copies of its signature.
  • Duplicated rationale in the training docstrings is trimmed.

CI: a new lint job runs ruff check, ruff format --check and mypy, and mypy is clean. The four unformatted test files are now formatted. Optional imports (marimo, mpl_toolkits) are in mypy's ignore-missing-imports list, because the lint job doesn't install the examples extra.

Testing

  • pytest passes locally: 523 passed, including all example notebooks (test_marimo_examples_run.py)
  • marimo export html of several notebooks (animation, SDN, gallery, echo density) runs with no cell errors
  • New tests: import stays light and every export resolves; seeded reproducibility of the paraunitary constructors; NumPy tiny rotation matrix; gallery name aliases; plot_FDN_build alias; plot and SDN tests now check the Matplotlib figures
  • Coverage not measured separately

Checklist

  • ruff check, ruff format --check and python -m mypy all clean
  • Docstrings updated for the changed public API
  • HISTORY.rst (Unreleased) and CONTRIBUTING.rst updated
  • sdist and wheel build; check-wheel.py and twine check pass

🤖 Generated with Claude Code

https://claude.ai/code/session_01CNoCYhy16up9ifdTYpfUFX

- construct_cascaded_paraunitary_matrix (and the Velvet/RandomDense
  gallery types) could not be reproduced with np.random.seed, because
  shift_matrix_distribute and random_matrix_shift drew from an unseeded
  np.random.default_rng(). They now use NumPy's global state like the
  rest of pyFDN.generate; the test fixture no longer needs to
  monkeypatch default_rng, and a regression test covers it.
- random_orthogonal takes an optional rng; fdn_build_gallery and
  build_fdn use it instead of their own copies of the Haar QR draw
  (outputs are bit-identical for the same seed).
- td.connectors reuses operators._as_2d instead of a verbatim copy.
- rt_to_gain_per_sample docstring: 10^-3 after rt seconds is -60 dB,
  not -30 dB.
- Drop the flareverb runtime dependency (never imported; torchaudio
  still arrives through flamo) and two mypy overrides for modules that
  no longer exist (pyFDN.recursive, translate.dss_to_pr_flamo).
- Remove docs/references.py, an unused compatibility shim.
- Makefile: lint with ruff (flake8 is not a dependency and pointed at a
  non-src path); build/install via python -m build / pip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E5CGmfvpxMnpAt4QYPKs6c
- import pyFDN no longer loads torch or flamo (~2.9 s -> ~0.9 s). The
  training API and the FLAMO pole finder resolve on first access through
  a module __getattr__; every name in __all__ stays exported. FLAMO
  wrappers import flamo inside the functions that need it.
- flamo and scipy are required dependencies, so the "is it installed?"
  guards, the placeholder AttenuationFilter/OutputEQ classes and the
  scipy-less sqrtm fallback are gone.
- generate/: fifteen one-function modules become orthogonal.py,
  paraunitary.py and structures.py. The allpass structure builders
  (series/nested/poletti) move there from auxiliary/allpass.py, which
  re-exports them; is_almost_zero moves to auxiliary/utils.py.
- tiny_rotation_matrix and rotation_matrix_from_angles are NumPy (every
  caller converted the torch result anyway) and draw from NumPy's global
  random state; the dtype argument is gone.
- Remove auxiliary/coupled_rooms.py: a hand-wired FLAMO FDN used only by
  its own test; example_coupled_rooms builds the network with
  dss_to_flamo. It also carried a private copy of matrix_sqrt.
- Shared default_device() and to_numpy() helpers replace five copies of
  the CUDA-else-CPU default and four tensor-to-NumPy conversions;
  flamo_to_pr reads delays with flamo_graph's reader instead of its own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E5CGmfvpxMnpAt4QYPKs6c
- process_dss is now dss_to_td(...).process_signal: one time-domain FDN
  engine. dss_to_td runs the recursion on the delay bank itself, so
  post_delay sees the delay output in real time (bit-identical to the old
  process_dss for LTI, nonlinear and time-varying hooks) and hooks may be
  SOS banks, td operators or any process_block object. dss_to_impz and
  build_to_impz share one impulse loop.
- dss_to_flamo and trainable_from_build build their FLAMO core through
  one fdn_core(); training only swaps in the parametrized matrix and the
  trainable flags.
- edc and energy_decay_curve share one backend-agnostic
  schroeder_integral (NumPy or torch); the loss octave edges derive from
  octave_bands(); array_namespace moves to auxiliary.utils.
- Residue extraction (null-vector formula and multipole handling) is one
  helper used by both dss_to_pr and flamo_to_pr.
- One as_generator() for seeds; the losses' channels check exists once.
- Breaking: DSS arguments are (delays, A, B, C, D) everywhere:
  dss_to_flamo(delays, A, B, C, D, fs, ...), is_allpass(delays, A, B, C,
  D), dss_to_impz(delays, A, B, C, D, ir_len). All callers updated.
- Gallery type names are snake_case in all three galleries
  (fdn_system_gallery: "nested_allpass", "allpass_in_fdn", ...;
  filter_matrix_gallery: "velvet", "random_dense", "from_elementals");
  the old fdnToolbox spellings are still accepted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E5CGmfvpxMnpAt4QYPKs6c
- Breaking: pole_boundaries takes the absorption as an (sections, 6, N)
  SOS bank, the FDNBuild.post_delay format, instead of an object with b
  and a fields; it also accepts a 2-D feedback matrix. Output is
  unchanged (checked against the old implementation).
- plot_fdn_build replaces plot_FDN_build, which stays as an alias. The
  plot module documents why plot_spectrogram and
  plot_impulse_response_matrix stay Matplotlib (notebook size, subplot
  grids) while the rest is Plotly.
- flamo_to_pr no longer prints by default, like dss_to_pr.
- matrix_sqrt keeps the input dtype (was float32 / complex64 always).
- fdn_build_gallery: drop the two @overload copies of its signature.
- AttenuationFilter reuses the eq RT-to-dB formula; its target
  validation is one function instead of two near-copies.
- Trim duplicated rationale in the training docstrings.
- CI: new lint job runs ruff check/format and mypy (now clean). Format
  the four test files that were not. Drop the setup.py shim and ruff's
  copy of its own default excludes.
- CONTRIBUTING and HISTORY describe the changes in this branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E5CGmfvpxMnpAt4QYPKs6c
Every plot helper now returns a pyplot-free Matplotlib figure, animate()
returns a FuncAnimation (shown with .to_jshtml()), and SDN.visualize draws
a Matplotlib 3-D axes. The Plotly-only helpers downsampled_scatter and
downsample_plotly_trace are removed, plotly is dropped from every extra,
and all example notebooks use Matplotlib.

This also fixes the two CI failures on this branch: example_train_fdn_to_rir
used plotly.express, which needs pandas, and mypy could not resolve the
optional marimo import in the lint job.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CNoCYhy16up9ifdTYpfUFX
@SebastianJiroSchlecht
SebastianJiroSchlecht merged commit a520955 into main Sep 27, 2026
7 checks passed
@SebastianJiroSchlecht
SebastianJiroSchlecht deleted the claude/affectionate-mccarthy-k91r9v branch September 27, 2026 10:08
SebastianJiroSchlecht pushed a commit that referenced this pull request Sep 27, 2026
Resolve conflicts with #255: keep both HISTORY entries, require
flamo>=0.2.20 and drop flareverb, take the (delays, A, B, C, D) argument
order in dss_to_flamo and the new tests, pass device through main's
wrap_fdn_shell call, accept the removal of auxiliary/coupled_rooms.py,
and use main's default_device in wrap_fdn_shell.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q6VyZo1pfcFKwK1Nz6cWcC
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants