Trainer-agnostic fixes extracted from the fast-llm branch - #151
Open
jlamypoirier wants to merge 12 commits into
Open
Trainer-agnostic fixes extracted from the fast-llm branch#151jlamypoirier wants to merge 12 commits into
jlamypoirier wants to merge 12 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…name; on-demand faulthandler Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…node Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…le, skip bf16 quant on non-bf16 dtype Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When seq_packing=false, run_preprocessing_loop drains processed_entries_queue with a fixed-count popleft for train_batch_size entries without first checking the queue has enough. If the queue holds fewer than train_batch_size, the loop raises IndexError: pop from an empty deque and kills the preprocessor. The bug was masked on fast-llm because fast-llm always takes the use_fast_llm branch and never enters the unpacked code path. It surfaced as soon as we ran DeepSpeed/HTTP mode with seq_packing=false to work around the missing flash-attn wheel for torch 2.10. Guard with a length check and break out of the inner writing loop; the outer loop then refills the queue and retries. (cherry picked from commit 8893d2c)
…label - Extract the datasets>=4 trust_remote_code compat wrapper into a shared pipelinerl/domains/_datasets_compat.py; chartqa and math import it instead of each carrying a byte-identical copy. - Drop the invalid "bf16" token from the vLLM dtype allowlist in launch.py (vLLM uses "bfloat16"; "bf16" is not an accepted --dtype value). - Fix the unpacked-branch preprocess log to say "Unpacked microbatch". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jlamypoirier
added a commit
that referenced
this pull request
Jul 14, 2026
Bring the extracted trainer-agnostic fixes (PR #151) in as the base of the Fast-LLM integration so #140 stacks on top and its diff shows only the integration-specific delta. Conflicts (the review improvements #151 gained after extraction) resolved in favor of #151: dedup the datasets>=4 load_dataset shim into pipelinerl/domains/_datasets_compat.py (drop the now-dead `import inspect` from both loaders), drop the invalid "bf16" vLLM dtype token, and label the unpacked preprocess log correctly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extracted from the fast-llm integration branch; all apply to the existing DeepSpeed / HTTP path. - actor.py: retry on aiohttp.ServerDisconnectedError; skip (don't retry) 4xx ClientResponseError rollouts, emitting an empty result; drop groups whose rollouts all failed instead of publishing empties; run the blocking result-queue put in an executor; relax the group-size assert to 1..attempts. - launch.py: drop the vLLM-v1-rejected num-scheduler-steps kwarg; give each actor vLLM a distinct VLLM_PORT base to dodge get_open_port()'s TOCTOU race. - async_llm.py: treat finish_reason == "abort" (weight update in progress) as a retryable TimeoutError. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
stats/lag reports last_broadcasted_version - min_actor_version, i.e. the staleness of the oldest sample in the batch (a max). Add stats/lag_min (vs the newest sample) and stats/lag_mean (batch mean) so the full lag distribution is visible. The mean accumulates over non-sentinel batches only, since sentinel batches carry the newest version and would bias it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- preprocess.py: the rollout-robustness changes let a group publish fewer than `attempts` rollouts (a 4xx-skipped or empty rollout is filtered out), but _check_group_sizes required exactly group_size and raised ValueError, crashing the preprocessor. Accept 1..group_size, matching the actor's group-size assert. - async_llm.py: remove the unreachable `finish_reason == "abort"` block (the retry loop already handles aborts via _is_retryable_abort_response, and make_training_text raises RetryableAbortedCompletionError) and its now-unused `import asyncio`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Log min/mean/max training lag
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
2 participants
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.
Trainer-agnostic fixes and improvements developed on the
fast-llmintegration branch (#140) but independent of the Fast-LLM trainer itself. Extracting them here so they can land onmainon their own and shrink the integration PR.Description prepared with Claude Opus 4.8 (Claude Code).
Changes
datasets>=4compat — a sharedload_datasetwrapper (pipelinerl/domains/_datasets_compat.py) forwardstrust_remote_codeonly when the installeddatasetsaccepts it; imported by the chartqa + math loaders.finetune.adam_beta1/adam_beta2threaded throughget_optimizer(adamw_torch/cpuadam).actor.py) — retry onaiohttp.ServerDisconnectedError; treat 4xx rollout responses as skip-not-retry (emit an empty result); drop groups whose rollouts all failed rather than publishing empties; offload the blocking result-queueputto an executor; accept1..attemptsrollouts per group.preprocess.py— accept groups of1..group_sizein the dataset loader's size check, so a skipped/dropped rollout no longer crashes preprocessing withInvalid group sizes; guard the unpacked-mode (seq_packing=false)popleftagainst a partial queue.launch.py— usesys.executablefor subprocess launches; resolve an absolute redis--dirand add--logfile(redischdirs to--dirbefore opening the logfile, so a relative path breaks); skip thebf16_last_layer_fp32quantization override when vLLM runs at a non-bf16 dtype; drop the vLLM-v1-rejectednum-scheduler-stepskwarg; give each actor vLLM a distinctVLLM_PORTbase to avoidget_open_port()port races.world.py— readGPUS_PER_NODEfrom the environment instead of hardcoding 8 for multi-node.WANDB_API_KEY_PATHlogin; resilientpython_envcollection (skips distributions with missing metadata); a component-suffixed W&B run name; on-demandfaulthandler(SIGUSR1 all-thread dump, no periodic timer); log training lag as astats/lag_min/stats/lag_mean/stats/lag_maxtrio instead of a single value.Notes
wandb.wandb_name,finetune.adam_beta*) exist onmain. Not yet runtime-tested — worth a quick smoke before merge.🤖 Generated with Claude Code