Skip to content

feat: Triton fused SoftSignGLU kernel for LYNXNet2 - #60

Open
KakaruHayate wants to merge 22 commits into
mainfrom
feat/fused-softsign-glu
Open

feat: Triton fused SoftSignGLU kernel for LYNXNet2#60
KakaruHayate wants to merge 22 commits into
mainfrom
feat/fused-softsign-glu

Conversation

@KakaruHayate

@KakaruHayate KakaruHayate commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Fuse nn.Linear + SoftSignGLU into single Triton kernel launches, reducing HBM traffic by ~5 GB/step on LYNXNet2 acoustic backbone.

Supported architecture: LYNXNet2 with SoftSignGLU activation only.

Key components:

  • modules/kernels/fused_linear_softsign_glu.py: Fused forward/backward kernels with weight-split strategy to avoid cross-program communication. SoftSignGLU is numerically exact (no Taylor approximation needed).
  • modules/kernels/integration.py: Non-invasive monkey-patch with automatic eval-mode fallback for ONNX export.
  • Autotune warmup at training start to avoid first-step compilation lag.

Usage:
Set 'use_fused_kernels: true' in config (default: true). ONNX export works automatically via model.eval() fallback.

Mirror of openvpi#312, created in this fork for CodeRabbit review.

Summary by CodeRabbit

  • New Features
    • Added SoftSignGLU activation support for LYNXNet2 models.
    • Added optional CUDA-optimized fused Linear + SoftSignGLU execution for supported hardware and data types.
    • Added automatic performance warmup during training.
    • Added configuration options to enable fused kernels for acoustic and variance tasks.
  • Bug Fixes
    • Added graceful fallback to standard execution when optimized kernels are unavailable or unsupported.

Fuse nn.Linear + SoftSignGLU into single Triton kernel launches,
reducing HBM traffic by ~5 GB/step on LYNXNet2 acoustic backbone.

Supported architecture: LYNXNet2 with SoftSignGLU activation only.

Key components:
- modules/kernels/fused_linear_softsign_glu.py: Fused forward/backward
  kernels with weight-split strategy to avoid cross-program communication.
  SoftSignGLU is numerically exact (no Taylor approximation needed).
- modules/kernels/integration.py: Non-invasive monkey-patch with
  automatic eval-mode fallback for ONNX export.
- Autotune warmup at training start to avoid first-step compilation lag.

Usage:
  Set 'use_fused_kernels: true' in config (default: true).
  ONNX export works automatically via model.eval() fallback.
- Variance model backbones (K=384/512) are too small for Triton fusion
  to benefit; skip patching to avoid compilation overhead.
- Warmup with M=50000 (matching max_batch_frames) so Triton cache hits
  on the first real training step instead of recompiling.
- Reduce autotune configs from 6+5+4=15 to 3+3+2=8, cutting compile
  time by ~50%.
Warmup at init time runs before Lightning moves the model to GPU,
so Triton cannot compile kernels there. Remove the calls entirely;
compilation happens naturally on the first real training step.

Also fix default glu_type to softsign_glu in both tasks.
Warmup at init time runs before Lightning moves the model to GPU,
so Triton cannot compile kernels there. Remove the calls entirely;
compilation happens naturally on the first real training step.

Also fix default glu_type to softsign_glu in both tasks.
- P1: keep use_fused_kernels opt-in (false default) for backward compat
- P1: split weight at midpoint not at K, supporting expansion_factor != 1
- P2: add N param (GLU output dim) separate from K (input/conraction dim)
- P2: save ctx.N for backward wrapper calculations
- Remove extra N arg in _softsign_glu_bwd_elem_kernel call (M,N,N → M,N)
- Read variance glu_type from nested predictor config paths
- Assert both predictors use softsign_glu before patching
- Backward: remove custom Triton GEMM (3.4x slower than cuBLAS), use
  cuBLAS matmuls with out= preallocation for grad_x/grad_w
- Autotune key: bucket M by next_power_of_2 to prevent ~2.6s per-batch
  re-benchmark under variable frame counts (DsBatchSampler)
- Forward configs: add large tiles for Ada/Blackwell (4090/5090) +
  GROUP_M swizzle for L2 reuse; small tiles retained for Turing
- Elem backward kernel: fix key=['M','K'] -> key=['N'], fix param
  name mismatch (M,K -> M,N at call site)
