Skip to content

Commit 0451356

Browse files
rahul-tulishanjiaz
andauthored
Fix verifier kwarg leaking into draft model constructors (#669)
<!-- markdownlint-disable --> > ⚠️ **DO NOT MERGE before #672.** This PR is stacked on #672 (remove eagle converter) and its base is `remove-eagle-converter`. Merging this first would pull in #672's changes prematurely and/or land against the wrong base. Merge order: **#672 first, then this.** Once #672 merges, I will rebase this onto `main`. The diff here is the 3-line loader change only. ## Purpose Fix `TypeError: Eagle3DraftModel.__init__() got an unexpected keyword argument 'verifier'` raised during draft model construction in `scripts/train.py --from-pretrained`. This breaks the EAGLE3 finetuning smoke test: ``` tests/e2e/smoke/test_finetuning_sanity.py::test_finetuning_weight_sanity TypeError: Eagle3DraftModel.__init__() got an unexpected keyword argument 'verifier' ``` `build_draft_model` passes `verifier=args.verifier_name_or_path` to `from_pretrained` (for the external-checkpoint auto-convert path added in #617). `SpeculatorModel.from_pretrained` had no `verifier` parameter, so it stayed in `**kwargs`, was forwarded to HF's `from_pretrained`, and reached the model constructor `cls(config, **kwargs)`. Draft models take only `config` and reject it. ## Fix Make `verifier` an explicit named parameter of `SpeculatorModel.from_pretrained` so it is consumed at the loader boundary (never forwarded into the constructor), used directly for auto-conversion, and forwarded through the base-class re-dispatch alongside `t2d`/`d2t`. With the attachment-style `EagleSpeculator` removed in #672, no model constructor takes `verifier` anymore, so consuming it is sufficient — no per-model special-casing needed. ## Tests - Failing test fixed: `tests/e2e/smoke/test_finetuning_sanity.py::test_finetuning_weight_sanity` (covered on CI). - `make style` / `make quality`: pass. - `pytest tests/unit/test_model.py tests/unit/convert/test_entrypoints.py`: 20 passed. ## Checklist I have filled in: - [x] The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)". - [x] The test plan/results, such as providing test command and pasting the results. - [ ] (Optional) The necessary documentation update. - [x] I (a human) have written or reviewed the code in this pr to the best of my ability. --------- Signed-off-by: shanjiaz <hezhao@redhat.com> Signed-off-by: Rahul-Tuli <rtuli@redhat.com> Co-authored-by: shanjiaz <hezhao@redhat.com>
1 parent bcdc38a commit 0451356

1 file changed

Lines changed: 3 additions & 1 deletion

File tree

src/speculators/model.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ def from_pretrained(
244244
weights_only: bool = True,
245245
t2d: torch.Tensor | None = None,
246246
d2t: torch.Tensor | None = None,
247+
verifier: str | None = None,
247248
**kwargs,
248249
) -> "SpeculatorModel":
249250
"""
@@ -319,7 +320,7 @@ def from_pretrained(
319320

320321
pretrained_model_name_or_path = maybe_convert_external_checkpoint(
321322
pretrained_model_name_or_path,
322-
verifier=kwargs.get("verifier"),
323+
verifier=verifier,
323324
cache_dir=cache_dir,
324325
config_dict=config_dict,
325326
)
@@ -362,6 +363,7 @@ def from_pretrained(
362363
weights_only=weights_only,
363364
t2d=t2d,
364365
d2t=d2t,
366+
verifier=verifier,
365367
**kwargs,
366368
)
367369

0 commit comments

Comments
 (0)