Skip to content

perf(htdemucs): default to GPU backend when available - #414

Closed
tilllt wants to merge 2 commits into
CrispStrobe:mainfrom
tilllt:fix/htdemucs-gpu-default
Closed

tilllt wants to merge 2 commits into
CrispStrobe:mainfrom
tilllt:fix/htdemucs-gpu-default

Conversation

@tilllt

@tilllt tilllt commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Description

Two commits:

1. perf(htdemucs): default to GPU backend when availablewant_gpu now defaults to true (opt-out via CRISPASR_HTDEMUCS_GPU=0; params.use_gpu still wins).

2. perf(htdemucs): GPU needs the ggml graph path — wire graph+FUSED to want_gpu — follow-up after live measurement showed that want_gpu alone does not use the GPU: the ggml graph path is the gate to the GPU backend (if (want_gpu && htdemucs_use_ggml())), and htdemucs_use_ggml() is default OFF. The gate is now wired so GPU setups get graph+FUSED together; CPU-only hosts keep their BLAS path (no behavior change for them).

Measured (RTX 3090 Ti, CrispASR 0.8.30 / 81595d1, htdemucs-f16.gguf)

  • No flags (CPU default): RTF ≈ 7.4 (25 min for a 202 s clip), 0% GPU util.
  • CRISPASR_HTDEMUCS_GPU=1 alone: still CPU (0% GPU util) — the graph gate blocked it.
  • CRISPASR_HTDEMUCS_GPU=1 + CRISPASR_HTDEMUCS_GGML=1 + CRISPASR_HTDEMUCS_FUSED=1: 30 s clip in 11 s (RTF ≈ 0.37 incl. model init), 65% GPU util.

Verification

  • Build: cmake --build build --target crispasr-cli — clean (100% built) on both commits.

Refs #413

Disclosure

Drafted with assistance from an AI agent on behalf of the contributor.

The fused graph path is dramatically faster on CUDA (measured RTF ~0.3 vs
~7.4 on an RTX 3090 for a 30s clip; the CPU default left htdemucs on CPU
even in CUDA builds). crispasr_init_gpu_backend() falls back to CPU on
GPU-less hosts, so the new default is safe everywhere.

CRISPASR_HTDEMUCS_GPU keeps working: =0 forces CPU, =1 forces GPU.
…ant_gpu

want_gpu alone still runs CPU (measured 0% GPU util with only
CRISPASR_HTDEMUCS_GPU=1): the ggml graph path is the gate to the GPU
backend, and FUSED is what makes the graph path beat CPU (no
host<->device roundtrips). CPU-only hosts keep their BLAS path.
CrispStrobe pushed a commit that referenced this pull request Sep 1, 2026
…eparation backends

The Kaggle proof kernel caught it: the --separate CLI passed bare
default params (use_gpu=false) to htdemucs AND mel-band-roformer, so the
new AUTO gates never probed the GPU and every arm ran BLAS — the exact
'CLI adapter forwards use_gpu' trap the workflow notes warn about. The
server and session C-ABI already forwarded; the CLI now does too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0138mFhGiCzAwiNqdtwtq439
CrispStrobe pushed a commit that referenced this pull request Sep 1, 2026
…eal-GPU hosts

Completes PR #414 (@tilllt): their measurements (RTX 3090 Ti — fused GPU
RTF 0.37 vs CPU/BLAS 7.4) motivate the flip, but the PR's gate left the
default on CPU (both env gates default off), made FUSED=1 alone select
the slow per-layer GPU path, and its params.use_gpu||env precedence made
the advertised CRISPASR_HTDEMUCS_GPU=0 opt-out dead (the CLI defaults
use_gpu=true).

src/htdemucs_gates.h: pure resolve() — AUTO picks graph+fused+GPU exactly
when a real GPU backend is present and permitted, BLAS otherwise (CPU
hosts unchanged; per-layer graphs never AUTO-selected — they measured
slower than BLAS). Explicit envs force either way; FUSED=1 implies the
graph it needs; env beats caller intent in both directions. Resolved once
per init against a real GPU probe; one unconditional gates line for
tests/kernels to assert.