- bf16 fallback: tl.dot bf16 crashes on pre-Ampere (Turing sm_75);
  detect at runtime, fall back to eager path
- Warmup: sweep M buckets under torch.autocast, derive cond hidden size
  from backbone instead of hardcoding 384
- Integration: restrict to softsign_glu only; glu_type default 'swiglu';
  per-predictor patching for variance (no blanket assertion)
- Acoustic/variance tasks: on_fit_start warmup hook; glu_type fallback

Benchmark (RTX 2070, fp16, K=1024):
  Before: 0.69x (fwd+bwd), 2669ms on new M (135x slower)
  After:  1.42x (fwd+bwd), 15ms on new M (1.41x faster)
- Remove global TF32 side-effects from acoustic_task / variance_task
  (allow_tf32 / set_float32_matmul_precision were unrelated to this PR
  and silently change fp32 training for every user).
- Revert 6 config glu_type from softsign_glu back to atanglu.
  softsign_glu stays available as an explicit opt-in; switching the
  default is an architectural decision that needs A/B evidence first.
- Add use_fused_kernels_variance: false to base.yaml so the key is
  discoverable instead of buried in a code comment.
- Protect kernel import on Windows / no-Triton: try/import triton in
  fused_linear_softsign_glu.py + capability gate for sm < 70.
  Guard Fn class + bwd kernel inside if _TRITON_AVAILABLE so the
  module loads cleanly when Triton is absent.
- Handle bias=None in Fn.forward (would AttributeError on .split()).
- Remove no-op view in forward(x.dim()>2): left/gate are already [M,N].
- Add one-time-per-(dtype,K) warning when fused falls back to eager
  (prevents fp32 / unsupported-dtype confusion).
- Integration: replace net closure with self.net[] to avoid the
  deepcopy stale-closure issue (EMA / SWA safety).
- Warmup: run no_grad forward only (DDP-safe); drop unused params
  (glu_type, num_channels) from signature.
- Task files: wrap patch calls in try/except ImportError so a missing
  Triton install logs a clear message and continues in eager mode;
  log autocast_dtype=None case so fp32 users know fused is inactive.
- Test: add assert thresholds so numerical regressions fail loudly.
- VarianceTask: add missing on_fit_start warmup (mirrors AcousticTask,
  sweeps pitch_predictor and variance_predictor over denoise_fn /
  velocity_fn). Previously use_fused_kernels_variance patched blocks
  but never warmed the autotune cache.
- _test(): compare fused AND eager against a shared fp32 reference
  instead of fused-vs-eager fp16. fp16 eager itself deviates ~1e-2
  relative from fp32 at K=512 (different accumulation order), so the
  old absolute threshold conflated rounding with kernel bugs. New
  criterion: fused error <= 2x eager error.

Verified on RTX 2070 (sm_75, torch 2.12.1+cu130, triton 3.2.0):
- kernel test: fused error consistently BELOW eager fp16 error
  (fp32 accumulators), K=256/512/1024 fwd+bwd all pass
- integration test: block fwd diff 1.95e-3, grad diff 2.20e-3
- eval-mode fallback matches train-mode fused output
- deepcopy safety: copied block uses own weights (stale-closure fixed)
- fallbacks: bias=None, CPU, fp32 (one-time warning), bf16-on-Turing
- simulated no-Triton import: module loads, eager fallback works
- warmup: no_grad forward-only, no grads created; full autocast fp16
  training step through patched LYNXNet2 works
If the modules.kernels.integration import fails, rank_zero_info (imported
on the following line inside the same try) is also unbound; import it
locally in the except clause like variance_task does.
…ory trick

Eager (common_layers.py):
- SoftSignGLUFunction: ATanGLUFunction-style custom backward.
  softsign'(x) = (1-|softsign(x)|)^2, so both partials are precomputable
  in forward; backward is two pure multiplies, saves one tensor vs naive
  autograd. SoftSignGLU now uses it in training mode.
- DoubleSoftSignGLU: y = softsign(out) * softsign(gate). softsign applied
  to the whole Linear output then split (elementwise => equivalent).
  Custom Function precomputes both partials into one [.., 2N] buffer.
- lynxnet2: register glu_type 'double_softsign_glu'.

Kernel (fused_linear_softsign_glu.py):
- IS_DOUBLE constexpr on fwd + bwd-elem kernels; Triton compiles two
  specializations, zero overhead in the single-gate path.
