Add Domino draft model training#118
Conversation
955f7c3 to
2fbf244
Compare
|
Thanks for adding Domino support! This is exciting to see and very encouraging. |
b7af636 to
2175da7
Compare
|
Finished running some experiment on 8*H100 node, verified that implementation works. attached results in PR description, need review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d021fcc277
ℹ️ 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".
| objective_weights = weight_mask | ||
| if ( | ||
| self.loss_objective == "decay" |
There was a problem hiding this comment.
Honor dpace objective for Domino loss
When a Domino run is configured with training.dflash_loss_objective=dpace, DFlashModel.__init__ accepts that value but this forward path only applies the decay branch; objective_weights stays equal to the binary mask, so the requested D-PACE weighting is silently disabled for both base and final losses. This affects any Domino experiment that reuses the existing DFlash dpace objective override and makes its results incomparable to DFlash D-PACE runs; either implement the dpace branch here or reject the option for Domino.
Useful? React with 👍 / 👎.
|
@SeaTuKeMa thanks for the great work! I will run e2e over the weekend |
thanks!let me know if there is any updates! I could further iterate based on feedback @yubofredwang |
|
The code looks clean to me, is it possible to combine DOMINO with DSpark? |
d021fcc to
35b9c30
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35b9c30224
ℹ️ 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".
| ) | ||
| loss, accuracy, loss_pp, acc_pp, count_pp, aux_metrics = output | ||
| self._last_domino_aux_metrics = aux_metrics | ||
| return loss, accuracy, loss_pp, acc_pp, count_pp |
There was a problem hiding this comment.
Return the loss-components slot from Domino forward
Any Domino training step that reaches super()._train_step() will fail before backward because DFlashTrainer._train_step unpacks six values from self._forward(batch), while this override returns only five. The same contract is also used by inherited eval, so Domino runs configured through the new trainer dispatch raise ValueError: not enough values to unpack instead of training; return a sixth dict (even empty or the aux metrics) to preserve the DFlashTrainer interface.
Useful? React with 👍 / 👎.
Signed-off-by: TukeMa <fivedguy001@gmail.com>
35b9c30 to
28477d2
Compare
|
@yubofredwang any thoughts or feedbacks on this PR? |
|
This PR has been inactive for two weeks and has been closed due to inactivity. Feel free to reopen it if it is still relevant. |
|
Replacement PR: #142. |
Summary
Why
This adds training support for the Domino speculative decoding method while reusing TorchSpec's existing DFlash anchor sampling, masking, metrics, and distributed trainer path.
Addresses #114.
Usage
The run script accepts normal config overrides after the optional config path, for example:
Validation
pytest tests/test_domino.py tests/test_dflash.py -q(76 passed)ruff checkon all changed Python filesgit diff --checkGPU Phase 1
iter_0000501.iter_0000501GPU Phase 2
examples/qwen3-8b-domino-8h100/run.sh./workspace/checkpoints.iter_0000251.iter_0000501;latest_checkpointed_iteration.txtreported501.Scope
This PR implements training support. Serving/export integration for the fused Domino head remains follow-up work.