Converters refactor [base implementations]: base converter architecture with registry-based algorithm detection #105

markurtz · 2025-08-31T22:04:28Z

Summary

Introduces a new registry-based converter architecture for transforming external research model checkpoints into the standardized Speculators format. The refactor replaces the previous hardcoded conversion logic with an extensible, generic converter system that supports automatic algorithm detection and provides a unified interface for various speculative decoding implementations (EAGLE, HASS, etc.).

Details

Added base converter architecture (base.py):
- SpeculatorConverter abstract base class with registry mixin for automatic converter resolution
- Generic type support for configuration and model types
- Automatic algorithm detection when algorithm="auto" is specified
- Standardized conversion pipeline with validation capabilities
Enhanced CLI interface (main.py):
- Improved parameter ordering and documentation with proper type annotations
- Added support for additional Hugging Face download parameters (cache_dir, force_download, etc.)
- Expanded algorithm support to include "auto", "eagle2", and "hass" options
- Better error handling and user experience with structured help text
Refactored conversion entrypoints (entrypoints.py):
- Rewrite of convert_model function to use the new converter architecture
- Enhanced parameter validation and error messaging
- Integration with Hugging Face utilities for model/config resolution
- Improved logging and user feedback throughout the conversion process
Code quality improvements:
- Added from __future__ import annotations for modern type annotation support
- Updated linting configuration to allow TC003 for typing imports
- Comprehensive docstring updates with proper parameter documentation
- Better separation of concerns between CLI, conversion logic, and converter implementations

Test Plan

Expanded and added unit tests to for the full functionality

Copilot

Pull Request Overview

This PR introduces a new registry-based converter architecture that replaces hardcoded conversion logic with an extensible system supporting automatic algorithm detection. The refactor standardizes the conversion of external research model checkpoints (EAGLE, HASS, etc.) into the Speculators format through a unified interface.

Key changes include:

Implementation of a registry-based converter system with automatic algorithm detection
Enhanced CLI interface with improved parameter handling and additional Hugging Face download options
Refactored conversion pipeline with better error handling and validation capabilities

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/speculators/convert/entrypoints.py	Complete rewrite of the conversion entry point with new converter architecture integration
src/speculators/convert/converters/base.py	New abstract base converter class implementing registry pattern and generic type support
src/speculators/convert/converters/init.py	Module initialization exposing the base converter interface
src/speculators/main.py	Enhanced CLI with improved parameter ordering, documentation, and additional download options
pyproject.toml	Updated linting configuration to allow TC003 for typing imports

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/speculators/convert/entrypoints.py

src/speculators/convert/converters/base.py

src/speculators/__main__.py

github-actions · 2025-08-31T22:06:46Z

📦 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/17595084537/artifacts/3968962354.
They will be retained for up to 30 days.
Commit: 8435f3f

Signed-off-by: Mark Kurtz <[email protected]>

Co-authored-by: Copilot <[email protected]> Signed-off-by: Mark Kurtz <[email protected]>

Signed-off-by: Mark Kurtz <[email protected]>

…/CLI to utilize it Signed-off-by: Mark Kurtz <[email protected]>

Co-authored-by: Copilot <[email protected]> Signed-off-by: Mark Kurtz <[email protected]>

Signed-off-by: Mark Kurtz <[email protected]>

…rters/base-entrypoints

Signed-off-by: Mark Kurtz <[email protected]>

rahul-tuli

LGTM pending comments!

rahul-tuli · 2025-09-10T11:28:24Z

src/speculators/convert/entrypoints.py

+    config: str | os.PathLike | PreTrainedModel | PretrainedConfig | dict | None = None,
+    verifier: str | os.PathLike | PreTrainedModel | None = None,
+    validate_device: str | torch.device | int | None = None,
+    algorithm: Literal["auto", "eagle", "eagle2", "hass"] = "auto",


Should we add eagle3 here?

rahul-tuli · 2025-09-10T11:31:19Z

src/speculators/convert/entrypoints.py

+    algorithm_kwargs: dict | None = None,
+    cache_dir: str | Path | None = None,
+    force_download: bool = False,
+    local_files_only: bool = False,


