Skip to content

fix: handle multi-dim activations in LoKr forward - #1980

Closed
undefeaterpeter wants to merge 1 commit into
leejet:masterfrom
undefeaterpeter:fix/lokr-multi-dim-activations
Closed

undefeaterpeter wants to merge 1 commit into
leejet:masterfrom
undefeaterpeter:fix/lokr-multi-dim-activations

Conversation

@undefeaterpeter

Copy link
Copy Markdown

Problem

Applying a LoKr LoRA (lokr_w1/lokr_w2 factors) to a model whose linear layers run on 3-D activations aborts the process with SIGABRT. Crash stack:

ggml_abort
ggml_reshape_3d
ggml_ext_lokr_forward
LoraModel::get_out_diff
MultiLoraAdapter::forward_with_lora
Linear::forward
Krea2::KreaAttention::forward

Root cause

The non-conv branch of ggml_ext_lokr_forward assumes a 2-D activation [q, batch] and takes batch = h->ne[1]. Krea2::KreaAttention (and other batched attention paths) feed linears 3-D activations [features, L, N]; whenever N > 1, ggml_nelements(h) = q·L·N ≠ vq·uq·L, so the first split reshape's element-count GGML_ASSERT fires and aborts. With N == 1 every reshape happens to pass, which is why LoKr works on many models and the gap went unnoticed.

Fix

  • batch = ggml_nelements(h) / q_actual — fold all trailing dims into the batch (the existing reshapes already assume a contiguous activation).
  • Reshape the result back to [up·vp, ne[1], ne[2], ne[3]] instead of flat 2-D, so forward_with_lora's ggml_add_inplace adds the diff element-wise onto the linear output rather than depending on a shape that no longer matches.
  • The conv branch already batches on ne[3] and is unchanged. The batch dim stays outermost through every reshape/transpose in the pipeline, so folding L·N and restoring at the end is index-exact; the Kronecker row convention (out = u·vp + p) is unchanged.

Verification

  • Repro: Krea-2 Q4_1 GGUF + a LoKr LoRA (full lokr_w1/lokr_w2 factors, OneTrainer/diffusers export) — 100% SIGABRT at the stack above before the fix.
  • After: the same model + LoRA stack renders to completion (Metal backend, Apple M4); output is coherent, confirming the restored layout is index-exact and not merely non-crashing.
  • Built and render-tested on the commit before the recent tokenizer changes; src/model/adapter/ is identical between that commit and current master.

🤖 Generated with Claude Code — reviewed and submitted by the repo owner per the AI-assisted contribution policy.

ggml_ext_lokr_forward's non-conv branch assumed a 2-D activation [q, batch] and took batch = ne[1]. Model blocks that run linear layers on 3-D activations [features, L, N] (e.g. Krea-2 attention with N > 1) hit the first split reshape with a mismatched element count, and the GGML_ASSERT inside ggml_reshape_3d aborted the process (SIGABRT) whenever a LoKr LoRA was active on such a layer.

Fold every trailing dim into the batch (nelements / q) and reshape the result back to the activation's original trailing dims, so the out-diff adds element-wise onto the linear output instead of relying on a flat 2-D shape. The conv branch already batches on ne[3] and is unchanged.
@undefeaterpeter

Copy link
Copy Markdown
Author

Withdrawn.

@undefeaterpeter
undefeaterpeter deleted the fix/lokr-multi-dim-activations branch September 15, 2026 16:30
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