Skip to content

fix(tests): update run_layer_stage callers to the layer_names kwarg - #3038

Merged
Qubitium merged 1 commit into
ModelCloud:mainfrom
Anai-Guo:fix/stage-tests-layer-names-kwarg
Aug 31, 2026
Merged

Qubitium merged 1 commit into
ModelCloud:mainfrom
Anai-Guo:fix/stage-tests-layer-names-kwarg

Conversation

@Anai-Guo

Copy link
Copy Markdown
Contributor

Problem

run_layer_stage takes a keyword-only layer_names with no default:

# gptqmodel/looper/stage_layer.py
def run_layer_stage(
    looper, *, layers, layer_modules, planning_layer_modules,
    layer_names: Optional[List[str]],   # keyword-only, no default -> required
    fallback, shared_kv_cache_dict, pb, layer_count,
    region_timer, finalize_progress_cls, logger=None,
) -> None:

but three tests in tests/test_stage_modules.py still pass the old
layers_prefix="model.layers", so each dies with a TypeError before reaching
any assertion — it is both an unexpected keyword and a missing required one.

call site test binds?
tests/test_stage_modules.py:826 test_run_layer_stage_invokes_subset_stage TypeError
tests/test_stage_modules.py:1044 test_run_layer_stage_stops_after_last_quantized_layer binds — already migrated
tests/test_stage_modules.py:1268 test_run_layer_stage_reuses_subset_plan_for_replay TypeError
tests/test_stage_modules.py:2008 test_run_layer_stage_replays_untouched_layer_outputs_when_all_modules_skipped TypeError
gptqmodel/looper/module_looper.py:1533 production caller binds

So one test and the production caller were migrated; three tests were missed.

What the new parameter means

layers_prefix was a single prefix string; layer_names is one full layer
path per layer
. run_layer_stage resolves it per index and forwards the
result as create_named_modules' layers_prefix:

layer_name = get_layer_name(layer_names, layer_index)   # stage_layer.py
...
    layers_prefix=layer_name,                           # stage_layer.py

So the fixtures now pass ["model.layers.<i>"] lists sized by layer_count,
matching the already-migrated test at line 1044.

One dummy create_named_modules actually consumes layers_prefix and built
f"{layers_prefix}.{layer_index}.{name}". Since it now receives the full layer
path, the .{layer_index} segment is dropped. The strings it produces are
byte-identical before and after
, so the dynamic exclusion pattern that test
exercises (r"-:^model\.layers\.0\.") still matches exactly the same modules:

  layer 0 mlp.gate_proj    old='model.layers.0.mlp.gate_proj'  new='model.layers.0.mlp.gate_proj'  same
  layer 0 mlp.up_proj      old='model.layers.0.mlp.up_proj'    new='model.layers.0.mlp.up_proj'    same
  layer 0 mlp.down_proj    old='model.layers.0.mlp.down_proj'  new='model.layers.0.mlp.down_proj'  same
  layer 1 mlp.gate_proj    old='model.layers.1.mlp.gate_proj'  new='model.layers.1.mlp.gate_proj'  same
  layer 1 mlp.up_proj      old='model.layers.1.mlp.up_proj'    new='model.layers.1.mlp.up_proj'    same
  layer 1 mlp.down_proj    old='model.layers.1.mlp.down_proj'  new='model.layers.1.mlp.down_proj'  same
all full_name strings unchanged: True

The other two dummies accept layers_prefix but never read it, so they are
unaffected.

Verification

The signature was parsed out of stage_layer.py with ast, rebuilt as a stub,
and every run_layer_stage call site in the repo replayed through
inspect.Signature.bind, with the already-migrated test and the production
caller as the control group:

before:  826 -> TypeError: missing a required argument: 'layer_names'
        1044 -> binds
        1268 -> TypeError: missing a required argument: 'layer_names'
        2008 -> TypeError: missing a required argument: 'layer_names'
        3 of 4 test call sites cannot bind

after:   826 -> binds   1044 -> binds   1268 -> binds   2008 -> binds
         0 of 4 test call sites cannot bind
control: gptqmodel/looper/module_looper.py:1533 (production) -> binds

Caveat, stated plainly: I could not execute pytest tests/test_stage_modules.py
to confirm the three tests now pass their assertions. Importing gptqmodel on
this Windows box crashes with a fatal access violation inside
gptqmodel/utils/threadx.py, and the Linux box available to me has no CUDA
build of torch. So this PR is verified at the signature level plus the
string-equivalence argument above; please let CI run it before merging.

Notes

  • Test-only change: 4 lines in tests/test_stage_modules.py, no library code.
  • The branch is based on my fork's main tip rather than upstream main,
    because my token lacks the workflow scope and both merge-upstream and a
    direct push are refused for .github/workflows/code_quality.yml. The line at
    1044 that upstream has already migrated is deliberately left untouched here
    so the merge keeps upstream's version.

🤖 Generated with Claude Code

run_layer_stage takes a keyword-only `layer_names: Optional[List[str]]` with
no default, but three tests in tests/test_stage_modules.py still pass the old
`layers_prefix="model.layers"`, so each fails with TypeError (unexpected
keyword 'layers_prefix' / missing required keyword-only 'layer_names') before
reaching any assertion. The production caller in module_looper.py already
passes layer_names.

layer_names holds one full layer path per layer: run_layer_stage resolves it
per index via get_layer_name and forwards the result as create_named_modules'
layers_prefix. The fixtures now pass ["model.layers.<i>"] lists sized by
layer_count, and the one dummy create_named_modules that consumes
layers_prefix drops the now-duplicated .{layer_index} segment. The full_name
strings that dummy builds are byte-identical before and after, so the dynamic
exclusion pattern it exercises (r"-:^model\.layers\.0\.") still matches
exactly the same modules.
@Qubitium
Qubitium merged commit 862d7a4 into ModelCloud:main Aug 31, 2026
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.

2 participants