Remove obsolete target model adapters - #144
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b4b8f981f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| from torchspec.models.target.eagle3_target_model import ( | ||
| Eagle3TargetModel, | ||
| Eagle3TargetOutput, | ||
| HFTargetModel, | ||
| ) |
There was a problem hiding this comment.
Keep target adapters until callers are migrated
This deletion removes the torchspec.models.target package, but live code still imports it: HFRunner imports HFTargetModel in torchspec/inference/engine/hf_runner.py:38, EagleMooncakeStore.get() imports Eagle3TargetOutput in torchspec/transfer/mooncake/eagle_store.py:361, and the Eagle3/DFlash trainers import TargetLMHead in their target-head initialization paths. In any HF-backend run or trainer initialization, these unresolved imports now fail before model setup, so the adapters need to remain or all of those callers need to be migrated in this commit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 652fc315fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if markov_head_type == "vanilla": | ||
| return VanillaMarkov(vocab_size=config.vocab_size, markov_rank=markov_rank) | ||
| markov_head_type = str(getattr(config, "markov_head_type", "classical")).lower() | ||
| if markov_head_type == "classical": |
There was a problem hiding this comment.
Preserve the existing
vanilla DSpark alias
With any DSpark draft config/checkpoint saved before this change, markov_head_type is still serialized as the previous default "vanilla"; DSparkConfig.from_dict preserves that value, and DSparkDraftModel now reaches build_markov_head and raises NotImplementedError because only "classical" is accepted. Since the implementation is the same bigram-bias head, accepting "vanilla" as a legacy alias would avoid breaking existing experiments and checkpoints.
Useful? React with 👍 / 👎.
What changed
Validation
git diff --check