Fix Gemma 4 for upcoming Transformers version - #49797
Conversation
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
|
cc @charlotte12l for model arch converter |
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
|
thanks for taking this on, @hmellor! I ran the PR's logic against Transformers main with the actual Hub The gate is correct, and the proposed alternative would breakthe concern raised on #48432 was that that does not reproduce. global_head_dim = kwargs.pop("global_head_dim", 512)
num_global_key_value_heads = kwargs.pop("num_global_key_value_heads", None)
if "per_layer_config" not in kwargs:
...so old-layout checkpoints come out already migrated. Loading the published configs on 5.15:
all three report the suggested alternative - keying off
|
single per_layer_config[i] access (by index) |
0.032 ms |
single per_layer_config["full_attention"] access (by type) |
0.035 ms |
get_head_size() by type, set()-deduped |
0.07 ms |
get_head_size() by type, un-deduped |
1.36 ms |
full convert() |
0.204 ms |
create_attention_instances total (2 accessors × 48 layers) |
~20 ms |
by-type versus by-index is a wash per access, so there's nothing to fix there. ~20 ms of added
startup is fine.
one trivial leftover: Gemma4Config.verify_and_update_config iterates hf_text_config.layer_types
un-deduped - that list is one entry per layer, so 48 lookups where 2 would do (1.36 ms vs
0.07 ms). model_arch_config_convertor.py already uses set(...) for the identical job. Worth
matching, purely for consistency.
total_num_kv_heads is now doing two jobs
switching get_quant_config to the arch converter is a real improvement - it picks up Falcon's
n_head_kv, ChatGLM's multi_query_group_num etc. that the old raw getattr missed, and it's
required anyway per the raising-getattr point above.
But note what it feeds: the whole-model value is total_num_kv_heads=8 for the 12B, while the
full-attention layers actually have kv=1, and the consumer is "TP-aware loading of attn_head
scales" - where the max is not obviously the right quantity, unlike the KV-cache buffer sizing the
new docstring cites as its rationale... this is pre-existing (the old code also read 8), so not a
regression, but routing it through a method documented for a different purpose cements the
conflation. gemma-4-12B-it-qat-w4a16-ct (the model in your benchmark table) is a
compressed-tensors checkpoint, so this path is live... so a comment at minimum would be great;
ideally the per-head-scale consumer asks for per-layer counts.
smaller things
- Both attribute removals in
gemma4.pycheck out:self.use_k_eq_vhas no readers and
self.is_full_attentionis only used inside__init__.gemma4_dspark.pycorrectly keeps its
ownself.use_k_eq_v, which is read inforwardand asserted on. no issue - just recording it !=tois notin_get_transformers_backend_clsis right and matches whatget_hf_text_config
already does for the nesting test... worth one line of comment on why (a deep config comparison
raises on heterogeneous configs), sinceis noton configs otherwise reads as a bugtransformers/base.pymakeshead_sizeandnum_kv_headsper-layer but leavesnum_heads
global... correct for Gemma 4 today; a comment would help if anything ever varies
num_attention_headsper layer.
Tests
this is my one substantive ask. There are none, and this PR silently changes per-layer attention
geometry for the whole Gemma 4 family on a path CI can't exercise until 5.15.0 ships. The three
behaviours above that work for non-obvious reasons - legacy back-fill, per-layer-config collapse,
and raising getattr - are each one upstream refactor away from breaking silently... synthetic
configs asserting the per-layer table (256/8 sliding, 512/1 full for the 12B) would be cheap and
would have settled the whole #48432 disagreement in CI rather than in review. #48432 had
tests/config/test_multimodal_config.py and tests/model_executor/test_gemma4_layer_geometry.py
which should mostly drop straight in.
I can run the full family (12B/26B/31B/E2B/E4B) end to end once there's a build against 5.15.0 -
happy to do that before or after merge, whichever is more useful.
my apologies for the huge comment, @hmellor :) thanks for pushing it forward!
|
Thanks for reviewing @lucianommartins
This is very much intentional otherwise any model that could be heterogeneous would need to be littered with many
Hmm, I'm not really sure what should be done in this case then. Perhaps a util on the Transformers side that gets all the possible values for a field per layer type as
Yes I am still working on a way to generalise this properly that doesn't require me to add |
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
transformers 5.15.0 landed on PyPI 2026-08-10, one day before upstream built the v0.27.1 images with an unbounded '>=5.5.3', and its heterogeneous-config machinery breaks Gemma-4 engine-config parsing (AmbiguousGlobalPerLayerAttributeError escapes vLLM's getattr default). The 2026-08-11 gate run proved every Gemma arm dies before touching the GPU while every Qwen arm passes. The vLLM-side fix (vllm-project#49797, 70b84f0) merged to main one day after the tag and does not apply cleanly to it. Pin the version v0.27.1 was developed against; the Dockerfile records the exit criterion. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Mazyad Alabduljaleel <maz@level3.io>
Fixes Gemma-4 models failing to load with Transformers v5 due to
`AmbiguousGlobalPerLayerAttributeError` when accessing per-layer
attributes like `num_key_value_heads` and `head_dim`.
Transformers v5 introduced heterogeneous configs where certain attributes
vary per layer. Accessing these directly on the config object raises an
exception. The fix involves:
1. Infrastructure changes to ModelArchitectureConfig:
- Add per_layer_overrides field
- Add __getitem__ for layer-specific views
- Add from_layers() classmethod to merge per-layer configs
2. Model arch config convertor updates:
- Add get_per_layer_hf_configs() to generate layer configs
- Refactor convert() to handle per-layer configs via from_layers()
- Override for Gemma4 to handle both Transformers v4 (layer_types)
and v5 (is_heterogeneous) formats
- Add get_total_num_kv_heads() override to return max across layers
3. Helper function for layer config access:
- gemma4_layer_config() handles both v4 and v5 formats
- Returns per-layer config for given layer_idx
4. Config comparison fix:
- Use object identity (is not) instead of equality (!=) in
model.py to avoid triggering __eq__ which accesses all attributes
Based on upstream vllm-project/vllm PR vllm-project#49797 but adapted for ROCm fork.
AIESW-41388
Signed-off-by: Callum McIntyre <callum.mcintyre@amd.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: <callumm@amd.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com> Signed-off-by: Wu, Xiaochang <xiaochang.wu@intel.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
v0.29.0 contains the Gemma 4 head_dim fix (vllm-project/vllm#49797, the fix for #51744), so the container-start `pip install transformers==5.14.1` override in both compose files and run_cluster.sh is no longer needed. Re-verified the default 26B-A4B config on the RTX PRO 6000 and on a DGX Spark: chat, thinking, auto tool calls and vision all pass; the same FlashInferCutlass NVFP4 GEMM + FLASHINFER_CUTLASS MoE kernels are picked; decode is 228.9 / 1,155 tok/s (single / c8) locally and 47.6 / 217 tok/s on the Spark, +3-4% over the v0.26.0 README figures. Checkpoint revision is unchanged since July.
Transformers v5.15.0 will introduce heterogeneous config machinery that has been adopted by Gemma 4.
This PR updates the Gemma 4 implementations and the Transformers modelling backend to be compatible with these new heterogeneous configs.
It also required making some small changes to the model arch converter so that it plays nicely with heterogeneous configs.
Supersedes #48432
With huggingface/transformers#47547, here is a performance comparison of vLLM vs Transformers backend for this model (
google/gemma-4-12B-it-qat-w4a16-ct, 1xH100, in/out = 1024/512):auto)This performance difference comes from the Transformers backend fusions not being generic enough for Gemma 4's shape, I will follow up to fix that.
Note
Model loading might be a little slow on Transformers
maindue to a known issue withper_layer_configlookups. This will be resolved in huggingface/transformers#47539 before the release of v5.15.0.