Conversation
Remove the hybrid (mamba/SSM) dependency from GraniteSwitch. The switch model is attention-only and never constructs a mamba layer, so it does not need the GraniteMoeHybrid family — GraniteMoeShared provides every class it actually uses (shared MLP, MoE, RMSNorm, RoPE, PreTrainedModel base) minus the mamba machinery. HF backend: - GraniteSwitchConfig now extends GraniteMoeSharedConfig; modeling and core/lora imports repointed to the granitemoeshared twins. - layer_types / position_embedding_type become switch-owned attributes (the shared parent does not declare them, but internal readers still depend on them). shared_intermediate_size fix: the shared parent defaults it to 0, which is also the "no shared MLP" sentinel for pure sparse-MoE bases. The config now resolves it itself instead of inheriting a magic default — explicit values (including 0) are honored verbatim; when unset, dense resolves to intermediate_size and pure MoE keeps 0. This is a compose-time decision frozen into config.json; it also closes a latent bug where a bare dense config silently inherited the old 1024 default. Guarded by new unit tests. composer: granite_moe_hybrid_arch/_sr_arch renamed to granite_moe_shared_arch/_sr_arch. The "granitemoehybrid" registry key is retained (mapped to the shared arch) because real Granite 4.x dense checkpoints are still typed granitemoehybrid upstream; a "granitemoeshared" key is added alongside. vLLM backend: - Removed the vestigial IsHybrid / HasInnerState marker mixins (no hybrid contract was implemented). - The two borrowed upstream classes (GraniteMoeMoE, GraniteMoeSharedMLP) now load via a version-tolerant helper that prefers the non-hybrid granitemoe / granitemoeshared modules and falls back to granitemoehybrid, so a single codebase works across the pinned vLLM versions. Local CPU tests pass (unit, config sis-trap, composer arch skinning, HF granite4 equivalence, HF forward/lora/multi-switch). vLLM and GPU generation tests to run on the cluster. Signed-off-by: antonp <antonp@il.ibm.com>
eval/gen_smoke.py loads a composed Granite Switch checkpoint in vLLM and generates the same question with the base path and with each adapter's control token, printing the outputs so a reviewer can confirm the de-hybridized (granitemoeshared) backend both loads and routes adapters. Used by the Vela validation job (vela_yamls/dehybridize_vllm_gen.yaml). Signed-off-by: antonp <antonp@il.ibm.com>
Signed-off-by: antonp <antonp@il.ibm.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Collaborator
Author
|
/gpu-test-multi |
❌ GPU tests failed —
|
❌ GPU tests failed —
|
Signed-off-by: antonp <antonp@il.ibm.com>
Collaborator
Author
|
/gpu-test-multi |
Signed-off-by: antonp <antonp@il.ibm.com>
Signed-off-by: antonp <antonp@il.ibm.com>
Signed-off-by: antonp <antonp@il.ibm.com>
Collaborator
Author
|
/gpu-test-multi |
Signed-off-by: antonp <antonp@il.ibm.com>
Collaborator
Author
|
/gpu-test-multi |
vLLM >=0.26 registers the fused expert bank via FusedMoEFactory at block_sparse_moe.experts.routed_experts.w13_weight / .w2_weight. The HF->vLLM stacked-MoE loader targeted experts.w13_weight (missing the routed_experts. segment), so the packed tensors were never loaded: vLLM reported them UNINITIALIZED and the fallthrough direct-load tripped "RoutedExperts.weight_loader() missing 3 positional args". Point the loader's w13/w2 targets at the routed_experts path and update the MoE weight-load test's readback to the routed_experts submodule. Verified against the 0.26 and 0.27 lines.
test_skinning_equivalence_vllm[granite-4.0-micro] and its _thorough variant compare the skinned GraniteSwitch model against the original through vLLM and gated the top-K logprobs at d > 0 -- literal bit-exactness. But the skinned model runs its projections through the fused SWITCH Triton kernel while the original uses vLLM's native linear, and the two have different float-reduction orders even at zero adapter (CLAUDE.md Gotcha #9). Whether that sub-ULP noise lands on zero is vLLM-version-dependent: both tests pass on 0.19.1 and 0.20 but fail on 0.26 with max |diff| = 1.907349e-06 (== 2^-19, i.e. 1-2 ULP at these logprob magnitudes). Every differing value across all 8 thorough requests was one of 2^-19 / 2^-20 / 2^-21 -- the reduction-order fingerprint, not drift. Gate at LOGPROB_ATOL = 1e-5 instead: ~5x above the observed noise floor, orders of magnitude below any real logit/weight regression (which shifts logprobs by >> 1e-3). This mirrors the philosophy already documented for the authoritative test_generation_equivalence.py, which gates distribution equivalence rather than token-exact match for exactly this reason. max_diff now tracks the true max including tolerated noise so drift toward the threshold stays visible even on a pass; the FAIL/OK prints and the module docstring drop the now-inaccurate "bit-exact" wording. Signed-off-by: antonp <antonp@il.ibm.com>
The zero-adapter / no-control-token vLLM equivalence tests asserted bitwise
identity (atol=0, rtol=0) between an inert GraniteSwitch and the upstream
GraniteMoeHybrid reference on identical transferred weights. That held only
while GraniteSwitch inherited GraniteMoeHybrid. After the de-hybridization
(GraniteMoeShared base), the inert forward runs an equivalent-but-different
reduction-order vLLM path, so the logprobs drift by ~1 bf16 ULP (measured
max_abs_diff 4.77e-7 == 2^-21, mean ~3e-8 across 4.0-mini {1b,350m,micro});
still far below model noise.
get_tolerances() now returns (1e-5, 1e-5) for the inert base-model path
instead of None (~20x the measured worst case, ~1000x tighter than the
adapter path, so a real logit regression still trips it), and the three
affected tests (TestGranite4FamilyEquivalence::test_logits_short/long,
TestZeroAdapterNoHiding::test_no_control_tokens) use it. Test-only change.
Signed-off-by: antonp <antonp@il.ibm.com>
… boundary Add CLAUDE.md Gotcha #13 explaining that shared_intermediate_size == 0 means skip the shared MLP (not build a zero-width one), and that the no-MLP-path rejection lives in the decoders, not the config. Add config-level tests pinning that boundary: pure-MoE excludes the shared_mlp LoRA targets, and a config with neither experts nor a shared MLP is permitted at config level.
The switch model is attention-only with unconditional RoPE, so both fields were redundant for every targeted Granite base (4.0-micro/4.1/4.2/5.0 all resolve to RoPE). layer_types was only ever consumed as a cache of a value transformers derives itself: DynamicCache(config=...) builds an all-full_attention per-layer layout from num_hidden_layers when layer_types is absent, so carrying it added a 32-default fallback that was wrong for every 40-layer target. position_embedding_type had no transformers-side reader and only gated our own RoPE construction. Remove both from GraniteSwitchConfig, the composer arch descriptors, and the DynamicCache-length synthesis; collapse the four decoder RoPE gates to always-on; replace the layer_types attention scan with the attention-only num_adapters>0 invariant. Tests now assert the config carries neither field and that DynamicCache derives the correct per-layer layout from num_hidden_layers, including across a from_dict round-trip (the load-from-config path). Signed-off-by: antonp <antonp@il.ibm.com>
… shim _upstream_layers.py was added in the de-hybridization reparent (7eb2138) as a version-tolerant wrapper: each getter tried the non-hybrid module (granitemoe / granitemoeshared) and fell back to granitemoehybrid on ImportError. That fallback is dead code across the supported vLLM range -- granitemoe.GraniteMoeMoE and granitemoeshared.GraniteMoeSharedMLP export directly on 0.19.1, 0.20.2, 0.26.0 and 0.27.1 (verified against the release tags), and granitemoehybrid merely re-imports from those two modules rather than owning the classes. So the try branch always wins and the wrapper reduced to an indirection around a one-line import. Inline the direct non-hybrid imports at the four call sites (has_experts / has_shared_mlp in both the LoRA and SR decoders) and delete the module. Behavior is identical; the switch keeps its non-hybrid sourcing (the point of de-hybridization) without the extra layer. Signed-off-by: antonp <antonp@il.ibm.com>
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
2 participants
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.
No description provided.