Skip to content

fix(models): decide the GELU formula for the families #2423 left open #2548

Closed

Description

@inureyes

Problem / Background

#2423 found that mlxcel_core::gelu_approx is the exact erf GELU while MLX's nn.gelu_approx / nn.GELU(approx="precise") is the tanh form, and inventoried every GELU call site against its upstream port in docs/gelu-call-sites.md (row ids below refer to that file). PR #2552 (for #2423) added the GeluActivation { Erf, Tanh, TanhF32, Fast } selector (with from_hf_name) and changed only the families it measured on local checkpoints: MiniCPM-V 4.6 encoder MLP and window merger, Gemma 3 VLM and PaliGemma SigLIP (missing hidden_act resolved to the SigLIP default), and the Gemma 4 vision GeGLU. InternViT and the MiniCPM-V 4.6 downsample merger keep erf because Hugging Face evaluates them as erf.

The measurement record (docs/benchmark_results/gelu-erf-vs-tanh-2026-10-23.md) showed the decided-position logit-trace gate cannot tell erf from tanh on any measured family: the swap moves logits by at most the bf16 quantum and the reverted arm does not fail. So each family below is decided by the activation the checkpoint declares (its config.json and the Hugging Face modeling code), cross-checked against the mlx-lm / mlx-vlm port, and changed through GeluActivation with a CLI (mlxcel generate, with --image for vision) and one mlxcel-server request on a real checkpoint where one exists locally. Where the upstream MLX port departs from Hugging Face, record which one the change follows.

Implement everything in one PR with a single verification pass at the end and as few pushes as possible (--profile test-fast --features cuda, MLX_CUDA_ARCHITECTURES=121, --test-threads=1, MLX_ENABLE_TF32=0 for any trace).

Acceptance Criteria

erf in mlxcel where the checkpoint and upstream use tanh:

  • Gemma3n sparse layers (G15, G17, dead G18): compiled_gelu_topk, the sparse branch of the fused gemma3n_mlp_forward, and the XLA host oracle sparse_gelu in src/lib/mlxcel-xla/src/emitter/gemma3n_math.rs switch to tanh together, so eager and XLA stay in parity. Local: gemma3n-e2b-4bit, gemma3n-e4b-4bit, gemma3n-e4b-bf16.
  • Gemma 4 non-quantized dense MLP and assistant drafter (G7, G12): compiled_gelu_mlp_forward_fp16 uses the erf GeGLU; upstream is tanh. Also reconcile Gemma v1, whose quantized path is tanh and bf16 path erf (G1, G2; mlx-lm gemma.py uses erf, HF says gelu_pytorch_tanh). Local: gemma-4-31b-it-assistant-bf16, gemma-2b-4bit.
  • Gemma 4 per-layer gate under the tensor-parallel runtime (G11, llama_runtime.rs) computes erf while the single-host path computes tanh.
  • RecurrentGemma (G20, G21).
  • MiniCPM-o encoder MLP (V5 for MiniCPM-o): read vision_config.hidden_act like MiniCPM-V 4.6 does once a checkpoint can be smoke-tested.
  • SigLIP towers whose config omits hidden_act and are still resolved to erf: Aya Vision (S3) and Granite Vision 3.x (S8; mlx-vlm uses fast, the SiglipVisionConfig default is tanh, decide which to follow). Local: aya-vision-8b, granite-vision-3.2-2b-4bit.
  • Vision MLPs: Phi-4-MM / Phi4-SigLIP (V6), DeepSeek-VL2 (V11, deepseek-vl2-small-4bit), Youtu-VL (V14, youtu-vl-4b-instruct, the XLA path already uses tanh), Jina-VLM (V16, jina-vlm-mlx), LFM2-VL (V25, lfm2-vl-*), Moondream2/3 vision MLP and projector (V27, V28, moondream2), PaddleOCR-VL (V29, paddleocr-vl-bfloat16, via mlxcel_core::gelu).
  • Text MLPs: Moondream2 LM (T19), Moondream3 LM dense MLP (T17), Phi-1/1.5/2 (T1, phi-2-4bit), Phixtral (T2), GPT-2 (T3). GPT-NeoX (T4) and ModernBERT (T10, T11) are erf in Hugging Face; record a keep decision for them.

fast (x * sigmoid(1.702x)) in upstream or in the Hugging Face default:

  • CLIP towers that omit hidden_act resolve to erf, but CLIPVisionConfig().hidden_act == "quick_gelu": LLaVA 1.5 / LLaVA-NeXT (S9), Bunny (S11), Phi-3.5-V (S12), Llama-Nemotron-VL-Embed (S14). Resolve by vision_config.model_type rather than flipping the shared serde default. Local: llava-1.5-7b-4bit, llava-next-mistral-7b-4bit, bunny-llama3-8b-4bit, phi-3.5-vision-4bit.
  • Qwen2-VL ViT (V19, HF default quick_gelu), Molmo ViT (V22), Llama 4 vision (V13, config says gelu, mlx-vlm hard-codes fast), LLaVA-Interleave (S10) and Molmo2 (V23), where mlx-vlm maps gelu_pytorch_tanh to fast: decide HF versus mlx-vlm per family and record it.

tanh in mlxcel where upstream uses erf (mlxcel follows Hugging Face):

  • Kokoro PLBert (T13), Qwen3-Omni vision MLP (V34), Qwen3-VL-MoE MLP and merger (V33). Record the keep decision or change it, with the reason.

Wrong activation and leftovers:

  • Nemotron-NAS (T8) maps gelu_new / gelu_fast to SiLU instead of tanh GELU; map them through GeluActivation::from_hf_name.
  • Fold the remaining private copies: src/models/gemma3_backbone.rs (tanh with power(x, 3), the op that produced the original bf16 NaN; folding changes numerics, so smoke-test Nemotron VoiceChat TTS) and src/audio/nemotron_codec/layers.rs gelu_exact.
  • XLA StarCoder2 dense MLP emits tanh (src/lib/mlxcel-xla/src/emitter/model.rs) while eager and mlx-lm use erf.
  • Update docs/gelu-call-sites.md so every row above shows its final decision.

Activity

  1. added
    type:bugBug fixes, error corrections, or issue resolutions
    area:modelsModel architectures, weights, loading, metadata
    area:coremlxcel-core: MLX FFI, primitives, KV cache, layers
    on Oct 22, 2026
  2. inureyes commented on Oct 22, 2026

    @inureyes
    MemberAuthor

    Additional item from the PR #2552 review (prefill cost of the tanh switch):

    • Fused tanh GELU. The eager GeluActivation::TanhF32 is about 11 unfused elementwise passes, against about 6 for the f32 erf in gelu_approx. On gemma-3-4b-it-4bit with one image it costs +151 to +172 ms of an 872 ms prefill (input-dtype Tanh: +42 ms); numbers in docs/benchmark_results/gelu-erf-vs-tanh-2026-10-23.md, "Prefill cost of the f32 tanh". A compiled kernel for TanhF32 (and likely for the erf path too) should remove most of it for every caller. Fusion changes rounding (FMA contraction), so the bit-exact fold tests in utils_gelu_tests.rs become tolerance tests, and the families already on TanhF32 (SigLIP explicit tanh, Molmo2, Qwen3-VL, plus the four investigate: gelu_approx computes erf GELU where upstream models use the tanh approximation #2423 switched) need a re-check against their StableHLO twins.
  3. inureyes commented on Oct 22, 2026

    @inureyes
    MemberAuthor

    Update from PR #2552: the switched families now run the f32 tanh GELU through a compiled kernel (compiled_gelu_tanh_f32). Measured image prefill against erf on GB10: Gemma 3 +2.8% pooled (n=16, same sign in both blocks, null arm centred on zero), Gemma 4 +2.0%, MiniCPM-V 4.6 and PaliGemma within the null arm; the eager form cost Gemma 3 +17%. On CUDA the compiled region still runs as more than one kernel (the cube and the tanh tail are separate cached modules), the likely source of the remaining Gemma 3 cost. Acceptance for the fused-kernel item here: a single-kernel tanh GELU (f32 inside, input dtype out) that brings Gemma 3 image prefill within the null floor of erf, measured with interleaved arms and a null arm. Record: docs/benchmark_results/gelu-erf-vs-tanh-2026-10-23.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:coremlxcel-core: MLX FFI, primitives, KV cache, layersarea:modelsModel architectures, weights, loading, metadatapriority:mediumMedium prioritystatus:doneCompletedtype:bugBug fixes, error corrections, or issue resolutions

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions