Refine reasoner regression assert rtol & refine workflow setting - #18
Merged
Conversation
The reasoner (llava_ov_datapacker) asserts all 10 loss iters, but iter-0 is bit-exact (pure forward on a seed-fixed batch/init, identical H100== H200) while iters 1+ drift run-to-run (non-deterministic Hopper FMHA backward + streamed LLaVA-OneVision data). Add a per-spec `strict_iters` knob: the leading `strict_iters` are held to the tight default 1e-3, the rest use the spec's loose band. Set the reasoner to strict_iters=1 with a 5e-2 tail band — so iter-0 catches a forward/init/first-batch regression tightly while iters 1-9 absorb the drift (two H200 samples differ by at most ~0.006 there, ~8x margin). The generator (vision_sft_nano) is unchanged: strict_iters=0, so all 10 iters still go through the single 1e-3 check.
Force dataloader_train.num_workers=0 (plus prefetch_factor/persistent_ workers unset) for the reasoner so the streaming LLaVA-OneVision data is fully ordered single-process — the only run-to-run noise left is then the FMHA backward kernel, not worker interleave order. An empirical 2-run num_workers=0 check still drifts ≤0.006 on iters 1-9, confirming the residual noise is the (non-deterministic) Hopper FMHA backward. Recapture the reasoner goldens to the num_workers=0 series (the data changed: iter-0 0.88798 -> 1.06924) and tighten the tail band from 5e-2 to 2e-2 now that data-order variability is removed. Tiered tolerance is now iter-0 1e-3, iters 1-2 1e-2, iters 3-9 2e-2 (worst observed tail diff 0.0057, ~3.5x margin). Also consolidate the duplicated comments. Generator (vision_sft_nano) and both smoke tests are unchanged.
CI runs on H200 but the goldens were H100-captured, leaving the tight iter-2 band at only ~1.9x margin. Recenter the reasoner loss goldens on the midpoint of two green H200 CI runs so each iter's worst-case deviation roughly halves (iter-2 worst diff 0.0058 -> 0.00365 vs its 1e-2 band, ~3x margin). Both H200 runs now use at most ~33% of their band. iter-0 is bit-exact across H100/H200 so it's unchanged (1.06924).
Make pre-commit.yml a reusable workflow (workflow_call) and add a pre-commit job to each GPU workflow that the GPU job needs:. The lint runs on ubuntu-latest first and the self-hosted H200 job is skipped if it fails, so the single expensive runner is never spent on a lint-failing commit. Standalone pre-commit triggers are kept so it still covers pull requests to non-main branches.
Replace the four separate GPU workflow files with a single gpu-tests.yml holding one pre-commit job (reusable workflow_call) plus the four GPU jobs, each `needs: pre-commit`. Pre-commit now runs ONCE and all four GPU jobs wait on that single run (skipped if lint fails), instead of each workflow spinning up its own pre-commit (previously 5 lint runs). The standalone pre-commit.yml is kept for lint coverage on non-main PRs.
lfengad
enabled auto-merge (squash)
June 4, 2026 08:50
yy-code-nv
approved these changes
Jun 4, 2026
lfengad
pushed a commit
that referenced
this pull request
Jul 21, 2026
…follow-up) Vision SFT trains no action tokens, so the recipe should not build the action head — the same recipe-scoped override PR #18 applied to Edge. NANO_MODEL_CONFIG keeps action_gen=True (the released checkpoint ships real DROID-trained action weights and the action-policy recipes rely on it); vision_sft_nano deepcopies it into _NANO_VISION_MODEL_CONFIG with action_gen=False, and vision_sft_nano_mapstyle_dataloader inherits via its existing deepcopy of the composed recipe. Accepted behavior change: vision-SFT'd Nano checkpoints and their HF/diffusers exports no longer carry the pretrained action heads (previously frozen pass-throughs, outside optimizer.keys_to_select). Start from the base checkpoint or the action-policy recipes if you need them. Loss is unchanged: the graph-consistency dummy forward contributed 0.0 * finite = 0. nano_training_smoke_test.py's diffusers golden comparison now ignores the action-only action_* tensors (action_proj_in/action_proj_out/ action_modality_embed) the export no longer contains, via the same prefix filter already used for the sound-only audio_* tensors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Uh oh!
There was an error while loading. Please reload this page.