Skip to content

[Refactor][Model Loader] Share post-load module iteration order - #47

Open
aoshen02 wants to merge 2 commits into
fix/hpc-rope-norm-stale-refitfrom
refactor/post-load-iteration
Open

aoshen02 wants to merge 2 commits into
fix/hpc-rope-norm-stale-refitfrom
refactor/post-load-iteration

Conversation

@aoshen02

@aoshen02 aoshen02 commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Why

Cold-start process_weights_after_loading (utils.py) and layerwise reload finalize (layerwise.py) each hand-wrote the same visiting order: one pass over every module, then a second pass over the deferred attention-like layers (is_deferred_attention_layer), whose hook reads weights sibling layers may have decompressed/repacked. The only thing keeping the two loops from sharing code was the attention hook's extra act_dtype argument (TODO(lucas) in utils.py).

Under RFC vllm-project#54477 (selective weight reload), refresh_derived_state needs exactly this ordering for a third consumer. Defining it once now means that PR consumes the iterator instead of inventing a third copy.

What

  • New iter_post_load_modules(model) in model_loader/post_load.py yields (name, module, phase) in order: LAYER (every module), ATTENTION (deferred attention-like layers again), MODEL (the model, only if it defines the hook).
  • utils.process_weights_after_loading and finalize_layerwise_processing consume it. Cold start still runs the model-level hook last; reload still skips it — unchanged.
  • Attention / MLAAttention / MMEncoderAttention / Kimi-K3 MultiHeadLatentAttention: process_weights_after_loading(act_dtype=None); the layer's own activation dtype (captured at construction from torch.get_default_dtype(), which is what the loader passed before) is the default. Explicit dtype still accepted, so no caller breaks. Backend impl signature untouched.
  • finalize_layerwise_processing(model, model_config=None): model_config was only used to thread dtype to the attention hook; kept optional so the seven existing callers need no change.

Zero behavior change by construction: the LAYER pass is named_modules() exactly as before, so attention layers' quant_method step still runs in tree order, before any attention hook.

Stacked on

#46 (removes the HpcModule dispatch site; this PR then only has three sites to unify).

Not a duplicate

vllm-project#46828 adds a plugin registry for which module types count as deferred attention (and is based on pre-is_deferred_attention_layer code); it does not touch the signature or the duplicated ordering. Orthogonal.

Tests

.venv/bin/python -m pytest tests/model_executor/model_loader/test_reload.py -k "not test_reload_weights and not test_kv_scale_reload and not test_online_quantize_reload"
# 32 passed (28 existing + 4 new)
pre-commit run --files <8 changed files>   # ruff, mypy, all pass

New tests: iterator order contract; cold-start dispatch order through the real process_weights_after_loading (every quant method incl. attention's before any attention hook, hook called zero-arg, model hook once, last); layerwise finalize order with a partially loaded ordinary layer processed before deferred attention and no model hook; Attention hook forwards self.dtype unless overridden.

Not covered here: constructing a real MLAAttention / Kimi MLA under a non-default dtype (needs an attention backend; belongs in the GPU MLA tests).

Audit

Reviewed by Codex (gpt-6-astra) in three rounds; round 1 caught a cold-start ordering change (attention quant step moved) and round 2 a callable vs hasattr semantic drift and an HF-dependent ModelConfig() in a CPU test — all fixed, round 3 PASS.

AI assistance (Claude Code + Codex) was used; every line reviewed and tests run by the submitter.

🤖 Generated with Claude Code

aoshen02 and others added 2 commits September 6, 2026 07:44
HpcRopeNorm kept float32 mirrors of q_norm/k_norm weights that were only
populated by process_weights_after_loading(), which runs solely from
BaseModelLoader.load_model. Every reload path (reload_weights, NCCL/IPC/RDT
weight transfer) bypasses it, so after an RL refit the fused kernel kept
normalizing with the pre-refit weights. Sleep level-2 was worse: the mirrors
were Parameters rather than buffers, so wake_up left them uninitialized.

Populate the mirrors eagerly from a wrapped weight_loader on the fallback
norm weights instead (same pattern as kimi_k3 KDA decode_norm_weight), and
store them as non-persistent buffers defaulting to ones. HpcRopeNorm was the
only HpcModule subclass, so the dedicated HpcModule loop in
model_loader.utils.process_weights_after_loading goes away with it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Cold-start process_weights_after_loading and layerwise finalize each hand-wrote
the same visiting order: one pass over every module, then a second pass over the
deferred attention-like layers. Define it once in iter_post_load_modules() and
consume it from both; cold start still runs the model-level hook last and
reload still skips it, as before.

The attention-layer hook no longer needs act_dtype threaded through the loader:
layers already know their activation dtype, so the argument becomes optional and
both loops call a uniform zero-argument hook (resolves the TODO(lucas) in
utils.py).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Ao Shen <aoshen524@gmail.com>
@aoshen02
aoshen02 force-pushed the fix/hpc-rope-norm-stale-refit branch 8 times, most recently from 6f9ff7b to 7b50dd0 Compare September 8, 2026 01:41
@aoshen02
aoshen02 force-pushed the fix/hpc-rope-norm-stale-refit branch from 7b50dd0 to 0d4c4b9 Compare September 13, 2026 15:47
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