- fused_linear_softsign_glu(x, w, b, is_double=False) public API;
  eager fallback handles both modes.
- integration.py: _FUSABLE_GLU_TYPES = (softsign_glu, double_softsign_glu).

Verified on RTX 2070:
- eager Functions vs naive autograd in fp64: max diff < 2e-15 (both)
- double-mode kernel vs fp32 reference: fused error 2-5x SMALLER than
  eager fp16 (fwd 3.2e-4 vs 1.8e-3) — fp32 accumulators
- single-gate regression: unchanged, all pass
- block integration both modes: fwd/grad diff ~2e-3, eval fallback exact
One switch for both acoustic and variance training. The original
rationale for a separate variance key ('backbones too small to benefit')
did not hold up in benchmarks: at 24k frames/batch the variance backbone
gains 1.35x (single) / 1.62x (double softsign). The per-predictor
glu_type check already prevents patching non-softsign backbones, so the
extra key only added confusion. base.yaml comment now documents the
actual trade-off (benefit scales with max_batch_frames).
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: aa9f22b2-7f85-4bfb-809f-43f3e1720354

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

LYNXNet2 now supports SoftSignGLU activation and optional Triton-fused Linear+SoftSignGLU execution. Training tasks patch supported backbones and warm up fused kernels on CUDA. Eager execution remains available through device, dtype, and integration fallbacks.

Changes

LYNXNet2 fusion

Layer / File(s) Summary
SoftSignGLU activation support
modules/commons/common_layers.py, modules/backbones/lynxnet2.py
Adds custom-autograd and evaluation implementations of SoftSignGLU. LYNXNet2 blocks accept softsign_glu.
Fused Linear+SoftSignGLU kernel
modules/kernels/fused_linear_softsign_glu.py, modules/kernels/__init__.py
Adds Triton forward and backward kernels, eager fallbacks, dtype and device handling, and CUDA numerical checks.
Model patching and warmup
modules/kernels/integration.py
Wraps supported LYNXNet2 blocks, patches diffusion backbones, preserves evaluation execution, and warms CUDA autotune buckets.
Training configuration and lifecycle wiring
configs/acoustic.yaml, configs/variance.yaml, training/acoustic_task.py, training/variance_task.py
Adds disabled fused-kernel settings. Training tasks conditionally patch backbones and warm them up at fit start.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TrainingTask
  participant DiffusionModule
  participant FusedLYNXNet2Block
  participant fused_linear_softsign_glu
  participant Triton
  TrainingTask->>DiffusionModule: patch configured backbones
  DiffusionModule->>FusedLYNXNet2Block: replace supported blocks
  TrainingTask->>FusedLYNXNet2Block: warm up CUDA execution
  FusedLYNXNet2Block->>fused_linear_softsign_glu: run fused projections
  fused_linear_softsign_glu->>Triton: execute forward and backward kernels
Loading

Suggested reviewers: yqzhishen

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a Triton-fused SoftSignGLU kernel for LYNXNet2.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/fused-softsign-glu

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@KakaruHayate

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (10)
modules/kernels/fused_linear_softsign_glu.py (4)

462-466: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add stacklevel=2 to the fallback warning.

Ruff reports B028. Without stacklevel, the warning points at this module instead of the caller, which makes the fallback harder to attribute to a specific model or layer.

♻️ Proposed fix
             import warnings
             warnings.warn(
                 f'Fused SoftSignGLU: dtype {x.dtype} not supported for this GPU; '
-                f'falling back to eager. (This message is shown once per (dtype, K) pair.)'
+                f'falling back to eager. (This message is shown once per (dtype, K) pair.)',
+                stacklevel=2,
             )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/kernels/fused_linear_softsign_glu.py` around lines 462 - 466, Update
the warnings.warn call in the fused SoftSignGLU fallback path to include
stacklevel=2, so the warning points to the caller rather than this module while
preserving the existing message and behavior.

Source: Linters/SAST tools


396-404: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use torch.split instead of chunk for consistency with the eager GLU modules.

SwiGLU, ATanGLU, and SoftSignGLU in modules/commons/common_layers.py all use torch.split with an explicit size, and the code comments there state that chunk breaks ONNX export. This fallback path is training-only today, so behavior is identical. Aligning the split style keeps one convention and protects the function if it is ever reached during export.

♻️ Proposed fix
 def _eager_linear_softsign_glu(x, weight, bias, is_double=False):
     """Unfused reference path — used as fallback for unsupported dtypes/GPUs."""
-    if bias is not None:
-        left, gate = F.linear(x, weight, bias).chunk(2, dim=-1)
-    else:
-        left, gate = F.linear(x, weight).chunk(2, dim=-1)
+    y = F.linear(x, weight, bias)
+    left, gate = torch.split(y, y.size(-1) // 2, dim=-1)
     if is_double:
         return F.softsign(left) * F.softsign(gate)
     return left * F.softsign(gate)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/kernels/fused_linear_softsign_glu.py` around lines 396 - 404, Update
