Repository navigation
Conversation
…evice DSpark's pseudo_speculative_generate adds its Markov bias, computed on the draft's device, to the backbone logits that the base model's lm_head returns. scripts/ar_validate.py loads with device_map="auto", and accelerate keeps a tied lm_head with embed_tokens on the first GPU while the draft can land on a later one. The logits then stay on the first GPU, so the sum raises "Expected all tensors to be on the same device" for every sample and AR validation reports nothing. This has been the case since DSpark was added. The backbone logits now move to the draft's device before the loop. On a single device the move is a no-op. Verified: a new GPU test dispatches the model with accelerate's dispatch_model, embed_tokens and lm_head on the CPU and everything else on cuda:0, and generates with DFlash and DSpark. DSpark fails without the fix and passes with it. The bitwise A/B against #2757 shows DSpark's drafted tokens unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: h-guo18 <67671475+h-guo18@users.noreply.github.com>
…nt and head HFDominoModel did not override pseudo_speculative_generate, so AR validation ran DFlash's, which decodes block positions 1..bs-1 with the backbone alone. DFlash trains position k on the token at anchor + k; Domino trains it on anchor + k + 1 (_label_shift = 1). Every drafted token was therefore compared with the target one position too early, so drafts were almost never accepted, and the correction head that training optimizes and the exporter ships was never applied. domino.yaml keeps AR validation off, so shipped training did not hit this. DFlash's default decode now starts at block position 1 - _label_shift and returns at most bs - 1 + _label_shift tokens. For DFlash, DFlash2 and LiLiCorr (shift 0) that is the same slice as before. Domino overrides it the way DSpark does for its Markov head: it decodes the block left to right, stepping the GRU over the anchor and the tokens drafted so far, and from pure_draft_prefix_len on adds embed_proj([hidden_k; GRU state]) to the backbone logits, as _apply_domino_head does in training. As in DSpark's decode, the backbone logits and the drafted tokens' embeddings are moved to the draft's device: under device_map="auto" the embedding table, and a tied lm_head with it, can sit on another GPU. The eval forward warned that Domino's acceptance rates were backbone-only, but that method computes no acceptance rate, and AR validation now runs the head. The warning goes, and so do the other copies of the claim: the comments in domino.yaml, dspark.yaml and the Nemotron-3.5 DSpark warm-start recipe, the note in the Domino launcher example, and the speculative-decoding skill's Domino, DSpark and validation sheets. DSpark's generation has applied its Markov head since DSpark was added. The Domino sheet and example now say how to add the HF AR eval: DFlash's task_2 with --steps equal to the block size, since Domino drafts the whole block. The flag values do not change. Verified: a new test checks that Domino's greedy drafts equal the argmax of _apply_domino_head teacher-forced on them, with head weights large enough to overturn the backbone. It fails on the parent commit, and also when the head is skipped, its GRU state lags a step, or the prefix boundary is off by one. The GPU test that puts lm_head on another device now covers Domino as well, and fails without the move. Bitwise A/B against #2757: DFlash and DSpark loss, gradients and drafted tokens are unchanged; only Domino's drafted tokens change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: h-guo18 <67671475+h-guo18@users.noreply.github.com>
DSpark and Domino train block position k on the token at anchor + k + 1 (HFDFlashModel._label_shift = 1). The same fact was also written as DSparkModule.shift_label = True, as DominoModule.shift_label read from the config, as a NotImplementedError in HFDominoModel.modify, and as the exporters' getattr(draft_config, "shift_label", True), which read the user's raw architecture config. A DSpark config with shift_label: false therefore trained with the shift but exported shift_label: false, so a loader that honors the field would read every drafted position one token off. Domino refused the same config, but only after building the draft. HFDFlashModel.modify now refuses shift_label: false for every variant whose _label_shift is 1, before anything is built; unset or true converts as before. Both exporters write shift_label from _label_shift. The modules' shift_label attributes and Domino's own check go, since nothing reads them any more. Domino's error becomes a ValueError, like the other convert-time config checks, and the skill's Domino and DSpark sheets list it. The refusal applies at convert only. Restore replays convert, so restore_dflash_model drops shift_label from the saved architecture config, as it already drops the warm-start path. A DSpark checkpoint saved with shift_label: false trained next-token alignment all the same: it still loads, and now re-exports shift_label: true. Verified: a new test converts DSpark and Domino with shift_label unset, as the recipes leave it, restores a modelopt_state that carries shift_label: false, and expects shift_label: false to be refused at convert. On the parent commit DSpark does not raise and Domino raises NotImplementedError; without the restore change the restore raises. A tiny DSpark checkpoint saved by 0.47.0 with shift_label: false loads through from_pretrained and exports shift_label: true. The existing export tests still read shift_label: true. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: h-guo18 <67671475+h-guo18@users.noreply.github.com>
… was built with DSparkModule resolves its head fields from the architecture config as it builds them: it lower-cases markov_head_type, casts markov_rank to int and use_confidence_head to bool, and fills in defaults for the ones left out. DSparkExporter read the raw config dict again with its own defaults. markov_head_type: Gated therefore trained the gated head but exported "Gated", which a serving loader that compares the name exactly does not recognize. DominoExporter likewise re-derived pure_draft_prefix_len with a second default. Both exporters now read the head fields off the built module, as LiLiCorrExporter already does for its reranker, so the exported config describes the head that the saved weights belong to. Verified: the DSpark export test now converts with markov_head_type "Gated" and expects "gated" in config.json. It fails on the parent commit. Domino's exported values are unchanged for every config, since DominoModule stores them as given. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: h-guo18 <67671475+h-guo18@users.noreply.github.com>
DSpark and Domino have shipped since 0.46.0, so these are fixes to released features, and two of them ask something of users: drop shift_label: false from DSpark configs, which now fail to convert, and re-export DSpark drafters whose export carried shift_label: false or a markov_head_type spelled in another case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: h-guo18 <67671475+h-guo18@users.noreply.github.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Type of change: Bug fix
Stacked on #2757 (its branch is the base). DSpark and Domino train block position
kon the token atanchor + k + 1, and DFlash, DFlash2 and LiLiCorr train it onanchor + k. #2757 made the class constant_label_shiftdrive the training targets. The same fact was still written in four more places:DSparkModule.shift_label = TrueDominoModule.shift_label, read from the configNotImplementedErrorinHFDominoModel.modifygetattr(draft_config, "shift_label", True), which re-read the user's raw architecture configThose copies, and the AR-validation decode, caused four bugs:
HFDominoModelinherited DFlash'spseudo_speculative_generate, which decodes block positions1..bs-1with the backbone alone.domino.yamlkeeps AR validation off, so shipped training was not affected.scripts/ar_validate.pyloads withdevice_map="auto". accelerate keeps a tiedlm_headwithembed_tokenson the first GPU, while the draft can land on a later one. DSpark added its Markov bias, computed on the draft's GPU, to backbone logits left on the first GPU, so every sample raised a device error. This dates from DSpark's addition, and Domino's new decode needs the same handling.shift_label: false. It trained with the shift anyway and exportedshift_label: false, so a loader that honors the field would read every drafted position one token off. Domino refused the same config, but only after building the draft.DSparkModulelower-casesmarkov_head_type, soGatedtrained the gated head but exported"Gated".markov_rank,use_confidence_headand Domino'spure_draft_prefix_lenwere also re-read, each with its own default.The fixes, one commit each:
1 - _label_shiftand returns at mostbs - 1 + _label_shifttokens. For shift 0 this is the same slice as before.pure_draft_prefix_lenon, it addsembed_proj([h_k; GRU state])to the backbone logits, as_apply_domino_headdoes in training. Like DSpark's decode, it brings the backbone logits and the token embeddings to the draft's device.domino.yaml,dspark.yamland the Nemotron-3.5 DSpark warm-start recipe, the Domino launcher example, and the speculative-decoding skill's Domino, DSpark and validation sheets. DSpark's generation has applied its Markov head since DSpark was added.task_2with--stepsequal to the block size, since Domino drafts the whole block. The flag values do not change.HFDFlashModel.modifyrefusesshift_label: falsefor every variant whose_label_shiftis 1, before anything is built. Unset ortrueconverts as before.restore_dflash_modeldropsshift_labelbefore it replays convert, as it already drops the warm-start path. DSpark checkpoints saved withshift_label: falsetherefore still load, and now re-exportshift_label: true.shift_labelfrom_label_shift. The modules'shift_labelattributes and Domino's own check are removed, since nothing reads them any more.DSparkExporterandDominoExporterreadmarkov_rank,markov_head_type,use_confidence_head,pure_draft_prefix_len,gru_hidden_dimandemb_dimoff the module, asLiLiCorrExporteralready does for its reranker.Usage
N/A. Bug fix with no API change.
Testing
TestDominoGenerate::test_drafts_are_the_training_head_argmax[0|1]checks that Domino's greedy drafts equal the argmax of_apply_domino_headteacher-forced on those drafts, forpure_draft_prefix_len0 and 1.test_next_token_variants_refuse_shift_label_false_at_convert[dspark|domino]converts both variants withshift_labelunset, as the recipes leave it. It then restores a modelopt_state that carriesshift_label: false, and expectsfalseto be refused at convert. On refactor(speculative): one home each for the DFlash family's shared rules #2757 DSpark does not raise and Domino raisesNotImplementedError; without the restore change, the restore raises.test_export_config_has_dspark_fieldsnow converts withmarkov_head_type: Gatedand expectsgatedinconfig.json. It fails on refactor(speculative): one home each for the DFlash family's shared rules #2757.TestLmHeadOffTheDraftDevice::test_generate[dflash|dspark|domino]uses accelerate'sdispatch_modelto putembed_tokensandlm_headon the CPU, standing in for the first GPU, and the layers and draft oncuda:0. Generation must return on the caller's device. DSpark fails on refactor(speculative): one home each for the DFlash family's shared rules #2757, and Domino fails without its device move.taskset -c 0-3).tests/unit/torch/speculativeandtests/unit/torch/export/test_hf_spec_rope_export.pyon CPU: 415 passed on each of transformers 5.5.4, 5.12.1 and 5.18.0 (411 on refactor(speculative): one home each for the DFlash family's shared rules #2757). 5.5.4 and 5.18.0 fall within CI'stf_minandtf_latestpins.tests/gpu/torch/speculative/pluginson 1 GPU: 17 passed, 1 skipped. The skipped test needs 2 GPUs to shard the base.ar_validate.py'sload_vlm_or_llmwith realfrom_pretrainedand accelerate dispatch, in both layouts transformers picks for 2 GPUs. The drafts equal the single-device drafts, andvalidate_onlinecompletes with the inputs on the first device, wherear_validate.pyputs them. Without the device move in Domino's decode, the same load crashes.shift_label: falseloads throughfrom_pretrainedand exportsshift_label: true.steps0, 2 and 4 in the online ones, in fp32 and bf16.steps=4, the drafts go from[[22, 8, 8], [17, 17, 17]](3 tokens, positions 1 to 3, backbone only) to[[8, 22, 8, 8], [21, 17, 17, 17]](4 tokens, positions 0 to 3, head applied). The old tokens reappear one position later: this is the off-by-one.fully_shardunit, one rank on the zero-loss branch): DFlash finishes on both ranks, with the same losses as on refactor(speculative): one home each for the DFlash family's shared rules #2757. This PR does not change training code.pre-commit run --files <changed files>: clean.Before your PR is "Ready for review"
Make sure you read and follow Contributor guidelines and your commits are signed (
git commit -s -S).Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded
trust_remote_code=True,torch.load(..., weights_only=False),pickle, etc.).shift_labelunset (ortrue, indomino.yaml), and checkpoints saved withshift_label: falsestill restore. Only a new conversion behaves differently: DSpark withshift_label: falsenow raises instead of training next-token alignment anyway and exportingfalse, and Domino raisesValueErrorfor that config instead ofNotImplementedError.CONTRIBUTING.md: N/AAdditional Information
Follow-up to #2757; review after it.
Found while testing, not fixed here. Both fail the same way on #2757:
DFlashModuleas its own FSDP2 unit, DSpark and Domino training fails withgot mixed torch.Tensor and DTensor.prefix_gruandembed_proj) run outsideDFlashModule.forward, so FSDP2 never unshards their parameters.DFlashModuleits own unit is the MiniMax-M2.7 DFlash one, so no shipped DSpark or Domino setup hits this.AcceptanceRateValidation.validate_onlinecompares the baselm_head's predictions with drafts returned on the caller's device. Whendevice_map="auto"puts an untiedlm_headon another GPU than the inputs, it raises a device error for every DFlash-family variant. This was reproduced on 1 GPU with the CPU as the first device, not on a 2-GPU host.🤖 Generated with Claude Code