Tests: test-htdemucs-gates.cpp locks the 9-case decision table;
test-htdemucs-separate-live.sh proves AUTO-stays-BLAS on CPU hosts,
forced-fused engagement, and per-stem BLAS-vs-fused parity (worst
max|diff| 4.3e-4, q4_k). tools/kaggle/htdemucs-default-gpu-ab adds the
CUDA proof (default-arm engagement + FUSED parity f16/q8_0 + speedup —
the arm #398's kernel did not cover). 1772/1772 unit tests green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0138mFhGiCzAwiNqdtwtq439
CrispStrobe pushed a commit that referenced this pull request Sep 1, 2026
…eparation backends

The Kaggle proof kernel caught it: the --separate CLI passed bare
default params (use_gpu=false) to htdemucs AND mel-band-roformer, so the
new AUTO gates never probed the GPU and every arm ran BLAS — the exact
'CLI adapter forwards use_gpu' trap the workflow notes warn about. The
server and session C-ABI already forwarded; the CLI now does too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0138mFhGiCzAwiNqdtwtq439
@CrispStrobe

Copy link
Copy Markdown
Owner

Merged to main (41e01751, 605d88f7 — your commits with authorship preserved — plus two follow-ups), with the full proof chain. Closing manually since the rebase rewrote SHAs and GitHub can't auto-detect it.

Two review catches fixed on top (d379bbf7):

  1. The gate want_graph = use_ggml() || (want_gpu && use_fused()) left the default on CPU (both envs default off) despite the title — and made FUSED=1 alone select GPU with per-layer graphs, the path your own comment notes is slower than BLAS.
  2. if (params.use_gpu) want_gpu = true after the env read, with the CLI defaulting use_gpu=true, made the advertised CRISPASR_HTDEMUCS_GPU=0 opt-out unreachable.

The gates now live in src/htdemucs_gates.h as a pure resolve(): AUTO = fused-graph-GPU exactly when a real GPU backend is present and permitted, BLAS otherwise (CPU hosts unchanged), explicit envs force either way, FUSED=1 implies the graph it needs, env beats caller intent in both directions. The full decision table is locked by tests/test-htdemucs-gates.cpp (9 cases).

And one bug your PR's goal exposed (208b2d59): the --separate CLI never forwarded use_gpu/n_threads/gpu_device to htdemucs or mel-band-roformer — bare zero-init defaults, so even a correct gate never saw the GPU. Caught by the proof kernel's engagement assert: its first run showed all arms on BLAS with vacuously perfect "parity".

Proof on a real CUDA box (Kaggle P100, chr1str/crispasr-htdemucs-default-gpu-ab, f16 + q8_0, 11 s 44.1 kHz stereo):

CPU/BLAS ref new default
f16 193.8 s 9.8 s 19.8×, gates graph=1 fused=1 gpu=1 engaged by itself
q8_0 191.1 s 9.8 s 19.5×

Per-stem parity default-vs-BLAS: worst cos 0.99968, worst max|diff| 9.2e-5, magnitude ratio 1.000 — same bar as the #398 GPU proof. On CPU-only hosts a live test (tests/test-htdemucs-separate-live.sh) locks AUTO-stays-BLAS plus forced-fused parity (worst max|diff| 4.3e-4 on q4_k).

Your 3090 Ti measurement was spot on — thanks for another contribution that came with real numbers attached.

@CrispStrobe CrispStrobe closed this Sep 1, 2026
CrispStrobe pushed a commit that referenced this pull request Sep 1, 2026
…erged

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0138mFhGiCzAwiNqdtwtq439
tilllt added a commit to tilllt/CrispASR that referenced this pull request Sep 3, 2026
…header

Document CRISPASR_MELBAND_GGML/GPU/FUSED/SEG_S/NO_SEGMENT in
docs/environment-variables.md in the htdemucs-CrispStrobe#414 style (AUTO semantics,
measured numbers, test reference). Add the BSRoformer.cpp cleanroom
attribution note the maintainer asked for in CrispStrobe#415 to the file header.
tilllt added a commit to tilllt/CrispASR that referenced this pull request Sep 3, 2026
…header

Document CRISPASR_MELBAND_GGML/GPU/FUSED/SEG_S/NO_SEGMENT in
docs/environment-variables.md in the htdemucs-CrispStrobe#414 style (AUTO semantics,
measured numbers, test reference). Add the BSRoformer.cpp cleanroom
attribution note the maintainer asked for in CrispStrobe#415 to the file header.
CrispStrobe pushed a commit that referenced this pull request Sep 3, 2026
…h (tilllt)

Refs #415. Cleanroom port following the in-repo htdemucs graph pattern, with
chenmozhijin/BSRoformer.cpp (MIT) used as a correctness oracle only.

Contributed by tilllt: band-split and the RoFormer transformer blocks as ggml
graphs, then a FUSED single graph running band-split + the full 12-block
time/freq stack + the mask estimator on-device in one pass (removing 13
host<->device roundtrips per segment), Demucs-style segmentation with 25%
overlap for long audio, and path selection in src/mel_band_gates.h mirroring
the #414 htdemucs-gates architecture.

Measured by the contributor (RTX 3090 Ti): 30 s clip 28 min CPU -> 4:47
per-layer GPU graphs -> 2.57 s fused; RTF ~0.076 on a 359 s track.