_eager_linear_softsign_glu to replace both F.linear(...).chunk(2, dim=-1) calls
with torch.split using the explicit half-width expected by the eager GLU
modules, preserving the existing bias and activation behavior.

496-537: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Extend _test to cover the is_double=True path and the bias gradient.

Two gaps in this validation:

  1. The test only exercises is_double=False. The IS_DOUBLE=True branches in _fused_linear_softsign_glu_fwd_kernel (Line 183) and _softsign_glu_bwd_elem_kernel (Line 273) implement different math and are never checked against a reference. DoubleSoftSignGLU is a selectable glu_type, so this path can reach production training unverified.
  2. Line 525 resets x16.grad and w16.grad but not b16.grad. The fused backward therefore accumulates on top of the eager bias gradient. No assertion reads b16.grad, so the test passes, but the bias gradient is untested and the state is wrong for any future assertion.
💚 Proposed fix
-        # fp16 fused
-        x16.grad = w16.grad = None
-        y_fused = fused_linear_softsign_glu(x16, w16, b16)
-        y_fused.backward(grad)
-        fused_fwd = rel_err(y_fused, ref)
-        fused_dx = rel_err(x16.grad, x32.grad)
-        fused_dw = rel_err(w16.grad, w32.grad)
+        eager_db = rel_err(b16.grad, b32.grad)
+
+        # fp16 fused
+        x16.grad = w16.grad = b16.grad = None
+        y_fused = fused_linear_softsign_glu(x16, w16, b16)
+        y_fused.backward(grad)
+        fused_fwd = rel_err(y_fused, ref)
+        fused_dx = rel_err(x16.grad, x32.grad)
+        fused_dw = rel_err(w16.grad, w32.grad)
+        fused_db = rel_err(b16.grad, b32.grad)
 
         assert fused_fwd <= eager_fwd * MARGIN + 1e-6, \
             f'K={K} fwd: fused={fused_fwd:.4e} vs eager={eager_fwd:.4e}'
         assert fused_dx <= eager_dx * MARGIN + 1e-6, \
             f'K={K} grad_x: fused={fused_dx:.4e} vs eager={eager_dx:.4e}'
         assert fused_dw <= eager_dw * MARGIN + 1e-6, \
             f'K={K} grad_w: fused={fused_dw:.4e} vs eager={eager_dw:.4e}'
+        assert fused_db <= eager_db * MARGIN + 1e-6, \
+            f'K={K} grad_b: fused={fused_db:.4e} vs eager={eager_db:.4e}'

Add an equivalent loop, or an is_double parameter, that builds the reference as F.softsign(l32) * F.softsign(g32) and calls fused_linear_softsign_glu(x16, w16, b16, is_double=True).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/kernels/fused_linear_softsign_glu.py` around lines 496 - 537, Extend
the validation loop around the existing fp32/eager/fused comparisons to exercise
both `is_double=False` and `is_double=True`, using `F.softsign(l32) *
F.softsign(g32)` as the double-path reference and passing `is_double=True` to
`fused_linear_softsign_glu`. Reset `b16.grad` alongside `x16.grad` and
`w16.grad`, and compare the fused bias gradient against `b32.grad` for each
mode.

42-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Declare Triton as an optional dependency and pin a supported version.

use_fused_kernels enables Triton kernels, but requirements.txt does not list Triton. Add an optional Triton extra and document the minimum version that supports the used API surface: triton.next_power_of_2, triton.Config(..., num_stages=...), and tl.tensor.T for transposes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/kernels/fused_linear_softsign_glu.py` around lines 42 - 47, Declare
Triton as an optional dependency in the project’s dependency configuration,
pinning the minimum supported version required by the fused kernels. Document
that this version must support triton.next_power_of_2, triton.Config with
num_stages, and tl.tensor.T, while preserving the existing optional-import
behavior in the fused kernel module.
modules/kernels/integration.py (3)

64-87: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Validate the net layout before you patch the forward pass.

fused_forward hard-codes indices 0 through 9 of block.net and assumes index 4 and index 6 are the two gated nn.Linear layers. LYNXNet2Block.__init__ builds exactly that layout today, but nothing enforces it. If the nn.Sequential gains, loses, or reorders a layer, this patch either raises IndexError or silently computes a different graph while training reports no error.

Add a cheap structural check in wrap_lynxnet2_block before patching, and return the block unpatched if the layout does not match.

♻️ Proposed check
     is_double = glu_type == 'double_softsign_glu'
 
+    # The fused forward hard-codes the Sequential layout built by
+    # LYNXNet2Block.__init__. Refuse to patch anything else.
+    net = block.net
+    if not (
+        len(net) == 10
+        and isinstance(net[4], nn.Linear)
+        and isinstance(net[6], nn.Linear)
+        and net[4].out_features == 2 * net[6].in_features
+    ):
+        import warnings
+        warnings.warn(
+            'Unexpected LYNXNet2Block.net layout; leaving block unpatched.',
+            stacklevel=2,
+        )
+        return block
+
     def fused_forward(self, x):
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/kernels/integration.py` around lines 64 - 87, Add a structural
validation in wrap_lynxnet2_block before replacing fused_forward, confirming
block.net has the expected ten-layer layout and that indices 4 and 6 are the
gated nn.Linear layers used by fused_forward. If validation fails, return the
original block without patching; otherwise preserve the existing wrapping
behavior.

280-287: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Narrow the blanket except Exception or log the traceback.

Ruff reports BLE001. The handler catches every failure at Line 280, formats only str(e), and breaks the sweep. A real bug in the fused forward, such as a shape mismatch in wrap_lynxnet2_block, then surfaces as a one-line warning at startup and reappears later as a confusing failure during the first real training step. Log the traceback so the cause is recoverable, and keep the non-fatal behavior.

♻️ Proposed change
         except Exception as e:
             # Autotune failure should not crash training — Triton cache
             # can be built on the first real step instead.
-            warnings.warn(f'Fused kernel warmup skipped at T={T} ({e})')
+            import traceback
+            warnings.warn(
+                f'Fused kernel warmup skipped at T={T} ({type(e).__name__}: {e})\n'
+                f'{traceback.format_exc()}',
+                stacklevel=2,
+            )
             break
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/kernels/integration.py` around lines 280 - 287, Update the warmup
exception handler in the fused-kernel sweep to preserve non-fatal behavior while
recording the full traceback, using traceback-aware warning or logging support
instead of only formatting str(e); alternatively narrow the caught exception to
the expected autotune/cache failures. Resolve Ruff BLE001 without changing the
existing break and cleanup flow around wrap_lynxnet2_block.

Source: Linters/SAST tools


174-202: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the unused patch_acoustic_model / patch_variance_model helpers.

The training tasks call patch_diffusion_module directly, not these wrappers. patch_variance_model also applies one glu_type to both variance predictors and ignores its own glu_type argument, so it would not preserve the per-predictor fallback behavior used in training/variance_task.py.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/kernels/integration.py` around lines 174 - 202, Remove the unused
patch_acoustic_model and patch_variance_model helper functions, leaving
patch_diffusion_module as the direct integration entry point. Do not alter the
existing per-predictor fallback behavior in training/variance_task.py.
training/acoustic_task.py (3)

123-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The warmup body is duplicated across both task files. The precision parsing, the autocast_dtype selection, the fallback log message, and the warmup_fused_backbone loop are near-identical in the two hooks. The two copies already differ in how they locate the backbones, and any fix to the precision parsing must be applied twice. Move the shared logic into one helper in modules/kernels/integration.py, for example warmup_fused_backbones(backbones, max_frames, precision), and let each task supply only its backbone list.

  • training/acoustic_task.py#L123-L141: replace the inline body with a call to the shared helper, passing the backbones resolved from self.model.diffusion.
  • training/variance_task.py#L165-L181: replace the inline body with a call to the same helper, passing self._fused_kernel_backbones.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@training/acoustic_task.py` around lines 123 - 141, The duplicated
