Skip to content

nemotron_duplex: live calls at one-shot cost (full duplex 5/6) - #595

Merged
dghosef merged 78 commits into
duplex/nemotron-livefrom
duplex/perf
Oct 27, 2026
Merged

dghosef merged 78 commits into
duplex/nemotron-livefrom
duplex/perf

Conversation

@dghosef

@dghosef dghosef commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

Stack: full duplex (#186), part 5 of 6. Stacked on #594. It also carries #816's 20 commits (bounded KV), cherry-picked onto this stack's base. #816 merged on 2026-10-23, so they leave the diff once main is merged into the stack. The whole stack is listed at the end.

What this does

This PR makes live calls cost close to what one-shot calls cost.

Profiling on one H100 found that the difference was the duplex encoder, not the engine or the rest of the model. One-shot encodes a call once. Live mode steps the encoder on every 80 ms append, and each step was about 1,000 small eager kernels plus per-call Python. At 32 calls that cost 16–24 ms of GPU-thread host time, on a worker that is CPU-bound. LLM, talker and codec steps cost the same in both modes.

What changed in this revision

This revision adds commit 7, plus #816.

The encoder's attention window moved off the model and onto the engine's bounded-KV resource (#816).

  • Before: each layer's keys and values for the last 70 frames sat as an opaque block in ENCODER_STATE. The model gathered that block, appended the new frame, scattered it back, and ran its own relative-position attention (_attend / _mask).
  • Now: the encoder declares ENCODER_KV (a BoundedKVSpec, per-query window 70, one slot per session). Each step declares its calls' positions and new frames. _attend calls kv.attend(q + u, k, v, rel_bias=...). The resource stores the window and runs the attention (Triton).
  • Deleted: the K/V block, the mask, the gather / scatter, and the regions' frames input.
  • Why: invariant 1 says a KV cache and its attention belong to a declared resource, not to model code. It was also this PR's listed follow-up.

Most of the performance numbers below are from before this change, and were not re-measured in full. The one check on this revision is a short interleaved A/B on one H100 ("This revision" under "How it was tested"). It has 2 runs per arm, and no profiling pass on the new arm. It shows the new encoder level with the old one on live calls alone, and ahead with uploads mixed in. That's what I'd expect, since the per-layer gather / scatter of the whole window is gone. But two runs is not a verdict. The correctness gate was rerun on the stack's steps.

How it works

The PR is seven commits.

  1. Capture the streaming encoder's layer stack.
    • Each call's layer state moves into engine resources, one slot per session:
      • the depthwise-conv inputs go into a RecurrentStatePool (ENCODER_STATE);
      • since commit 7, the keys and values of the last 70 frames go into the bounded KV (ENCODER_KV).
    • The layer stack runs over counts kept on the device. So a step where every call has at most one new frame (every live step) replays a piecewise CUDA graph (conformer_layers).
    • Steps with more frames (a one-shot clip, a large append, the end of a call) run the same function eagerly.
    • RecurrentStatePool gains gather / scatter_, the same methods as MiniCPM-o 4.5: speech output, CUDA graphs, fused vocoder, benchmark entry #383.
    • Per-message deflate is turned off on /generate/ws. PCM barely compresses, and deflate cost the event loop about 8% at 32 calls.
  2. Capture the codec's steady-state decode. The steady window (3 context frames plus one 5-frame chunk) replays a graph per batch bucket (codec_decode). Calls that start at different times reach chunk boundaries on different frames. So a live load runs about 5× as many small decodes as one where all calls run in lockstep.
  3. Codec fix: build the iSTFT window envelope once per shape. _spec_to_wav used to assert the envelope on every decode. That was a host sync on every codec step, one-shot included. It also made the decode impossible to capture.
  4. A bounded front end in a pool, and a live step as one graph.
    • A step reads only a bounded window of each call: its last frame of audio and its last 16 mel frames. These live in a second pool (ENCODER_FRONT). Host counters replace the buffers that used to hold the whole call.
    • When calls append exactly one frame of audio, on the frame grid, the whole step runs as one graph (conformer_step): preemphasis, STFT, mel, subsampling and layers.
    • Other calls in the same step take the eager path. That path is bitwise equal to nemotron_duplex: live input over a client stream, cache-aware streaming encoder (full duplex 4/6) #594's streamer in fp32. Through the graphs, outputs differ by under 4e-7.
  5. Keep cuFFT out of the captured graphs.
    • torch's cuFFT plan cache is an LRU, and its eviction destroys plans. Graph replays don't touch the cache. So once there were enough newer eager FFT shapes, the plans that conformer_step and codec_decode replay got destroyed (an illegal memory access on H100).
    • Both regions now do their fixed-size transform as a float64 matmul against a precomputed real-DFT basis.
    • One-shot text is unchanged. An upload runs the encoder's eager path, which keeps torch.stft.
    • One-shot codec audio is not unchanged. The Talker→Codec edge is a 5-frame FixedChunkPolicy in every mode. So after a call's first chunk, every 8-frame window replays codec_decode (_decode_region → codec.decode(by_matmul=True) → irfft_dim1), one-shot calls included. The first chunk, and any window with graphs off, keep irfft. So one-shot PCM is no longer bitwise identical to nemotron_duplex: live input over a client stream, cache-aware streaming encoder (full duplex 4/6) #594's. How far it differs on served one-shot audio is not measured.
    • Component check before the rebase (torch 2.12.1+cu132; the run's record doesn't name the GPU): on 8-frame windows at batch 32, captured codec_decode and the eager decode differ by at most 1.8e-7, or one int16 step, in fp32 and in bf16.
    • nemotron_duplex: per-frame codec in the talker's step (full duplex 6/6) #770 makes the matmul iDFT the only path for served codec audio, eager included.
  6. Long chunks leave a batch through the scheduler hook, not inside the forward.
    • can_batch (with input_seq_len in samples) keeps a chunk over 1 s (an upload, a large append) out of a batch. The engine then runs that step one call at a time.
    • RecurrentAddressing.narrow, which let the forward split rows itself, is removed.
    • The trade-off: a step that holds a long chunk now runs its live calls one at a time too. Each one still replays its captured region at batch size 1. A load run with uploads sharing steps with 32 live calls finished every call, but what this costs their latency is not measured. The general fix is batch grouping in the engine (see "Follow-ups").
  7. The attention window on bounded KV (see "What changed in this revision").
    • Positions come from each call's frame count on the host. A step declares each call's new frames. The captured regions declare their own rows, and padding rows are skipped.
    • A steady call takes the whole-step region only when two things hold: that region has a graph for the steady calls, and the step's eager plan goes unused (every other call's frames go through the layer region, or there are none). Otherwise it runs eagerly with the rest. This fallback matters when a capture bucket was dropped. The cost is that a steady call sharing a step with a call making several frames runs eagerly.
    • The front end counts a call's new frames once, in prepare_inputs. The declaration and the forward both read that count.
    • validate_config_yaml counts encoder_kv in the session-capacity check.

Engine changes

Outside nemotron_duplex and the API server (the deflate setting), the engine changes are #816's bounded KV (merged; carried here until main is merged into the stack), and these in the recurrent pool:

  • RecurrentStatePool.gather / scatter_ (commit 1). They refuse padding rows on a pool without a sink slot.
  • RecurrentBlockConfig.dtype may be None, meaning the engine's compute dtype.
    • RecurrentStatePool.build resolves it in place to info.kv_dtype (torch's default dtype if that is unset). This is the same way it already applies shard to the spec's config.
    • engine_manager passes kv_cache_type=autocast_dtype, so this is the autocast dtype the layers compute in.
    • The encoder's layer state uses it. If the state were stored in another dtype, a call's frames would drift from the offline ones.
    • Any pool can now declare a block this way.
  • The comment on RecurrentAddressing.num_rows now says it includes a replay's padding rows, which is what plan already sets.

How it was tested

This revision

0b7b7de9c: commit 7 on top of #816's commits. One H100 80GB HBM3, torch 2.12.1+cu132 (CUDA 13.2), Triton 3.7.1, mstar_rust built from this tree.

  • ruff check .: clean.

  • CPU Core (OMP_NUM_THREADS=2, every CPU_TEST_PATHS directory, without xdist; see below for why): 6864 tests, 4 failed. The previous head 246222c91 in the same environment fails the same 4: two test_eval_compare.py process-stop cases, test_e2e_validators.py's SIGHUP case, and test_serving_metrics.py's case, which needs prometheus_client.

  • On CUDA: the bounded-KV tests, 89 passed. Together with the nemotron_duplex submodule and model tests, 221 passed. They include a step that mixes the whole-step region with an eager chunk longer than the window, and a dropped capture bucket falling back to eager.

  • Correctness gate (correctness.py, outside the repo, against main abdfd5802). Steps: configs, encoder parity, main a/b, stack a/b, graphs off, edge cases, and main's standalone reference for margins.

    • Encoder parity passes.
      • Frame counts equal offline.
      • fp32 streaming vs main's offline fp32: min cosine 0.9999996.
      • bf16 is within 0.99x the yardstick on every clip and chunk size.
      • The new encoder is slightly closer to fp32 than the previous head on every clip (e.g. turn_taking whole-clip mean |d| 6.35e-4 vs 6.48e-4).
      • Both regions capture every bucket and replay.
    • Batching up to 32 calls, graphs off vs on, live whole-clip append vs upload, and every edge case: pass.
    • One-shot text vs main fails on two frames. As far as the evidence goes, both are bf16 knife-edges:
      • interruptions frame 329 (, vs ., margin 0.375). A near-tie, the same as on the previous head.
      • turn_taking frame 204. An upload drops a "Here" that main and live calls emit. Margin 1.31 in main's standalone run, just over the gate's 1.0 near-tie bar.
      • Frame 204 is where main's own output flipped between earlier runs (see "Earlier revisions"). Main's served output already diverges from its standalone reference at frame 47 of that clip (margin 8.1). So the margin is only a rough guide.
      • Likely cause (hypothesis, not shown): an upload's long eager pass used to round like main's offline encoder, and now runs the bounded-KV kernel.
  • Live A/B. Interleaved fresh servers, 246222c91 vs this revision. First 32 paced calls spread over 2 s. Then the same with a 20 s upload every 2 s and 36 slots, so uploads share steps with live calls. Lag p50 / p95 / p99, then drain p50:

    load 246222c91 this revision
    32 live calls 65 / 142 / 266 ms, 0.49 s 63 / 143 / 289 ms, 0.51 s
    + uploads, run 1 165 / 378 / 1594 ms, 2.09 s 133 / 263 / 1333 ms, 1.79 s
    + uploads, run 2 138 / 304 / 1594 ms, 2.24 s 91 / 217 / 1283 ms, 1.79 s

    0 failed calls and 21/21 uploads on both. With two runs per arm and no profiling pass on the new arm, read this as "no regression seen", not as a measured speedup.

Previous head 246222c91

This head includes #766's and #593's fixes. CPU only, on a 24-vCPU Xeon Platinum 8488C, Python 3.12.13, torch 2.12.1+cu132, pytest 9.1.1, with mstar_rust built from this tree.

  • ruff check .: clean.
  • CPU Core (OMP_NUM_THREADS=2 python -m pytest <CPU_TEST_PATHS> -p xdist -n 4): 6154 passed, 3 failed, 612 skipped. Main abdfd5802 in the same environment: 6061 / 3 / 612, with the same 3 failures.
    • The 3 failures come from the box, not the code. test_serving_metrics.py::test_refusals_count_from_when_metrics_are_turned_on needs prometheus_client, which [all] doesn't install (CI installs [metrics]). The two test_eval_compare_multigpu.py::test_dump_stacks_retries_with_sudo_only_when_asked cases find no processes under nohup + xdist. All three pass on main once the environment is fixed.
    • test/modular/test_piecewise_config_signature.py ran outside xdist. It parametrizes over a set, so xdist workers collect it in different orders (on main too). It passes.
  • The recurrent pool's padding-row tests: 8 passed.
  • One-shot codec audio against nemotron_duplex: live input over a client stream, cache-aware streaming encoder (full duplex 4/6) #594 (commit 5): not measured.
  • GPU: not rerun on this head. The latest GPU run (below) is of the previously pushed head, before Non-blocking sends from the preprocess thread (full duplex 2/6) #766's ff5002e7e, Client streams: the client as a streaming edge into a running request (full duplex 3/6) #593's 0ed426659 and this PR's docstring fix e4030eefb. None of them changes nemotron_duplex's code.
  • CI only runs on PRs into main, so it will run on this PR once the PRs below it merge.

The correctness gate harness

correctness.py, kept outside the repo. It runs one-shot vs main, live vs one-shot, batching up to 32 calls, graphs off, and the protocol edge cases. Each is checked against criteria fixed before the run. The H100 subset below ran correctness.py all --steps prep,p0_configs,srv_main_a,srv_main_b,srv_stack_a,srv_stack_b,srv_stack_eager,srv_stack_edge,p8_gate.

Latest GPU run of the previous head

One H100 80GB HBM3, torch 2.12.1+cu132 (CUDA 13.2). The whole stack through #770 at its previously pushed head, against main d519553db.

  • Correctness gate: every row that ran passes, except one. One-shot text vs main on interruptions differs at frame 329 (, vs .). That is the same kind of bf16 knife-edge as before. The stack's output there is unchanged from the previous revision's run, and main's own output flips at nearby frames between H100 and H200.
    • Not run this time: encoder parity, served vs standalone, the cuFFT check, listening.
  • Every region captures at batch buckets 1–32 (conformer_step, conformer_layers; codec_decode is gone in nemotron_duplex: per-frame codec in the talker's step (full duplex 6/6) #770), with no bucket dropped.
  • New edge rows pass:
    • the append record and its shorthand stream end to end;
    • a text append to nemotron_duplex ends the call with a 400;
    • end_input: ["audio_in"] ends the call the same as true;
    • an unknown stream on an append or on an opening gets a 400;
    • a live call past 160 s gets a 400 naming client_streams.audio_in.max_items (it used to be a 500).
  • Load: 32 paced live calls plus a 20 s upload every 2 s, with 36 slots so uploads share steps with live calls. 32/32 calls and 21/21 uploads finish with no error, and every reply has words.
  • Not measured: what running a step per request costs live calls when the step holds a long chunk (commit 6; see the batch-grouping follow-up).
Earlier revisions
  • Previous head, on the H100 box: CPU Core (CI's command) failed exactly main's 4 environment tests. Those are test_nohup…, test_ctrl_c… and test_sigterm… (timing-flaky on both trees), and test_refusals… (needs prometheus_client). 6135 passed and 607 skipped, against main's 5887 passed.
  • Previous revision, after the rebase, on one H200 (torch 2.11.0+cu128, CUDA 12.8). The whole stack through nemotron_duplex: per-frame codec in the talker's step (full duplex 6/6) #770 (321763dd8, which differs from the heads pushed then only in test files, a startup guard and comments), against main d519553db, with the same gate harness:
    • Every required row passes except two that don't test the stack:
      • One-shot text vs main on 2 of 3 clips. Main's own output moved at near-tie frames since the earlier run (frame 204, margin 1.375; frame 205, margin 0.0), while the stack's is unchanged.
      • The cuFFT plan-eviction check. It exercises raw torch.stft / irfft, and the stack's captured regions run no FFT.
    • Rows that pass include:
      • 80 ms live appends give the one-shot text (apart from the last frame and one near-tie);
      • batching up to 32 calls;
      • graphs on vs off are identical;
      • the 11 protocol edge cases: hang up before any audio, append after end_input, append from another socket, the input budget, the session cap, abort mid-call and recovery, 409 on a live id, null fields, and upload channel / rate / length refusals;
      • Whisper WER of the agent's audio 0.140 (main 0.202).
    • Live calls: 0 failed over 5 clips x 6 calls and a 1–32 session sweep.
  • Before the rebase (one H100, torch 2.12.1+cu132, CUDA 13.2):
    • ruff check .: clean.
    • CPU core in the CI image: 1633 passed, 16 failed. The 16 are the same set main failed in the same container. None were new.
    • GPU, 1×H100 (the bitwise check ran before commit 5, the matmul DFT):

Performance

Measured before commit 7 (see "What changed in this revision", and "This revision" under "How it was tested").

All runs are on one H100, torch 2.12.1+cu132, with MSTAR_SHM_ARENA=1, before the rebase. A/B arms were interleaved, on one server process each, and repeated.

Against vLLM-Omni and SGLang-Omni

Setup:

  • One H200. One engine at a time.
  • mstar is this PR before the rebase onto d519553db (48f8f69ed with Keep a fresh request's final stream chunk in a speculative step (full duplex 1/6) #769's fix, on main fa37d227c), configs/nemotron_duplex.yaml, MSTAR_SHM_ARENA=1.
  • Identical int16 input (the same sha256 per clip on every engine), sent in 80 ms appends on an absolute schedule.
  • The same system prompt on every engine. SGLang-Omni refuses instructions, but its built-in default is the same prompt.
  • Each engine's shipped duplex config: vLLM-Omni 0.30.0 nemotron_labs_voicechat_duplex.yaml, SGLang-Omni 0.1.8 examples/run_nemotron_voicechat_duplex.py.
  • 3 fresh server processes × 5 calls per clip, per engine.
  • Harness: compete.py (phases single and capacity, paced client rt_client.py), kept outside the repo. Its verdict rules were fixed in its docstring before the run:
    • a latency difference counts only if the medians are further apart than the mstar build's own round-to-round range (floor 10 ms), and the rounds don't overlap;
    • and only if both engines pass its quality gate: pooled Whisper large-v3-turbo WER of the engine's audio against its own text of at most 0.2.

Response latency: from the user stopping to the first voiced agent audio reaching the client (s, median over server processes).

clip this PR vLLM-Omni SGLang-Omni
demo_8s 2.39 2.20 8.08
turn_taking 1.14 0.95 11.92
interruptions 1.06 0.95 12.17
tool_call 1.10 1.06 14.20
u1_8s 1.38 0.95 11.90
  • No latency verdict. Every engine failed the quality gate (pooled WER: mstar 0.300, vLLM-Omni 0.307, SGLang-Omni 0.326). So compete.py excludes every latency cell and makes no latency claim against either engine. The medians above are what was measured: mstar was 0.04–0.43 s behind vLLM-Omni on every clip.
  • The model answers within 2 frames of vLLM-Omni's on every clip (27 vs 27 on demo_8s). mstar's server lag p50 is 0.206 s here, against vLLM-Omni's 0.010–0.090 s. That points at mstar's 5-frame codec chunks. nemotron_duplex: per-frame codec in the talker's step (full duplex 6/6) #770 decodes one frame per step inside the talker, and its build measured 0.91–2.15 s on the same pod (see nemotron_duplex: per-frame codec in the talker's step (full duplex 6/6) #770; also no verdict).

Real-time sessions on demo_8s with this PR, as compete.py judges them (every session finished in every row):

start 1 4 8 16 32
lockstep yes missed the Whisper spot check only missed the Whisper spot check only yes no (heard p50 3.25 s, 15.5% of packets late at 160 ms)
staggered over 2 s not run yes yes yes no (79.5% late)
  • So compete.py's sustained count (the largest N where every N' ≤ N is in real time) is 1 lockstep / 16 staggered.
  • vLLM-Omni as shipped refuses a second session. With its multi-session patch (#8597 upstream, profile at 8) it sustains 1 lockstep / 2 staggered. compete.py marks a capacity claim against it as not shown. The lockstep counts are both 1.
  • SGLang-Omni sustains 0 / 0, both as shipped and with draft #2694 at --max-sessions 8 (with or without --talker-attention triton). compete.py calls this PR ahead of it.
  • With the default MSTAR_SHM_ARENA=0, only 1 and 32 sessions were run. 1 keeps up, 32 doesn't.

Caveats:

  • Measurement quirks:
    • vLLM-Omni's stream carries more audio than the input.
    • SGLang-Omni's DeepGEMM needed NVCC 13, so CUDA 13.0's nvcc was installed for it alone.
  • Multi-session upstream: both projects only have it unofficially, as vLLM-Omni #8597 and SGLang-Omni draft #2694.

Follow-ups

  • Batch grouping in the engine. When can_batch refuses a batch, Engine.exec falls back to running one request at a time. It should split the batch into groups instead, each a batched sub-step with its own region leases. Then a step holding one long chunk would run that chunk alone and keep the rest batched. The encoder would declare a chunk over 1 s as its own group. Flux2 Klein, LTX-2.5 and Chatterbox fall back to per-request on mixed inputs today, and would gain too.
  • Per-call memory is sized for the longest call, not for what attention reads.
    • The talker's and the LLM's KV pools are sized so every session can reach max_session_s (160 s) at the same time. The talker pool alone is 1,024 pages of 128 positions, about 28 GiB for 32 sessions. This PR sizes and checks nano_kv the same way.
    • A call's pages are only freed when it ends. So the pools hold the worst case whether calls last 10 s or 160 s, and raising max_session_s lowers how many sessions fit.
    • The engine's windowed retention (RetentionPolicy, mstar/engine/windowing.py) can free pages behind a window. But it only helps layers that read a window. The LLM's attention layers and the talker's full-attention layers read the whole call, and the talker's sliding window (7,500 positions) is longer than any session.
    • So this is a memory-per-session trade-off, not a bug. Making long calls cheap would need the model to run with a window over old context, which changes its output. Not addressed in this stack.
  • RecurrentStatePool.gather / scatter_ here have the same names and signatures as MiniCPM-o 4.5: speech output, CUDA graphs, fused vocoder, benchmark entry #383's. Whichever lands second keeps this PR's check for padding rows that a pool without a sink slot can't address.

Stack

  1. Keep a fresh request's final stream chunk in a speculative step (full duplex 1/6) #769: worker: keep a fresh request's final stream chunk in a speculative step.
  2. Non-blocking sends from the preprocess thread (full duplex 2/6) #766: the preprocess thread's non-blocking sends.
  3. Client streams: the client as a streaming edge into a running request (full duplex 3/6) #593: client streams.
  4. nemotron_duplex: live input over a client stream, cache-aware streaming encoder (full duplex 4/6) #594: nemotron_duplex live input and a cache-aware streaming encoder.
  5. This PR: duplex performance.
  6. nemotron_duplex: per-frame codec in the talker's step (full duplex 6/6) #770: the per-frame codec in the talker's step.

🤖 Generated with Claude Code

dghosef added a commit that referenced this pull request Oct 20, 2026
conformer_step captured torch.stft (steady_step's mel) and codec_decode
captured torch.fft.irfft (the codec's iSTFT). torch's cuFFT plan cache is a
per-device LRU whose eviction calls cufftDestroy, and graph replays don't
touch the LRU, so enough newer eager FFT geometries (eager front-end windows,
uploads, eager codec windows) destroy the plans the graphs replay.

Both regions now take their fixed-size transform as a float64 matmul against a
precomputed real-DFT basis (components/dft.py): the STFT basis folds in the
checkpoint's window and is built when the streamer is (after load, before
capture); the n=16 inverse basis is cached per device and first built in the
region's eager warmup. float64 keeps TF32 and autocast out of it. The eager
paths keep torch.stft / irfft, so one-shot output stays bitwise #594's and
main's Perception, and the eager streamer stays bitwise #594's.

GPU, 1x H100 PCIe, served checkpoint, regions captured as the engine does:
- after cufft_plan_cache.clear() + eager FFT churn + poisoned free memory, the
  #595 head hits an illegal memory access on its next conformer_step replay;
  with this change encoder frames (2 clips, single and batched) and codec
  windows (bs 32) are bitwise unchanged, fp32 and bf16.
- matmul vs fp64 reference: STFT within fp32 rounding (torch.stft fp32: 1e-6 on
  values up to 16); irfft 3e-8 (torch fp32: 2.4e-7); same under TF32 and bf16
  autocast.
- encoder, captured vs main offline: fp32 min cos 0.9999996 (C1.2 bar
  0.9999); bf16 mean|diff| 0.99-1.01x the bf16-vs-fp32 yardstick (bar 2x).
  Captured vs eager in fp32: max 4.7e-7 (was 2.3e-7 through cuFFT).
  Codec captured vs eager: 1.8e-7, at most one int16 step.
- graph replay at bucket 32: conformer_step 8209 -> 8226 us (bf16), codec_decode
  18657 -> 18669 us (bf16): +0.2% / +0.1%.

Out of scope: an engine-level guard against FFTs in captures; chatterbox's
captured vocoder on main has the same exposure.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dghosef dghosef changed the title nemotron_duplex: live calls at one-shot cost (full duplex 3/3) nemotron_duplex: live calls at one-shot cost (full duplex 5/6) Oct 21, 2026
@dghosef
dghosef marked this pull request as ready for review October 21, 2026 04:06
Comment on lines +1087 to +1088
if runner is not None and tf == self._steady_window and runner.can_run(len(idxs)):
wav = runner.run(static_inputs={"codes": batch}, real_bs=len(idxs))["wav"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] One-shot audio also goes through the captured matmul iDFT, so the description's "the eager paths keep torch.stft / irfft, so one-shot output is unchanged" doesn't hold for the codec.

The Talker→Codec edge is FixedChunkPolicy(chunk_size=codec_chunk_frames) in every mode (nemotron_duplex_model.py:750). So in a one-shot call, every chunk after the first has a 3 + 5 = 8-frame window (_steady_window). That window takes this branch and runs _decode_region → codec.decode(..., by_matmul=True) → irfft_dim1 (a float64 matmul against irfft_basis), not torch.fft.irfft. The encoder side does keep one-shot on the eager path, because an upload runs alone in front and the layers run eagerly. One-shot PCM, however, now comes from a different transform than #594's. The "bitwise identical to #594" GPU check is listed under "Before the rebase", so it is unclear whether it ran with commit 5. Please either re-run the one-shot audio comparison on this head and report the result, or change the claim to the measured difference.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. After a call's first chunk, every 8-frame codec window replays codec_decode → codec.decode(by_matmul=True) → irfft_dim1, one-shot calls included; only the encoder keeps one-shot on the eager torch.stft path.

Description corrected (commit 5): one-shot text is unchanged; one-shot codec audio is no longer bitwise identical to #594's, and that difference on served audio is not measured. The "bitwise identical to #594" check is listed as having run before commit 5. The only measurement is component level, from before the rebase: captured vs eager decode on 8-frame windows at batch 32 differ by at most 1.8e-7, one int16 step, in fp32 and bf16. #770 then makes the matmul iDFT the only path for served codec audio.

@github-actions

This comment has been minimized.

@dghosef
dghosef marked this pull request as draft October 21, 2026 05:12
@dghosef
dghosef marked this pull request as ready for review October 21, 2026 05:34
@github-actions

This comment has been minimized.

@dghosef
dghosef marked this pull request as draft October 21, 2026 07:19
dghosef and others added 2 commits October 21, 2026 02:51
A live step at 32 calls spent ~19 ms (in isolation; ~50 ms beside the
worker's other threads) launching the encoder's ~1000 eager kernels, which
put the live path past the 80 ms tick from 24 calls up. One-shot encodes
a call once, so it never paid this.

Each call's layer state (attention keys/values, conv inputs) moves into a
RecurrentStatePool (ENCODER_STATE), and the layer stack runs over device
counts, so a step where every call has at most one new frame (every live
step) replays it as a piecewise CUDA graph. Steps with more frames (a
one-shot clip, a large append, the call's end) run the same function
eagerly. RecurrentStatePool gains gather/scatter_ (as in #383), and its
addressing gains narrow: a chunk of more than a second still runs apart
from the batch (no other call is padded to it), now on its own row of the
step's plan.

Also turns off per-message deflate on /generate/ws: PCM barely
compresses, and it cost the event loop ~8% at 32 live calls.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Live calls start at different times, so their 5-frame codec chunks come
due on different frames: at 24 staggered calls the codec ran ~5x the
steps of a lockstep load (168 vs 32), ~4 rows each, every one an eager
decode. A steady-state window (3 context frames + one chunk) now replays
a piecewise CUDA graph per batch bucket; other window lengths (a call's
first chunk, the flush) stay eager.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment on lines +5 to +6
conformer_encoder encoder_state, 16 kHz speech -> per-frame LLM embeds (+ RNN-T); per
encoder_front session, its layers' state and its audio / mel tails

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] The module docstring doesn't mention encoder_kv, the third resource commit 7 added to the encoder.

The node table lists conformer_encoder as declaring only encoder_state and encoder_front. A few lines down (line 15), "The encoder keeps a call's state in its two pools" says the same. But get_node_resources now also gives the node ENCODER_KV, which holds the attention window and runs the attention. That is the table this docstring claims to mirror, and it is the commit's main change (AGENTS.md S2, stale comments). Line 15 needs the same fix, e.g. "in its bounded KV and two pools".

Suggested change
conformer_encoder encoder_state, 16 kHz speech -> per-frame LLM embeds (+ RNN-T); per
encoder_front session, its layers' state and its audio / mel tails
conformer_encoder encoder_kv, 16 kHz speech -> per-frame LLM embeds (+ RNN-T); per
encoder_state, session, its attention window, its layers' conv state
encoder_front and its audio / mel tails

@mstar-review

mstar-review Bot commented Oct 22, 2026

Copy link
Copy Markdown

Automated review

🔴 1 P1, 1 P2 on 0b7b7de, light review

Commit 7 moves the encoder's attention window onto the bounded KV (#816). The earlier one-shot codec P1 and the stale select docstring P2 were answered and fixed. This pass found one new P2: the submodules module docstring still lists two encoder resources, not three.

Description

What the review checked
  • The front end and the declaration count frames the same way. new_frames in prepare_inputs gives exactly 1 for a steady call (S = (k-1)·1280 → mels 8k-1 → frames k), which matches what steady_step advances. So the outer step's KV commit (written = pos + span) agrees with st.frames at the next admit, and that admit's pos.written == req.written check passes.
  • Mixed steps: the step region serves the steady calls only when the outer step's KV plan goes unused, i.e. the other calls have no frames or all go through conformer_layers. The region and the outer step both commit pos.written + 1 from the same declared position, which BoundedKVManager.commit says is harmless to do twice.
  • _region_kv_step reads each call's position before advance and before the st.frames += 1 after the step region runs. Padding and capture rows get no position, and admit/plan treat them as EMPTY_ROW.
  • An all-steady step (declare_step returns None) only happens under a STEP_REGION lease. Its calls already hold KV, front and state slots from earlier non-steady steps, so the region's admit takes none fresh.
  • conformer_layers runs after the outer step has admitted every call, so its calls hold slots. It declares span 1 only for calls with exactly one new frame.
  • prepare_inputs reads StreamState on the GPU thread after the previous forward returned. The encoder has no whole-forward graph, so pre_plan_for_batch never declares an encoder step ahead of it.
  • validate_config_yaml checks BoundedKVSpec.max_slots (no sink) against max_concurrent_requests, alongside the recurrent pools and the KV page budgets.
  • ws_per_message_deflate is documented in docs/serving.rst and defaults to uvicorn's own default (on).

Not reviewed: mstar/engine/resources/kv/bounded/* (#816's commits, carried here; reviewed on #816), test/modular/test_bounded_kv.py, test/modular/test_nemotron_duplex_submodules.py, test/modular/test_nemotron_duplex_model.py, mstar/model/nemotron_duplex/components/audio_codec.py (unchanged since the previous review).

Advisory, not a merge gate. Rules: AGENTS.md. Reply to discuss.

dghosef and others added 2 commits October 21, 2026 18:26
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vasilevklart
vasilevklart previously approved these changes Oct 26, 2026
@vasilevklart
vasilevklart dismissed their stale review October 26, 2026 03:52

Posted by mistake

@Gaurav-Shah05
Gaurav-Shah05 self-requested a review October 26, 2026 06:02

@Gaurav-Shah05 Gaurav-Shah05 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I only have a nit added below

# each call's step on its own (declare, admit, plan, forward, commit).
# Such chunks are uploads and large appends, so it is a rare step.
longest = int(self.BATCHED_CHUNK_S * self.perception.config.sample_rate)
return len(model_inputs) == 1 or all(inp.input_seq_len <= longest for inp in model_inputs)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since can_batch runs after the rows are picked, one chunk over the untuned 1 s BATCHED_CHUNK_S runs every live call in its step one at a time. I think the batch-grouping follow-up belongs in the scheduler's capture groups instead of Engine.exec, which needs capture groups to work without a whole-forward capture and to see each chunk's length.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. This will go in a followup

dghosef and others added 15 commits October 26, 2026 12:00
Conflicts:
- mstar/engine/resources/kv/bounded/config.py, manager.py, test/modular/test_bounded_kv.py (add/add): took main's, which is this branch's bounded KV as it landed in #816 plus the max_span and max_slots checks; the branch-only BoundedKVConfig.slot_bytes had no caller.
- mstar/engine/resources/recurrent/pool.py: main's gather / scatter_ (#891), which follow the step's plan by label and read a row with no saved state as zeros (or `start`), replace this branch's (name, rows, layer) version; from_spec keeps this branch's dtype=None resolution and passes main's resource-key name; main's has_state comment.
- docs/adding_models.rst: kept the bounded-KV paragraph's encoder example; the two RecurrentStateSpec rows (this branch's, describing the old gather API, and main's) are now one row.

Adapted to main (no conflict markers):
- mstar/model/nemotron_duplex/components/conformer.py, submodules.py: the encoder's front end and layer stack call main's gather / scatter_ without an addressing argument. The rows / front_rows parameters and the _pool helper that threaded them are deleted. Every slot those calls read is admitted zeroed, so main's zero-fill for rows with no saved state reads the same values; padding rows read zeros now instead of the sink, and their outputs are discarded.
- test/modular/test_recurrent_padding_rows.py: dropped this branch's four tests of the old gather / scatter_ (main's test_recurrent_gather_scatter.py covers the landed API).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#891's gather / scatter_

Main's RecurrentStatePool.gather / scatter_ (#891) take no addressing: they read the plan the pool holds when they run, so the test's wrapper, written for the (name, rows) API, broke the call (TypeError) rather than checking anything.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

# Conflicts:
#	configs/nemotron_duplex.yaml
#	configs/nemotron_duplex_disagg.yaml
… gets a 503

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t back

build() wrote the engine's kv_dtype into the model's RecurrentStateConfig,
which its specs (and a second engine) share. Resolve it into the pool's
own blocks instead, as the bounded KV does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The config's byte counts size a dtype=None block in torch's default
dtype now that the pool no longer writes its resolved dtype back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The factory read the block's declared dtype from the shared config, which
is None for a block in the engine dtype since P3a. RecurrentStatePool.
block_dtype is the one rule the pool allocates by, and the factory asks it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
numel, nbytes, slot_bytes and total_bytes had no caller since the pool
sizes its startup log from its own tensors (P3a), and were wrong for a
dtype=None block. Their only test restated them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The rule moves from the static RecurrentStatePool.block_dtype to
RecurrentBlockConfig.resolved_dtype, which the pool and the linear-attention
builder both call, so linear_attn/base.py no longer imports the pool. The
to_blocks state_dtype hints admit None (the engine's dtype).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts: none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mstar-review mstar-review Bot added size/L and removed size/XL labels Oct 27, 2026
@dghosef
dghosef merged commit 8750409 into main Oct 27, 2026
7 checks passed
@dghosef
dghosef deleted the duplex/perf branch October 27, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants