vulkan: decline GATED_DELTA_NET raw gates instead of computing them wrong - #239
Merged
Merged
Conversation
…rong ggml_gated_delta_net_set_raw_gates() delivers beta and g pre-activation, so the op must apply beta = sigmoid(beta) and g = a * softplus(g + dt_bias), with dt_bias in src[7] and a in src[8]. gated_delta_net.comp has neither of those bindings nor that math - it applies exp(g) unconditionally - but supports_op never checked the flag, so Vulkan claimed these ops and returned results uncorrelated with the reference (ERR ~1.0 against a 1e-7 tolerance), silently and with no fallback. Decline raw gates in supports_op so they fall back to the CPU, which implements them. This mirrors what the SYCL backend already does. On Arc B390, test-backend-ops -b Vulkan0 goes from 3 failing GATED_DELTA_NET cases to a clean run: the full unfiltered suite reports FAIL before and OK after.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #237.
Problem
ggml_gated_delta_net_set_raw_gates()deliversbetaandgpre-activation, so the op has to applyas the CPU implementation does (
ggml/src/ggml-cpu/ops.cpp, theif (raw_gates)branch).gated_delta_net.comphas neither binding and does none of that math — it appliesexp(g)unconditionally. Butsupports_oponly checkssrc[6], the head size and the src types; it never looks atop_params[1]. So the Vulkan backend claims these ops and returns results uncorrelated with the reference, silently — no abort, no warning, no fallback.Measured on an Intel Arc B390 before this change,
test-backend-ops test -o GATED_DELTA_NET -b Vulkan0:head_count=4, head_size=128)n_seq_tokens=1, n_seqs=1, v_repeat=1n_seq_tokens=1, n_seqs=2, v_repeat=2n_seq_tokens=64, n_seqs=1, v_repeat=115/15 failures over five consecutive runs. All 45
raw_gates=0cases pass, and noraw_gates=1case has ever passed — so the split is exactly on this flag. The SYCL backend on the same machine declines all fourraw_gates=1cases asnot supported, which is the behaviour this change adopts.Change
Nine lines in
supports_op: decline whenggml_get_op_params_i32(op, 1) != 0, with a comment recording why.This makes the case fall back to the CPU, which computes it correctly. It does not implement raw gates on Vulkan — that needs two new shader bindings plus the sigmoid/softplus math and care around the
kdapath, which is a much larger change. The trade here is speed for correctness on those shapes, which seems clearly right when the current behaviour is silent corruption.Result
test-backend-ops test -o GATED_DELTA_NET -b Vulkan0, three consecutive runs: 36/36 passed, Backend Vulkan0: OK each time. The fourraw_gates=1cases now reportnot supported [Vulkan0].Full unfiltered
test-backend-ops test -b Vulkan0on the same device:prism422590f5dThe three failures become
not supported; the Vulkan backend passes the suite on this device for the first time.Note for #187
PR #187 implements the rows-indexed state path, which makes
supports_opaccept a fourthraw_gates=1case (K=2, rows_mode=1) that stockprismdeclines for an unrelated reason. That case then executes and hits this same bug, which looks like a regression in #187 but is not one — it is this bug surfacing on one more shape. With this change applied that case is declined too, and #187's GDN results are clean.