fused-backbone warmup logic should be centralized in a new helper such as
warmup_fused_backbones in modules/kernels/integration.py, including precision
parsing, autocast selection, fallback logging, and warmup_fused_backbone
iteration. In training/acoustic_task.py lines 123-141, resolve the denoise_fn
and velocity_fn backbones and pass them to the helper; in
training/variance_task.py lines 165-181, replace the inline logic with the same
helper using self._fused_kernel_backbones. Ensure both sites retain their
existing backbone-selection behavior.

100-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

_fused_kernels_fallback is written but never read.

Line 101 and Line 115 maintain this flag, and training/variance_task.py does the same at Line 124 and Line 156. No code reads it. Either use it to gate the warmup and the log message, or remove it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@training/acoustic_task.py` around lines 100 - 111, Remove the unused
_fused_kernels_fallback state and its assignments in the acoustic task, and
apply the same cleanup to variance_task.py if present. Keep the fused-kernel
patching and existing logging behavior unchanged.

123-128: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Read the effective precision from the trainer, not from hparams.

hparams['pl_trainer_precision'] records the configured value. Inside on_fit_start, the trainer already exists, so self.trainer.precision reports the precision Lightning resolved at runtime. If the two disagree, the warmup autotunes for the wrong dtype and the warmup no longer protects the first real step.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@training/acoustic_task.py` around lines 123 - 128, Update the precision
lookup in on_fit_start to use self.trainer.precision instead of
hparams.get('pl_trainer_precision', '32'), while preserving the existing
autocast_dtype mapping for 16-bit, bf16, and other precisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@modules/kernels/integration.py`:
- Around line 115-119: Make Triton absence degrade to eager execution
consistently: in modules/kernels/integration.py lines 115-119 within
patch_lynxnet2_model and lines 232-236 within warmup_fused_backbone, replace the
RuntimeError path with a warning and return 0, ensuring the non-CUDA early
return remains reachable. In training/acoustic_task.py lines 112-115 and
training/variance_task.py lines 153-156, retain the existing ImportError
handlers with no direct change because the integration functions will no longer
raise for unavailable Triton.
- Line 338: Remove the unnecessary f-string prefix from the integration success
message print statement, leaving the message text and output behavior unchanged.
- Around line 89-94: Move fused_forward from its local closure into module scope
so it is picklable, and replace the captured is_double value with a block
attribute assigned before monkey-patching forward. Update the bound-method
assignment in the surrounding factory flow to reference the module-level
function while preserving dynamic self binding and fused-kernel behavior.

---

Nitpick comments:
In `@modules/kernels/fused_linear_softsign_glu.py`:
- Around line 462-466: Update the warnings.warn call in the fused SoftSignGLU
fallback path to include stacklevel=2, so the warning points to the caller
rather than this module while preserving the existing message and behavior.
- Around line 396-404: Update _eager_linear_softsign_glu to replace both
F.linear(...).chunk(2, dim=-1) calls with torch.split using the explicit
half-width expected by the eager GLU modules, preserving the existing bias and
activation behavior.
- Around line 496-537: Extend the validation loop around the existing
fp32/eager/fused comparisons to exercise both `is_double=False` and
`is_double=True`, using `F.softsign(l32) * F.softsign(g32)` as the double-path
reference and passing `is_double=True` to `fused_linear_softsign_glu`. Reset
`b16.grad` alongside `x16.grad` and `w16.grad`, and compare the fused bias
gradient against `b32.grad` for each mode.
- Around line 42-47: Declare Triton as an optional dependency in the project’s
dependency configuration, pinning the minimum supported version required by the
fused kernels. Document that this version must support triton.next_power_of_2,
triton.Config with num_stages, and tl.tensor.T, while preserving the existing
optional-import behavior in the fused kernel module.

In `@modules/kernels/integration.py`:
- Around line 64-87: Add a structural validation in wrap_lynxnet2_block before
replacing fused_forward, confirming block.net has the expected ten-layer layout
and that indices 4 and 6 are the gated nn.Linear layers used by fused_forward.
If validation fails, return the original block without patching; otherwise
preserve the existing wrapping behavior.
- Around line 280-287: Update the warmup exception handler in the fused-kernel
sweep to preserve non-fatal behavior while recording the full traceback, using
traceback-aware warning or logging support instead of only formatting str(e);
alternatively narrow the caught exception to the expected autotune/cache
failures. Resolve Ruff BLE001 without changing the existing break and cleanup
flow around wrap_lynxnet2_block.
- Around line 174-202: Remove the unused patch_acoustic_model and
patch_variance_model helper functions, leaving patch_diffusion_module as the
direct integration entry point. Do not alter the existing per-predictor fallback
behavior in training/variance_task.py.

In `@training/acoustic_task.py`:
- Around line 123-141: The duplicated fused-backbone warmup logic should be
centralized in a new helper such as warmup_fused_backbones in
modules/kernels/integration.py, including precision parsing, autocast selection,
fallback logging, and warmup_fused_backbone iteration. In
training/acoustic_task.py lines 123-141, resolve the denoise_fn and velocity_fn
backbones and pass them to the helper; in training/variance_task.py lines
165-181, replace the inline logic with the same helper using
self._fused_kernel_backbones. Ensure both sites retain their existing
backbone-selection behavior.
- Around line 100-111: Remove the unused _fused_kernels_fallback state and its
assignments in the acoustic task, and apply the same cleanup to variance_task.py
if present. Keep the fused-kernel patching and existing logging behavior
unchanged.
- Around line 123-128: Update the precision lookup in on_fit_start to use
self.trainer.precision instead of hparams.get('pl_trainer_precision', '32'),
while preserving the existing autocast_dtype mapping for 16-bit, bf16, and other
precisions.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9d2b7a82-f5c0-4c9a-95e1-923a1db5d748

📥 Commits

Reviewing files that changed from the base of the PR and between 753b7cc and c13588f.

📒 Files selected for processing (8)
  • configs/base.yaml
  • modules/backbones/lynxnet2.py
  • modules/commons/common_layers.py
  • modules/kernels/__init__.py
  • modules/kernels/fused_linear_softsign_glu.py
  • modules/kernels/integration.py
  • training/acoustic_task.py
  • training/variance_task.py

Comment thread modules/kernels/integration.py Outdated
Comment thread modules/kernels/integration.py
Comment thread modules/kernels/integration.py Outdated
@KakaruHayate

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@KakaruHayate

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
modules/kernels/fused_linear_softsign_glu.py (1)

364-406: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restore the one-dimensional input contract.

[K] is valid for the documented [..., K] input shape. The custom function reshapes it to [1, K], but it restores shapes only when the rank is greater than two. Fused CUDA execution then returns [1, N], and backward returns [1, K], instead of the eager [N] output and [K] gradient.

Restore the shape for every rank other than two. Add a [K] forward and backward regression test.

Proposed fix
-            if x.dim() > 2:
+            if x.dim() != 2:
                 out = out.view(*orig_shape[:-1], N)
...
-            if len(ctx.orig_x_shape) > 2:
+            if len(ctx.orig_x_shape) != 2:
                 grad_x = grad_x.view(*ctx.orig_x_shape)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/kernels/fused_linear_softsign_glu.py` around lines 364 - 406, Update
