Skip to content

rocm: use numerically stable SiLU to avoid NaN on large-magnitude gate#498

Open
Flowers-of-Romance wants to merge 1 commit into
antirez:mainfrom
Flowers-of-Romance:fix/rocm-stable-silu
Open

rocm: use numerically stable SiLU to avoid NaN on large-magnitude gate#498
Flowers-of-Romance wants to merge 1 commit into
antirez:mainfrom
Flowers-of-Romance:fix/rocm-stable-silu

Conversation

@Flowers-of-Romance

Copy link
Copy Markdown

The ROCm MoE/FFN kernels computed SiLU as the naive gate / (1.0f + expf(-gate)). For a large negative gate, expf(-gate) overflows to +inf, so the expression becomes -inf / inf = NaN. The CPU reference (ds4.c silu/sigmoid_stable) deliberately avoids this by branching on the sign, so the two backends disagreed on extreme inputs and only ROCm produced NaN — which then propagates through the rest of the network (e.g. a later router selecting expert id -1).

Add dev_sigmoid_stable/dev_silu_stable in ds4_rocm_common.cuh (matching the CPU sign-branched form) and replace the 27 naive SiLU sites across ds4_rocm_moe.cuh, ds4_rocm_q8.cuh, and ds4_rocm_output.cuh. No behavior change for well-conditioned inputs; only removes the overflow path on extreme gate values.

The ROCm MoE/FFN kernels computed SiLU as the naive
`gate / (1.0f + expf(-gate))`. For a large negative gate, `expf(-gate)`
overflows to +inf, so the expression becomes `-inf / inf = NaN`. The CPU
reference (`ds4.c` `silu`/`sigmoid_stable`) deliberately avoids this by
branching on the sign, so the two backends disagreed on extreme inputs and
only ROCm produced NaN — which then propagates through the rest of the
network (e.g. a later router selecting expert id -1).

Add `dev_sigmoid_stable`/`dev_silu_stable` in `ds4_rocm_common.cuh`
(matching the CPU sign-branched form) and replace the 27 naive SiLU sites
across `ds4_rocm_moe.cuh`, `ds4_rocm_q8.cuh`, and `ds4_rocm_output.cuh`.
No behavior change for well-conditioned inputs; only removes the overflow
path on extreme gate values.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@OPS-NeoRetro OPS-NeoRetro 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.

So, your numerically stable Swish that Claude wrote is directly taken from ds4.c, but it's just ported to AMD ROCm. Are you planning to test it today or tomorrow? I hope that somebody would port this fix to CUDA, or I don't think that it's there.

Comment thread rocm/ds4_rocm_common.cuh
Comment on lines +11 to +23
/* Numerically stable sigmoid/SiLU matching the CPU reference (ds4.c
* sigmoid_stable/silu). The naive gate/(1+expf(-gate)) overflows for large
* negative gate: expf(-gate) -> +inf makes it -inf/inf = NaN, which the CPU
* path avoids. Use these everywhere the ROCm MoE/FFN applies SiLU. */
__device__ __forceinline__ static float dev_sigmoid_stable(float x) {
if (x >= 0.0f) return 1.0f / (1.0f + expf(-x));
const float e = expf(x);
return e / (1.0f + e);
}
__device__ __forceinline__ static float dev_silu_stable(float x) {
return x * dev_sigmoid_stable(x);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great job. Claude did the common stable Swish.

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