What's the rationale behind adding a local_files_only pathway?

rahul-tuli · 2025-09-10T11:32:27Z

src/speculators/__main__.py

+                "The source repo/algorithm to convert from into the matching algorithm "
+                "in Speculators"
+            ),
+            click_type=click.Choice(["auto", "eagle", "eagle2", "hass"]),


Same, is leaving out eagle3 intentional?

markurtz requested review from anmarques, dsikka, rahul-tuli and Copilot August 31, 2025 22:04

markurtz self-assigned this Aug 31, 2025

Copilot AI reviewed Aug 31, 2025

View reviewed changes

src/speculators/convert/entrypoints.py Show resolved Hide resolved

src/speculators/convert/converters/base.py Show resolved Hide resolved

src/speculators/convert/converters/base.py Outdated Show resolved Hide resolved

src/speculators/__main__.py Outdated Show resolved Hide resolved

dsikka requested a review from shanjiaz September 2, 2025 15:04

markurtz force-pushed the features/converters/transformers-utils branch from f8de1f7 to 622462c Compare September 4, 2025 22:49

markurtz force-pushed the features/converters/base-entrypoints branch from 23fe6c8 to 6e27c36 Compare September 4, 2025 23:00

markurtz force-pushed the features/converters/transformers-utils branch 5 times, most recently from 50eb873 to a3c0f80 Compare September 9, 2025 16:00

markurtz force-pushed the features/converters/base-entrypoints branch from 6e27c36 to 193edda Compare September 9, 2025 18:54

markurtz changed the base branch from features/converters/transformers-utils to features/converters/config-updates September 9, 2025 18:55

markurtz and others added 8 commits September 9, 2025 12:57

Updates for model.py, tests, and formatting

25b751c

Signed-off-by: Mark Kurtz <[email protected]>

Update src/speculators/model.py

02163d7

Co-authored-by: Copilot <[email protected]> Signed-off-by: Mark Kurtz <[email protected]>

Rebase, fixes from review, fixes to enable tests to pass

c6b87b2

Signed-off-by: Mark Kurtz <[email protected]>

fix test failures

a68c257

Signed-off-by: Mark Kurtz <[email protected]>

Add base converter implementation with registry and adapt entrypoints…

837a366

…/CLI to utilize it Signed-off-by: Mark Kurtz <[email protected]>

Apply suggestion from @Copilot

95caf94

Co-authored-by: Copilot <[email protected]> Signed-off-by: Mark Kurtz <[email protected]>

Apply suggestion from @Copilot

ad11336

Co-authored-by: Copilot <[email protected]> Signed-off-by: Mark Kurtz <[email protected]>

Add unit tests

d7918aa

Signed-off-by: Mark Kurtz <[email protected]>

markurtz force-pushed the features/converters/base-entrypoints branch from 193edda to d7918aa Compare September 9, 2025 18:57

markurtz and others added 2 commits September 9, 2025 12:58

Merge branch 'features/converters/config-updates' into features/conve…

ac0be41

…rters/base-entrypoints

Updates to enable backwards compatibility

8435f3f

Signed-off-by: Mark Kurtz <[email protected]>

rahul-tuli approved these changes Sep 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Converters refactor [base implementations]: base converter architecture with registry-based algorithm detection #105

Converters refactor [base implementations]: base converter architecture with registry-based algorithm detection #105

Uh oh!

markurtz commented Aug 31, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 31, 2025 •

edited

Loading

Uh oh!

rahul-tuli left a comment

Uh oh!

rahul-tuli Sep 10, 2025

Uh oh!

rahul-tuli Sep 10, 2025

Uh oh!

rahul-tuli Sep 10, 2025

Uh oh!

Uh oh!

Converters refactor [base implementations]: base converter architecture with registry-based algorithm detection #105

Are you sure you want to change the base?

Converters refactor [base implementations]: base converter architecture with registry-based algorithm detection #105

Uh oh!

Conversation

markurtz commented Aug 31, 2025

Summary

Details

Test Plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rahul-tuli left a comment

Choose a reason for hiding this comment

Uh oh!

rahul-tuli Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

rahul-tuli Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

rahul-tuli Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Aug 31, 2025 •

edited

Loading