FusedLinearSoftSignGLUFn’s forward and backward shape restoration to handle
every input rank other than two, including one-dimensional [K] inputs, so fused
outputs and gradients match the eager [N] and [K] shapes. Add regression
coverage for one-dimensional forward output and backward gradient shapes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@configs/acoustic.yaml`:
- Around line 6-7: Align the fused-kernel settings in configs/acoustic.yaml
lines 6-7 and configs/variance.yaml lines 6-7: set use_fused_kernels to true and
change each relevant glu_type from atanglu to softsign_glu so both
configurations use the supported fused integration.

---

Outside diff comments:
In `@modules/kernels/fused_linear_softsign_glu.py`:
- Around line 364-406: Update FusedLinearSoftSignGLUFn’s forward and backward
shape restoration to handle every input rank other than two, including
one-dimensional [K] inputs, so fused outputs and gradients match the eager [N]
and [K] shapes. Add regression coverage for one-dimensional forward output and
backward gradient shapes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b104879-003b-4cbb-bad1-09ac3b8fed8e

📥 Commits

Reviewing files that changed from the base of the PR and between c13588f and 8cd54bf.

📒 Files selected for processing (8)
  • configs/acoustic.yaml
  • configs/variance.yaml
  • modules/backbones/lynxnet2.py
  • modules/commons/common_layers.py
  • modules/kernels/fused_linear_softsign_glu.py
  • modules/kernels/integration.py
  • training/acoustic_task.py
  • training/variance_task.py
💤 Files with no reviewable changes (1)
  • modules/commons/common_layers.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • modules/kernels/integration.py
  • training/variance_task.py

Comment thread configs/acoustic.yaml
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.

1 participant