REVIEW (posted on the PR) and what the contributor did with it:
- clang-format: fixed in bdbee25. Re-verified here — 0 violations.
- Segment length disagreed with the checkpoint's TRAINED window: the default
  was a hardcoded 10 s while Kim vocals declares chunk_size 352800 = 8.0 s, so
  at hop 441 the time-transformer's RoPE ran to 1000 positions against the 800
  it was trained on, on every segment of long audio. Fixed in fb91d39, and
  they took the thorough option rather than the cheap one: the converter now
  carries chunk_size into GGUF metadata, the loader reads it, and the segment
  default derives from it, with an 8 s fallback for GGUFs predating the KV.
  I traced that plumbing rather than trusting the message — _load_config()
  returns the dict that merges chunk_size from either the `model` or `audio`
  section, and the writer reads that same dict, so the KV genuinely gets
  written.

Two review findings remain open and are NOT addressed by this merge; they are
recorded here so they are not mistaken for settled:
- SEGMENTATION HAS NO REFERENCE-ANCHORED EVIDENCE. The PR's 30 s arm compares
  per-layer vs fused, but segmentation lives above the gates, so both arms are
  segmented identically and any overlap-add error cancels exactly. golden10s is
  exactly at the threshold (n_samples <= seg_len) so it never segments. I did
  verify the overlap-add MACHINERY independently — replicated the schedule with
  the model as identity over ~200 length/segment combinations including every
  boundary at seg_len+1: zero coverage holes, exact reconstruction (2.4e-07).
  So the adder is sound; what is missing is an end-to-end comparison against
  the Python reference on a >10 s clip.
- The new default round-trips through integer seconds (chunk_size /
  sample_rate), exact for 352800/44100 = 8 but lossy for any checkpoint whose
  chunk is not a whole number of seconds.

Also noted, not a defect: the fused path does not reach bindings or the server
— src/crispasr_c_api.cpp inits with bare default params and never sets use_gpu,
so the gates resolve to the legacy CPU path there. CRISPASR_MELBAND_GPU=1
rescues it.

Merge conflict in tests/CMakeLists.txt was mine, not theirs: c59c615 added
test-sidon-rpe-gates at the same anchor as their test-mel-band-gates. Both
blocks kept.

Verified here before merging: build clean; test-mel-band-gates 30 assertions /
9 cases; test-sidon-rpe-gates 51 / 10; ctest -R mel.band 9/9.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CrispStrobe pushed a commit that referenced this pull request Sep 4, 2026
…st dev profile

TWO UNRELATED FIXES, both small, landed together at the end of the day.

1. THE FUSED GPU PATH NEVER REACHED BINDINGS OR THE SERVER. The mel-band arm of
crispasr_session_open() called mel_band_roformer_init_from_file() with a bare
mel_band_roformer_default_params(), which sets use_gpu=false. The #422 gates
therefore resolved to the legacy CPU path for every C-ABI consumer, so the fused
graph (RTF ~0.076) was unreachable outside the CLI and those callers got CPU
RTF ~57 — the whole 112x win stopped at the CLI boundary.

This arm was the lone outlier among its neighbours: htdemucs immediately below
and rvc/omnivoice adjacent all forward g_open_use_gpu_tls, and htdemucs even
carries the comment explaining why (crispasr_session has no use_gpu member).
Same "encoder gap" shape as the #414 CLI catch that PR fixed. Found during the
#422 review; deliberately held for hours because the box had SwapFree at 0-16 kB
and two Claude sessions had already been OOM-killed.

VERIFIED: builds clean. NOT verified behaviourally — this box has no GPU, so
the gates cannot resolve differently here regardless. The change is a
one-for-one match with three sibling arms, which is an argument, not a
measurement.

2. [profile.dev] debug = "line-tables-only" for the Rust crates.

/mnt/volume1 hit 99% today. Rust target/debug dirs held 24.6 GB across the box,
essentially all debug artifacts. Measured with `readelf -S` on a real 187.9 MB
dev-profile test binary (no rebuild — an existing artifact answers it):

    ALL .debug_*      155.1 MB   82.5% of the binary
    std's fixed floor   ~3.9 MB  (precompiled rlibs; the profile cannot touch it)
    profile-governed   ~151 MB

    dev default      187.9 MB
    line-tables-only  ~60 MB   -68%
    debug = 0         ~36.7 MB -80%

line-tables-only ALONE. Two traps, both hit during the investigation:

  - `strip = "debuginfo"` alongside it CANCELS it. strip removes DWARF sections
    and .debug_line IS one, so the pair lands within 200 bytes of debug=0 with
    zero .debug_line sections. My first recommendation had both and was wrong;
    it went to four sessions before being caught.
  - The ~1.5 GB/repo that debug=0 would additionally save is not worth losing
    per-frame `at ./src/...` backtrace lines. `panicked at file:line` survives
    either setting (it is panic location metadata, not DWARF), so debug=0 is not
    blind — but the frames beneath go.

Measurement credit: crispasr-4a/kerotakis-5f/bw-fixes-1 across sessions. Also
worth recording: an initial hello-world measurement said line-tables-only saves
0.1% and was off by ~700x, because std's untouchable floor IS the whole binary
on a toy. For "what would stripping save", readelf an artifact from the class
you care about; a minimal repro is the worst possible choice.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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