Shared-input Hessian dedup: plan metadata, E2E validation, and telemetry - #3052
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
:in=<tag>) + CPU forward probe:in=<tag>) + CPU forward probe
Review: P1 — do not infer shared-Hessian eligibility from subset grouping by defaultThe current default rule — same immediate parent + same expanded quantization subset/block => same input tensor — is not a safe semantic contract. The numeric subset/block historically describes quantization/execution grouping, not tensor identity. This PR itself demonstrates the problem: MLA definitions had modules in the same subset that receive different activations, so they needed explicit The risk becomes correctness-critical in the follow-up that skips follower The real-forward suite is good, but I recommend one of these safer contracts before wiring this into the looper:
The optimization is valuable, but the failure mode here is silent quantization error, so the default should be conservative rather than inferred from an unrelated grouping primitive. |
Review: P1 —
|
Review: P2 —
|
Review summaryI reviewed the planner/probe implementation, the model-definition annotations, the targeted synthetic tests, the tiny real-forward coverage, existing PR discussion, and current CI state. Verdict: changes requested before this becomes a Hessian-dedup contract. The implementation is well-tested structurally, but there are two correctness hazards that can turn into silent quantization error once follower Hessian collection is skipped:
I also flagged the probe API semantics because Current Ruff workflow is green; I did not find a CI failure driving these findings. The concerns are semantic/correctness issues in the new contract rather than formatting or test hygiene. |
…nflicting leaf metadata; strict probe ok - untagged modules are singleton groups; only same-parent same-:in= leaves dedup - collect_leaf_specs raises ValueError when a template is redefined with different flags - SharedInputProbeReport: has_errors / fully_verified; ok == fully_verified - :in= tags added only to definitions verified by the tiny CPU forward suite (llama family, qwen3, phi3, qwen3/qwen2 moe, mixtral, deepseek v3, glm4 moe, qwen3.5 dense/moe, qwen3-next, gpt-oss, llama4)
|
Re "P1 — do not infer shared-Hessian eligibility from subset grouping by default" (comment 5536991865): agreed, fixed in e7cfd32. Untagged modules are now singleton groups (key = module path); dedup only happens for same-parent leaves carrying the same |
|
Re "P1 — |
|
Re " |
…t groups - HessianConfig.dedup_shared_inputs (default True, dynamic-overridable) - GPTQ.adopt_hessian_from(leader): private fp32 copy of leader H/nsamples/fwd_counter - GPTQProcessor elects one leader per explicit group within a subset; follower hooks are no-ops; followers adopt after the forward, before coverage/quantize - only plain GPTQ tasks with matching columns participate; singletons/untagged never - stage_subset wires begin/end_shared_input_capture around hook install/removal - unit tests (adopt semantics, election, capture/adopt) + tiny-Llama CPU e2e: dedup on/off yields bit-identical quantized weights
:in=<tag>) + CPU forward probe:in=<tag> plan metadata + CPU probe + looper leader/follower capture
|
@ZX-ModelCloud Validate |
Follow-up validation reviewI reviewed the current head ( P1 —
|
…nt to same-subset followers P1: module_tree is inherited, so LlamaQModel's :in= tags enabled dedup for unverified Llama clones. Add BaseQModel.shared_input_verified_model_types (not inherited, checked via cls.__dict__) and shared_input_verified(); plans for unlisted model types ignore tags (explicit_tags=False). List exactly the model types covered by the CPU real-forward suite (+ gemma/granite/olmo2/ stablelm/cohere/cohere2 cases) and add a MODEL_MAP-wide regression test. P2: the looper elects leaders per subset block, so groups whose members span blocks never dedup. Add SharedInputGroup.dedup_followers/dedups_at_runtime/ members_in_subset and make SharedInputPlan.dedup_count count same-subset followers only; document it and test the cross-subset path end to end against GPTQProcessor.
|
Re follow-up validation review — both addressed in 48cc0d2. P1 — inherited tags. Added a non-inherited gate: P2 — cross-subset groups. Went with option 2 (scope to same-subset), since it matches what the looper already does and keeps the Hessian lifecycle simple. New
|
|
CPU e2e smoke of shared-input Hessian dedup (48cc0d2) — real
Probe detailsProbe: 3 calibration rows → 2 batches, so full-capture Devin session: https://app.devin.ai/sessions/62474a853aa048bf95738e1e03f06e20 |
:in=<tag> plan metadata + CPU probe + looper leader/follower capture:in=<tag> plan metadata + CPU probe + looper leader/follower capture
|
Re-review of head The new Validation of this update:
The earlier review findings remain resolved. This closes my outstanding finding; the validation above does not establish GPU runtime parity. Original review at 48cc0d2 — retained for context; P2 now resolvedReview of head P2 — Respect per-module Hessian accumulation settings when electing followersLocation: The election checks the explicit input group, exact GPTQ task type, For example, use a verified Llama q/k group with FP16 activations, This is a numerical difference, not just configuration bookkeeping: for one token with an FP16 input component Suggested fix: partition each shared-input group by compatible effective Hessian accumulation settings, or conservatively leave members with differing settings on independent capture. Include chunking settings in that decision because they determine the accumulation path/order. Add a regression using low-precision activations and different per-module staging settings, comparing the follower's H with dedup disabled. The existing default-FP32 tests and dynamic dedup-disable test do not exercise this case. Previous findingsThe current changes address the earlier reported issues: untagged leaves remain singletons; conflicting variant metadata raises; probe Validation
|
… accumulation settings Follower election now also requires matching hessian.staging_dtype, chunk_size and chunk_bytes, since these change the accumulated H numerically (e.g. bf16 staging vs an fp32 dynamic override). Members with differing settings fall back to independent capture.
|
Re #issuecomment-5548509642 (P2) — fixed in 08f797f. Follower election now also requires identical effective Hessian accumulation settings: Tests added in
|
:in=<tag> plan metadata + CPU probe + looper leader/follower capture:in=<tag> plan metadata
Add full dense and MoE quantize/save/reload/generate coverage, lifecycle telemetry, and GIL=0 synchronization for shared-input Hessian deduplication.
:in=<tag> plan metadata
Validation updateThis PR now includes full real-model E2E coverage and lifecycle telemetry for shared-input Hessian deduplication.
Hardware note: the validation host exposes one |
Summary
Model-definition metadata that identifies decoder-layer modules consuming the same input activation, and looper wiring so the GPTQ Hessian (
H = XᵀX) is collected once per group and copied to the other members (e.g.q/k/v_proj,gate/up_proj).Phase 1 (8bf42b7, e7cfd32): metadata + CPU probe. Phase 2 (e7ec666): looper dedup. Review round 2 (latest commit): per-model_type verification gate + same-subset
dedup_count.Contract (after review): dedup is explicit opt-in only, and only for real-forward-verified model types. Every quantizable module is a singleton group unless sibling leaves under the same parent carry the same
:in=<tag>and the definition listsmodel_config.model_typein its own (non-inherited)shared_input_verified_model_types. Subset digits are never used to infer sharing. Runtime dedup only happens between group members in the same subset block.What Changed
gptqmodel/models/shared_input.py::in=<tag>module_tree leaf flag (ordinary flag to existing parsers; does not alter emitted blocks).build_shared_input_plan(module_tree, layer_modules) -> SharedInputPlanwith frozenSharedInputGroup(key, parent, modules, subset_indices, explicit); helpersleader_for,followers_of,shares_input,is_explicit,for_subset,filter_modules,with_prefix,dedup_count.key == module path); same parent + same:in=<tag>-> shared (key == "<parent>:in=<tag>").:!/:?leaves excluded;experts.{i}expand per expert (routed experts never share with each other).build_shared_input_plan(..., explicit_tags=False)ignores all tags (singletons only).SharedInputGroup.dedup_followers= members that share a subset block with an earlier member (one leader per block);dedups_at_runtime,members_in_subset(i);SharedInputPlan.dedup_count = sum(len(g.dedup_followers)). A group spanning blocks (qwen3.5in_proj_qkv:0/in_proj_z:1, glm4_moe nestedshared_experts) is still probe-verified but contributes 0 — matching whatGPTQProcessoractually does.followers/leaderremain the structural view.collect_leaf_specs()raisesValueErrorwhen a leaf is repeated acrossmodule_treevariants with differentsubset_tag/input_tag/quantize; identical duplicates are fine.probe_shared_inputs(layer, plan, forward) -> SharedInputProbeReport: pre-hooks every planned module, runs a real forward, compares captured inputs (shape/dtype/values/call-count). Status:has_errors(mismatches or undeclared identical inputs),fully_verified(no errors, no missing modules, no uncalled groups),ok == fully_verified(strict).BaseQModel.shared_input_plan(model_config, quantize_config, is_awq_quantize=False)classmethod, gated byBaseQModel.shared_input_verified(model_config):module_treeis inherited (every Llama clone), the verification set is not, so DeciLM/Dream/Ernie4.5/InternLM/Instella/MobileLLM/Xverse/... and anymodel_typemerely mapped ontoLlamaQModelstay singleton-only until they get a real-forward case.model_config=None-> unverified.LlamaQModel {llama, mistral, gemma, granite, olmo2, stablelm, cohere, cohere2},Qwen2QModel {qwen2},Qwen3QModel {qwen3},Gemma2QModel {gemma2},Gemma3QModel {gemma3_text},Phi3QModel {phi3},Qwen3MoeQModel {qwen3_moe},Qwen2MoeQModel {qwen2_moe},MixtralQModel {mixtral},DeepSeekV3QModel {deepseek_v3},GLM4MoEGPTQ {glm4_moe},Qwen3_5TextQModel {qwen3_5_text},Qwen3_5_MoeTextQModel {qwen3_5_moe_text},Qwen3NextGPTQ {qwen3_next},GPTOSSGPTQ {gpt_oss},Llama4TextQModel {llama4_text}— exactly theCASESof the CPU forward suite. The image-text wrappers (Qwen3_5QModel,Qwen3_5_MoeQModel,Llama4QModel,Gemma3ForConditionalGeneration) keep their tags but are not verified (singleton plans) until covered.q_b_proj:…:in=q_a,kv_b_proj:…:in=kv_aas documented singletons — the probe showed they receive different tensors ((2,6,16)vs(2,1,6,16)).full_layer_modules()/simple_layer_modules()output unchanged for all definitions (:in=is transparent to block emission).Phase 2 — looper Hessian dedup
HessianConfig.dedup_shared_inputs: bool = True(serialized to quant meta; dynamic-overridable per module, e.g.{".*\\.v_proj$": {"hessian": {"dedup_shared_inputs": False}}}).GPTQ.adopt_hessian_from(leader):leader.materialize_global_hessian()then a private fp32 copy ofH(copy=True, follower's target device) plusnsamples/fwd_counter; clears follower partials. Raises on column mismatch. Independence matters becausequantize()mutatesHin place andfree()drops it.LoopProcessor.begin_shared_input_capture(model, subset_names, is_lm_head_module) -> {follower: leader}/end_shared_input_capture(subset_names): no-op defaults;GPTQProcessoroverrides them:model.shared_input_plan(model_config, quantize_config);explicitgroup: candidates = members present in this subset, with a task of type exactlyGPTQ(GPTAQ/FOEM excluded), per-modulehessian.dedup_shared_inputson, samecolumns; first in subset order leads, others follow; <2 candidates -> no dedup;pre_process_fwd_hook(name)returns a no-op hook for followers (leader/singleton hooks unchanged, incl. keep-mask splitting);end_shared_input_capture->follower.adopt_hessian_from(leader), marks follower as having captured input ids, clears election state;shared_input_dedup_counttracks adoptions.stage_subset._run_single_subset_pass:begin_…before hooks are installed (only whenexecute_forward),end_…right after hooks are removed and before coverage validation / worker quantization.Tests
tests/module_tree/test_shared_input.py: flag parsing, leaf-spec extraction incl. repeated nested keys, expert placeholder resolution, singleton default, explicit same-subset / cross-subset grouping, conflicting-variant metadata raises (tag,:!,:?, subset) while identical duplicates pass,:!/:?exclusion, block output unchanged with:in=, prefix/filter, concurrent determinism, MoE, every registered definition covers its quantizable paths and has no non-explicit shared group, synthetic probe pass/fail modes (shape, dtype, value, missing, uncalled, partial, multi-call, kwarg tensors, hook cleanup on success/exception, undeclared pairs,has_errors/fully_verified/strictok).tests/module_tree/test_shared_input_cpu_forward.py: tiny HF configs on CPU (hidden 32–64, 2 layers) throughconvert_model+ realmodel(input_ids)probe for Llama, Qwen2/3, Mistral, Gemma/Gemma2/3, Granite, OLMo2, StableLM, Cohere/Cohere2, Phi3, Qwen2/3-MoE, Mixtral, DeepSeek-V3, GLM4-MoE, Qwen3.5 dense + MoE, Qwen3-Next, GPT-OSS, Llama4. Every layer must befully_verifiedwithundeclared == ()and every shared groupexplicit; negative tests show a wrong MLA tag ->mismatches, an untagged q/k/v triple ->undeclared, a wrongly split q/k ->undeclared, and un-routed experts ->unverified(strictokFalse,has_errorsFalse).test_only_real_forward_verified_definitions_dedup[<every MODEL_MAP type>]asserts: verified <=> has a real-forward case (same class), and unverified types yieldshared_groups == ()/dedup_count == 0;test_verified_model_types_are_not_inheritedchecks aLlamaQModelsubclass and an unlisted model_type stay singleton-only.tests/test_shared_input_hessian_dedup.py(27, CPU): cross-subset group (qkv:0/z:1) drives the processor through all subset passes — no election, both members capture their ownH(equal values, distinct storage), andshared_input_dedup_count == plan.dedup_count;adopt_hessian_fromequal-but-independentH(mutating follower leaves leader intact), nsamples/fwd_counter carry-over, idempotent multi-follower, uncalled leader -> zeroH, self no-op, column mismatch raises,quantize()after adopt == independent collection; election: first-in-subset leads, singletons/untagged never, subset-restricted members, missing tasks,dedup_shared_inputs=False(global + dynamic), lm_head, model without plan API, non-plain-GPTQ excluded, column mismatch skipped, state reset on nextbegin; capture: followers get 0add_batchthen adopt, hooks resume normal afterend, dedup vs independentHallclose, keep-mask path,endwithoutbegin/ with pruned tasks.tests/test_shared_input_hessian_dedup_e2e.py: realGPTQModel.load+quantize()of a 2-layer tiny Llama (GQA, hidden 64) on CPU with dedup on vs off: elections are{k,v -> q}/{up -> gate}per layer, followers'fwd_counter == 0, and all 14 quantized weight tensors are bit-identical between the two runs.Pre-existing on
main(unrelated, also fail without this branch):test_moe_flag_parsing.py::TestMoEModuleName::test_get_moe_module_name_none_tree,test_subset.py::test_qwen3_5_moe_subset_early_stop…(needs CUDA).Review Requirements
Notes
Ready for review. Review round 1 (comments 5536991865 / 5536994939 / 5536996271) addressed in e7cfd32: singleton default + explicit opt-in, conflict detection across variants, strict
ok. Review round 2 (5539747040): P1 -> per-model_typeshared_input_verified_model_typesgate + MODEL_MAP coverage test (+6 Llama-clone real-forward cases); P2 -> chose "scope to same-subset":dedup_count/dedup_followersnow match the looper, README + tests updated. Cross-subset groups (qwen3.5in_proj_qkv:0/in_proj_z:1) intentionally stay as-is: reusing the leaderHacross passes would only skip oneadd_batchwhile keepingHalive longer, so it is not worth the lifecycle complexity. CPU e2e smoke (load+quantize, dedup on/off bit-identical, save/reload/generate) in comment 5547829240.Link to Devin session: https://app.devin.ai/sessions/62474a853aa048bf95738e1e03f06e20
Open in Devin Desktop: https://app.devin.ai/desktop/session/62474a853aa048bf95738e1e03f06e20?variant=devin
Requested by: @Qubitium