Skip to content

Conversation

@fynnsu
Copy link
Collaborator

@fynnsu fynnsu commented Oct 23, 2025

Currently, if verifier_attachment_mode is "full" or "train_only" we load the full verifier model on init.

However, for the "train_only" case, we want to only load the parts of the model that are needed. The logic to do so, is meant to reside in the subclasses of SpeculatorModel.

This change, makes it so that we don't auto-load the full verifier model if "train_only" is selected.
Additional changes:

  • SpeculatorModel.attach_verifier() no longer returns the loaded verifier (since it might not exist)
  • EagleSpeculator.attach_verifier() directly calls resolve_verifier() to load the verifier model and extract its components.
  • test_speculator_model_attach_verifier_invalid needed to be updated to reset the model between subtests because otherwise the verifier_attachment_mode gets set and kept by the previous try-excepts blocks.

@github-actions
Copy link

github-actions bot commented Oct 23, 2025

📦 Build Artifacts Available
The build artifacts (`.whl` and `.tar.gz`) have been successfully generated and are available for download: https://github.com/vllm-project/speculators/actions/runs/18918018372/artifacts/4409360027.
They will be retained for up to 30 days.
Commit: 2b17856

Copy link
Collaborator

@shanjiaz shanjiaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool

Copy link
Collaborator

@brian-dellabetta brian-dellabetta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bummer that we have to have all those # type: ignore[assignment,attr-defined] comments

@fynnsu
Copy link
Collaborator Author

fynnsu commented Oct 24, 2025

bummer that we have to have all those # type: ignore[assignment,attr-defined] comments

Yeah I'm honestly not too sure what the deal with those is. We might want to try removing them at some point, but it's also possible this eagle3 file will be removed in favor of the new training one

@fynnsu fynnsu enabled auto-merge (squash) October 29, 2025 18:18
@fynnsu fynnsu merged commit 5bb69ec into main Oct 29, 2025
13 checks passed
@fynnsu fynnsu deleted the fynnsu/reduce_verifier_load branch October 29, 2025 18:23
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.

4 participants