chore(sync): publish SFT, diffusion and agentic updates - #317
Merged
NINGBENZHE merged 34 commits intoSep 15, 2026
Merged
Conversation
SigureMo
requested review from
Aurelius84,
NINGBENZHE,
Yangruipis and
yxyOo
as code owners
September 15, 2026 08:23
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Nyanpasu 审查看板审查状态： 🚧 需要修改 审查版本： 已完成 aba6247 的增量复核：两处测试兼容性改动未引入新发现，既有六项问题仍未解决，保留原需要修改的评审。本轮仅在 sigma 原线程补充固定版本源码，澄清正常完整轨迹已包含末步 timestep，未重复提交评审。本地 S3 测试因缺少 yaml 在初始化阶段受阻；GPU/NPU 集成测试和镜像构建仍未运行。
Powered by Nyanpasu with gpt-6-astra medium, please check the suggestions carefully.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ca2fee2cb
ℹ️ 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".
Comment on lines
+321
to
+325
| self._processor_pool = ProcessorPool( | ||
| self.config.hf_checkpoint, | ||
| pool_size=prefetch_num_workers, | ||
| trust_remote_code=True, | ||
| ) |
There was a problem hiding this comment.
When RELAX_SFT_TQ_SHARDS > 1 enables _SFTBatchProducerActor, this processor pool omits multimodal_config=MultimodalConfig.from_args(self.config), unlike the local producer. Consequently, configured image token and resize limits are silently ignored in sharded multimodal SFT, potentially producing different tokenization or oversized encoder inputs that OOM; the processor pool in _init_remote_eval_pipeline has the same omission.
Useful? React with 👍 / 👎.
| task_type=task_type, | ||
| num_labels=getattr(self.config, "num_labels", None), | ||
| problem_type=getattr(self.config, "problem_type", "single_label_classification"), | ||
| classification_sentinel_token_id=classification_sentinel_token_id, |
There was a problem hiding this comment.
With sharded async prepacking and a separate --eval-prompt-data dataset, this remote-eval constructor leaves loss_last_turn_only and loss_ignore_empty_think at their defaults, while both the training dataset and local eval path pass the configured values. Enabling either option therefore makes evaluation use different masking semantics from training and silently reports incomparable PPL metrics.
Useful? React with 👍 / 👎.
Comment on lines
+3063
to
+3066
| resolved[key] = { | ||
| "model_path": spec["model_path"], | ||
| "num_gpus": spec["num_gpus"], | ||
| "num_gpus_per_engine": spec.get("num_gpus_per_engine") or args.genrm_num_gpus_per_engine, |
There was a problem hiding this comment.
Validate that each JSON num_gpus and num_gpus_per_engine is a positive integer and that the former is divisible by the latter before accepting the instance. For example, num_gpus=1 with num_gpus_per_engine=2 passes the current resource-sum check but later computes num_slots = 0, so the service can report healthy while every request fails with no GenRM engines; other non-divisible values silently reserve unused GPUs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
补充一个各实例 GPU 数都合法且可整除、仍会启动失败的情况：每节点 8 卡，实例 A 为 2 卡/TP2，实例 B 为 8 卡/TP8，总预算 10 卡。当前前缀偏移使 B 从 bundle 2 开始，GenRMManager._resolve_placement 最终生成 base_gpu_id=2, nnodes=1, tp_size=8，尝试使用单节点 GPU 2–9。
已通过实际验证与放置方法复现。因此除这里的正数/整除检查外，还需要按物理节点边界安排或验证每个引擎的连续 GPU 区间；仅验证总数和单实例整除仍不够。
| except Exception as _e: | ||
| import logging as _logging | ||
|
|
||
| _logging.getLogger(__name__).warning("Failed to import relax.models.gemma4: %s", _e) |
There was a problem hiding this comment.
Route this import-failure warning through relax.utils.logging_utils.get_logger(__name__) rather than a direct stdlib logger. This code executes during model-package import, so bypassing the lazy project logger can lose or misformat the only diagnostic explaining why Gemma registration was disabled, and the repository explicitly prohibits logging.getLogger.
AGENTS.md reference: AGENTS.md:L53-L54
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Synchronizes 33 development commits into main, delivering SFT prepacking, native diffusion RL, expanded agentic/NeMo Gym support, Gemma 4 and GenRM updates, plus reliability fixes.
Changes:
- Adds asynchronous SFT runtime validation and checkpoint rotation safeguards.
- Introduces native diffusion rollout, trajectory replay, FlowGRPO training, and multimodal preprocessing.
- Extends agentic protocols, recipes, model adapters, logging, and distributed runtime behavior.
File summaries
| File | Description |
|---|---|
| tests/utils/test_sft_runtime_env.py | Updated as part of this pull request. |
| tests/utils/test_rotate_ckpt.py | Updated as part of this pull request. |
| tests/utils/test_megatron_peft_utils.py | Updated as part of this pull request. |
| tests/utils/test_logging_utils.py | Updated as part of this pull request. |
| tests/utils/test_arguments_sft.py | Updated as part of this pull request. |
| tests/utils/multimodal/test_image_utils.py | Updated as part of this pull request. |
| tests/utils/data/test_processor_pool.py | Updated as part of this pull request. |
| tests/utils/data/test_processor_pool_downcast.py | Updated as part of this pull request. |
| tests/utils/data/test_identity_window_sampler.py | Updated as part of this pull request. |
| tests/tools/test_process_tool_chat.py | Updated as part of this pull request. |
| tests/tools/test_docformatter_compat.py | Updated as part of this pull request. |
| tests/test_s3_shm_consumer_sites.py | Updated as part of this pull request. |
| tests/test_s3_model_loader.py | Updated as part of this pull request. |
| tests/test_model_source.py | Updated as part of this pull request. |
| tests/test_agentic_rollout.py | Updated as part of this pull request. |
| tests/models/qwen_image/init.py | Updated as part of this pull request. |
| tests/models/gemma4/test_attention.py | Updated as part of this pull request. |
| tests/models/init.py | Updated as part of this pull request. |
| tests/examples/nemo_gym_agentic/test_run_agent_app.py | Updated as part of this pull request. |
| tests/examples/nemo_gym_agentic/test_result.py | Updated as part of this pull request. |
| tests/examples/nemo_gym_agentic/test_r2e_test_layout.py | Updated as part of this pull request. |
| tests/examples/nemo_gym_agentic/test_convert_dataset.py | Updated as part of this pull request. |
| tests/examples/nemo_gym_agentic/test_client.py | Updated as part of this pull request. |
| tests/engine/sft/test_runtime.py | Updated as part of this pull request. |
| tests/engine/sft/dataset/test_multimodal.py | Updated as part of this pull request. |
| tests/engine/sft/dataset/test_ms_last_round.py | Updated as part of this pull request. |
| tests/engine/sft/dataset/test_chat_template.py | Updated as part of this pull request. |
| tests/engine/rewards/test_pickscore.py | Updated as part of this pull request. |
| tests/distributed/ray/test_weight_sync.py | Updated as part of this pull request. |
| tests/distributed/ray/test_utils.py | Updated as part of this pull request. |
| tests/distributed/ray/test_teacher_manager.py | Updated as part of this pull request. |
| tests/distributed/ray/test_scale_out.py | Updated as part of this pull request. |
| tests/distributed/ray/test_generative_reward.py | Updated as part of this pull request. |
| tests/distributed/ray/conftest.py | Updated as part of this pull request. |
| tests/distributed/checkpoint_service/test_optional_megatron_import.py | Updated as part of this pull request. |
| tests/core/test_controller_s3_model_cleanup.py | Updated as part of this pull request. |
| tests/core/test_controller_global_restart.py | Updated as part of this pull request. |
| tests/core/test_control_plane_affinity.py | Updated as part of this pull request. |
| tests/components/test_rollout_weight_update_handshake.py | Updated as part of this pull request. |
| tests/backends/sglang/test_sigterm_eviction.py | Updated as part of this pull request. |
| tests/backends/sglang/test_image_patch.py | Updated as part of this pull request. |
| tests/backends/megatron/weight_update/test_lora_weight_sync.py | Updated as part of this pull request. |
| tests/backends/megatron/test_model_provider_vpp.py | Updated as part of this pull request. |
| tests/backends/megatron/test_megatron_patch_contents.py | Updated as part of this pull request. |
| tests/backends/megatron/test_frozen_weight_dgrad.py | Updated as part of this pull request. |
| tests/backends/megatron/test_data_vpp.py | Updated as part of this pull request. |
| tests/backends/megatron/test_actor_http_timeout.py | Updated as part of this pull request. |
| tests/backends/fsdp/init.py | Updated as part of this pull request. |
| skills/agentic-rollout/references/partial-and-async-lifecycle.md | Updated as part of this pull request. |
| skills/agentic-rollout/references/context-linearity.md | Updated as part of this pull request. |
| scripts/training/text/run-qwen35-35B-A3B-16xgpu-async.sh | Updated as part of this pull request. |
| scripts/training/sft/run-qwen3-vl-4B-pokemon-8xgpu.sh | Updated as part of this pull request. |
| scripts/training/multimodal/run-qwen35-9B-8xgpu-openr1mm-async.sh | Updated as part of this pull request. |
| scripts/tools/process_tool_chat.py | Updated as part of this pull request. |
| scripts/tools/kill_for_ray.sh | Updated as part of this pull request. |
| scripts/tools/kernel_cache.py | Updated as part of this pull request. |
| scripts/tools/generate_openapi.py | Updated as part of this pull request. |
| scripts/models/gemma4-31B.sh | Updated as part of this pull request. |
| scripts/models/gemma4-26B.sh | Updated as part of this pull request. |
| scripts/entrypoint/spmd-multinode.sh | Updated as part of this pull request. |
| scripts/entrypoint/ray-job.sh | Updated as part of this pull request. |
| scripts/entrypoint/local.sh | Updated as part of this pull request. |
| requirements.txt | Updated as part of this pull request. |
| relax/utils/types.py | Updated as part of this pull request. |
| relax/utils/training/ppo_utils.py | Updated as part of this pull request. |
| relax/utils/s3_model_loader.py | Updated as part of this pull request. |
| relax/utils/rotate_ckpt.py | Updated as part of this pull request. |
| relax/utils/multimodal/image_utils.py | Updated as part of this pull request. |
| relax/utils/logging_utils.py | Updated as part of this pull request. |
| relax/utils/genrm_client.py | Updated as part of this pull request. |
| relax/utils/env.py | Updated as part of this pull request. |
| relax/utils/data/identity_window_sampler.py | Updated as part of this pull request. |
| relax/models/qwen_omni/qwen3_omni_bridge.py | Updated as part of this pull request. |
| relax/models/qwen_image/init.py | Updated as part of this pull request. |
| relax/models/gemma4/gemma4_bridge.py | Updated as part of this pull request. |
| relax/models/gemma4/init.py | Updated as part of this pull request. |
| relax/models/dots_ocr/megatron/bridge.py | Updated as part of this pull request. |
| relax/models/init.py | Updated as part of this pull request. |
| relax/entrypoints/train.py | Updated as part of this pull request. |
| relax/engine/sft/predict/runner.py | Updated as part of this pull request. |
| relax/engine/sft/dataset/multimodal.py | Updated as part of this pull request. |
| relax/engine/sft/dataset/gemma4_chat_template_patch.py | Updated as part of this pull request. |
| relax/engine/rollout/data_source.py | Updated as part of this pull request. |
| relax/engine/filters/dynamic_sampling_filters.py | Updated as part of this pull request. |
| relax/distributed/ray/placement_group.py | Updated as part of this pull request. |
| relax/distributed/ray/actor_group.py | Updated as part of this pull request. |
| relax/core/service.py | Updated as part of this pull request. |
| relax/core/optional_roles.py | Updated as part of this pull request. |
| relax/components/rollout.py | Updated as part of this pull request. |
| relax/components/advantages.py | Updated as part of this pull request. |
| relax/backends/megatron/weight_update/hf_weight_iterator_bridge.py | Updated as part of this pull request. |
| relax/backends/megatron/megatron_patch/init.py | Updated as part of this pull request. |
| relax/backends/megatron/loss.py | Updated as part of this pull request. |
| relax/backends/fsdp/init.py | Updated as part of this pull request. |
| relax/agentic/rollout.py | Updated as part of this pull request. |
| relax/agentic/pipeline/reward.py | Updated as part of this pull request. |
| relax/agentic/pipeline/init.py | Updated as part of this pull request. |
| pyproject.toml | Updated as part of this pull request. |
| examples/on_policy_distillation/agentic_opd/alfworld/sglang-2p2d-tp2-qwen35-35B-A3B.yaml | Updated as part of this pull request. |
| examples/on_policy_distillation/agentic_opd/alfworld/sglang-1p1d-tp2-qwen35-35B-A3B.yaml | Updated as part of this pull request. |
| examples/on_policy_distillation/agentic_opd/alfworld/run_agent_app.sh | Updated as part of this pull request. |
| examples/on_policy_distillation/agentic_opd/alfworld/reward_alfworld.py | Updated as part of this pull request. |
| examples/on_policy_distillation/agentic_opd/alfworld/app/agent.py | Updated as part of this pull request. |
| examples/nemo_gym_agentic/test/test_verbose_logging.py | Updated as part of this pull request. |
| examples/nemo_gym_agentic/test/test_result.py | Updated as part of this pull request. |
| examples/nemo_gym_agentic/test/test_prepare_r2e_gym.py | Updated as part of this pull request. |
| examples/nemo_gym_agentic/test/test_gateway_app.py | Updated as part of this pull request. |
| examples/nemo_gym_agentic/test/conftest.py | Updated as part of this pull request. |
| examples/nemo_gym_agentic/test/init.py | Updated as part of this pull request. |
| examples/nemo_gym_agentic/service/run_adapter.py | Updated as part of this pull request. |
| examples/nemo_gym_agentic/service/patches/r2egym_test_layout.patch | Updated as part of this pull request. |
| examples/nemo_gym_agentic/service/patches/r2egym_test_layout_setup.patch | Updated as part of this pull request. |
| examples/nemo_gym_agentic/service/patches/openhands_r2e_runtime.patch | Updated as part of this pull request. |
| examples/nemo_gym_agentic/service/patches/claude_code_agent_pinned_install.patch | Updated as part of this pull request. |
| examples/nemo_gym_agentic/scripts/run_gateway.sh | Updated as part of this pull request. |
| examples/nemo_gym_agentic/scripts/run_agent_app.sh | Updated as part of this pull request. |
| examples/nemo_gym_agentic/scripts/convert_dataset.py | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/workplace-assistant/start_workplace_assistant_gym.sh | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/workplace-assistant/README.md | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/workplace-assistant/PITFAIL.md | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/reasoning-gym-cc/configs/reasoning_gym_cc.yaml | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/reasoning-gym-cc/configs/claude_settings.json | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/r2e-gym/submit_r2e_gym.sh | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/r2e-gym/run-qwen35-9B-8xgpu-nemo-gym-r2e.sh | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/r2e-gym/PITFAIL.md | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/gsm8k/start_gsm8k_gym.sh | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/gsm8k/README.md | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/gsm8k/PITFAIL.md | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/calendar/start_calendar_gym.sh | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/calendar/run-qwen3-4B-8xgpu-nemo-gym-calendar.sh | Updated as part of this pull request. |
| examples/nemo_gym_agentic/recipes/calendar/README.md | Updated as part of this pull request. |
| examples/nemo_gym_agentic/app/result.py | Updated as part of this pull request. |
| examples/nemo_gym_agentic/app/client.py | Updated as part of this pull request. |
| examples/diffusion/qwen_image/t2i_lora.yaml | Updated as part of this pull request. |
| examples/diffusion/qwen_image/t2i_full.yaml | Updated as part of this pull request. |
| examples/diffusion/common/lora.yaml | Updated as part of this pull request. |
| examples/diffusion/common/full_ft.yaml | Updated as part of this pull request. |
| examples/diffusion/assets/qwen-image-ft-lora-pickscore-summary.json | Updated as part of this pull request. |
| examples/deepeyes/run_deepeyes.sh | Updated as part of this pull request. |
| docs/zh/guide/sft-training.md | Updated as part of this pull request. |
| docs/zh/guide/low-rank-adaptation-training.md | Updated as part of this pull request. |
| docs/zh/api/genrm.md | Updated as part of this pull request. |
| docs/public/openapi/genrm.json | Updated as part of this pull request. |
| docs/en/guide/sft-training.md | Updated as part of this pull request. |
| docs/en/guide/low-rank-adaptation-training.md | Updated as part of this pull request. |
| docs/en/api/genrm.md | Updated as part of this pull request. |
| docs/.vitepress/config.mts | Updated as part of this pull request. |
| docker/patch/sglang/v0.5.15.post1.patch | Updated as part of this pull request. |
| docker/Dockerfile | Updated as part of this pull request. |
| AGENTS.md | Updated as part of this pull request. |
| .pre-commit-hooks/gitleaks_tracked.py | Updated as part of this pull request. |
| .pre-commit-hooks/docformatter_compat.py | Updated as part of this pull request. |
| .pre-commit-config.yaml | Updated as part of this pull request. |
| .gitleaks.toml | Updated as part of this pull request. |
Review details
- Files reviewed: 126/319 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+42
to
+44
| "enabled": false, | ||
| "failIfUnavailable": false, | ||
| "allowUnsandboxedCommands": true |
| set -euo pipefail | ||
|
|
||
| : "${GYM_HOST:?GYM_HOST must be reachable from every Relax worker}" | ||
| export NEMO_GYM_CALLBACK_ALLOWED_NETWORKS="${NEMO_GYM_CALLBACK_ALLOWED_NETWORKS:-10.0.0.0/8}" |
Comment on lines
+181
to
+182
| tokenizer = None | ||
| processor = None |
There was a problem hiding this comment.
复核后补充触发范围：rollout_max_prompt_len 和 rollout_max_context_len 的默认值均为 None，两个 diffusion 启动脚本及配套 YAML 也未设置它们，因此默认示例不受影响。显式设置非空长度限制时，eager Dataset 的字符串 prompt 路径确实会调用空 tokenizer 并抛出 TypeError；streaming 路径则捕获异常并保留样本。建议按这个条件修复并补测试。
# 🐛 Bug Fix ## Include remote model config in start telemetry - Read config.json with the resolved S3 access policy - Attach metadata to a copied start config without mutating training arguments - Continue startup safely when remote metadata is unavailable --- # ✅ Tests ## Cover S3 config loading and telemetry behavior - Verify endpoint, credentials, addressing style, object key, and body cleanup - Cover invalid config payloads, local models, and remote read failures (cherry picked from commit 912b5a8049e1be6159af06e640e8436afd58981f)
# 🐛 Bug Fix ## Avoid legacy SGLang S3 processor failures - initialize the fully-async rollout engines with dummy weights - rely on the mandatory Actor weight sync before the first rollout (cherry picked from commit 67b0e09fb30b77b1675c1e5bc894498d198776d6)
# 🐛 Bug Fixes - `docker/patch/sglang/v0.5.15.post1.patch:401`: the hunk header declared `@@ -1322,6 +1331,20 @@` but the body contains 6 context + 16 added = **22** new lines. `git apply` refuses to parse the file entirely: ```console $ git apply --numstat docker/patch/latest/sglang.patch error: corrupt patch at line 424 ``` Because `--numstat` only parses (it never touches a work tree), this is a defect in the patch file itself, independent of any sglang version. Every image build with `ENABLE_SGLANG_PATCH=1` fails at `docker/Dockerfile:147`. Introduced by `73206573 feat(LoRA): Support LoRA RL MoE`, which hand-edited the `/post_process_weights` hunk without updating its line count, and reached `dev` through merge `f49950c2`. Fixed the count to 22, and shifted the following hunk's new-side start from 1474 to 1476 — the extra 2 lines move every later hunk in that file, and it is the only one. `git apply` tolerates a stale start line (it searches for the context), so the count alone is enough to make the patch apply; correcting the offset keeps the file identical to what `git diff` regenerates. Verified against a real `v0.5.15.post1` worktree: `git apply --check` exits 0, and all 99 hunk headers now match a canonical regeneration byte for byte. Co-Authored-By: Claude <noreply@anthropic.com> (cherry picked from commit 6806055fd36f58f1f45f3e8936e21f1bbbb8e08c)
(cherry picked from commit 2d5f1cd141a03ae291d1645c0ccb3f6a3fccdced)
# 🐛 Bug Fix ## Serialize elastic removal with weight updates - Claim DRAINING before waiting for active fully-async or scale-out weight transfers - Reject new fully-async handshakes while graceful removal owns the topology fence - Persist SIGTERM eviction intent across explicit scale-in timeout and cleanup failures ## Batch simultaneous graceful evictions - Collect ready logical engines from one poll and claim the full batch before waiting - Remove each server batch with concurrent Router, DCS, and shutdown phases plus one drain wait - Keep failed engines fenced without blocking successful peers from cleanup ## Coordinate explicit scaling and graceful eviction - Atomically recheck and insert scale requests against lifecycle claims - Let target scale-in adopt matching pending evictions without duplicate cleanup - Retry unmatched URL-target evictions after the explicit request reaches a terminal state - Recover ACTIVE scaled groups while leaving DRAINING and REMOVING groups untouched ## Keep graceful eviction on the live-actor path - Make the SGLang SIGTERM handler publish eviction intent without blocking or performing I/O - Reuse Router, drain, DCS, actor shutdown, and placement-group cleanup for scale-in and eviction - Poll eviction probes per reference and keep elastic ranks monotonic across scale cycles --- # ✅ Tests ## Cover lifecycle and batch coordination - Verify batch claim-before-wait, single drain, concurrent removal phases, and isolated failures - Verify persistent intent, request ordering, target adoption, handshake rollback, and recovery filtering - Verify DCS failure stays out of the hard-kill path and SIGTERM only publishes intent (cherry picked from commit 61e2cddc4f5fee2c8ffaaa8fcca57b28f0d77e6e)
# 🐛 Bug Fix ## Normalize extreme Qwen-VL images before processing - Detect Transformers Qwen-VL image processors through their inner processor MRO - Expand only the short side when an image exceeds the hard 200 aspect-ratio limit - Preserve normal images and non-Qwen processor behavior --- # ✅ Tests ## Cover Qwen-VL ratio normalization - Test horizontal, vertical, boundary, and idempotent resize cases - Verify processor workers preserve image order and isolate non-Qwen processors (cherry picked from commit 25a3bf116981c6b69048eb21e737f8341c6fd5f7)
The mcore upgrade in 2d5f1cd1 (20260506-85bced0ae -> 20260728-0e6ac576f) dropped the whole multi_token_prediction.py block. Most of it was correct to drop -- upstream had absorbed it -- but two capabilities regressed. 1. Three-way detach routing became a silent no-op. Upstream only has a single coarse `mtp_detach_heads`, and it defaults to False (transformer_config.py:88). Relax's configure_mtp_detach_paths (relax/backends/megatron/model_provider.py:55) still setattr's mtp_detach_embedding / _backbone / _lm_head, which nothing reads anymore. So --mtp-detach-paths (default: detach all three) and --mtp-only-training silently did the opposite of what they declare: the MTP auxiliary loss backpropagated into embedding, backbone and lm_head. No error, args still accepted, logs still emitted. Re-routes upstream's three detach sites to the per-path flags, keeping upstream's own implementations: - process_mtp_loss -> mtp_detach_lm_head - _get_embeddings -> mtp_detach_embedding - MultiTokenPredictionBlock -> mtp_detach_backbone The backbone site also regains `offset == 0` and `.requires_grad_(True)` from the old patch. Under VPP, offset > 0 means hidden_states came from a previous MTP stage rather than the main backbone, so upstream's unconditional detach severs gradient flow between MTP layers. 2. Logged MTP loss was missing the scaling factor. mtp_loss_scale is defined after save_loss_to_tracker upstream. Moved it ahead and applied it to the logged sum. Verified save_loss_to_tracker (multi_token_prediction.py:461) only normalizes -- it absorbed the old patch's safe-divide but never applied the scaling factor -- so there is no double scaling on either the per-token or microbatch-normalized branch. Note this shifts MTP loss curves by mtp_loss_scaling_factor (default 0.2) relative to runs on the current image. Deliberately NOT restored, having verified each landed upstream: the functional_call-based lm-head detach (upstream detaches output_weight directly), the _checkpointed_forward non-tensor rewrite (upstream captures via closure), the labels-is-None early return (upstream derives labels from input_ids), bridge/peft/utils.py create_peft, tensor_parallel/layers.py dgrad fold, qwen35_vl_bridge.py packed MTP experts, and the yarn position-embedding choice. Also left alone: MultiTokenPredictionBlock.__init__'s grad_norm_group tag, which is an upstream addition the old patch never had. tests/backends/megatron/test_mtp_only_training.py: 37 passed (was 36 passed, 1 failed). test_frozen_weight_dgrad.py::test_megatron_patch_carries_dgrad_fold still fails -- that one is a stale guard for a fix now shipped by upstream and is tracked separately. Co-Authored-By: Claude <noreply@anthropic.com> (cherry picked from commit ec45bc8e08bbedc637609aa78694d3637c99492a)
# ⭐ Feature ## Add opt-in asynchronous SFT prepacking - Add --sft-async-prepack to offload complete TransferQueue shard fetches, sequence-length balancing, CPU THD packing, pinned-memory staging, and H2D copies from the training thread - Preserve the standard SFT buffer-level partitioning, loss scaling, and oversized-sample behavior while reconciling the DP-wide micro-batch count before forward/backward - Prefetch one future rollout window with explicit identity and error propagation, and overlap its first H2D copy on a dedicated stream - Add PrepackedBatch and recursive move, pin, and record-stream helpers so the training path can consume prepared batches without repeating packing work - Validate the supported dynamic-batch, per-rank-fetch, THD, PP, CP, VPP, and routing-replay configuration and auto-enable the lookahead depth required for overlap - Add a Qwen3 0.6B recipe and enable async prepacking in the Qwen3-VL Pokemon recipe --- # 🐛 Bug Fix ## Make distributed sampling and SFT execution robust - Attach total_lengths custom metadata to SFT partitions and validate the producer contract before token-budget sampling - Wait for an equal-size DP-ready round before balancing streaming samples, preventing sequence partition assertions when fewer samples than DP ranks are available - Keep async SFT prepacking on SeqlenBalancedSampler and lazy-load the streaming sampler only for fully-async dynamic batching - Repartition real samples to the DP-wide maximum micro-batch count instead of executing expensive full-batch dummy work - Synchronize prefetch errors across ranks, fail fast on pinned-memory failures, and keep copy-stream tensor lifetimes valid - Weight SFT rollout metrics by local sample count and create collective statistics on the active training device - Cache the Megatron main-rank role before SFT prediction tears down reloadable process groups --- # ⚡ Performance ## Reduce SFT input-pipeline stalls - Move CPU packing and pinned-memory preparation off the training thread - Pipeline first-batch and subsequent H2D copies without racing the training copy stream - Avoid streaming tail polling for SFT by fetching one complete balanced local shard per window --- # ✅ Tests ## Cover sampler and SFT data contracts - Add equal-size DP-round and missing-custom-metadata regression tests for the streaming sampler - Verify SFT producers publish sequence lengths and collective statistics use the training device - Pass 24 targeted SFT, sampler, iterator, component, and Megatron data tests - Pass pre-commit run --all-files --show-diff-on-failure (cherry picked from commit d5d2b1b567931135cf9891d89940ac94236baf73)
(cherry picked from commit d2713039635dfd614e438c5cff33e3f1814174c8)
- Add RELAX_SFT_TQ_SHARDS helpers and SFT shard partition naming. - Run multiple remote shard producers for async prepack when more than one shard is requested. - Keep the single-shard path local so existing behavior stays unchanged by default. - Fetch shard partitions rank-locally in the Megatron prepack path and concatenate them before training consumption. - Patch Qwen chat templates with generation markers so tokenizer-aware assistant masks avoid the slow fallback path. - Keep async prefetch from falling back to foreground work and support tensor-list conversion into NestedTensor batches. --- - Add coverage for shard partition helpers, SFT remote shard producer control flow, Qwen template patching, async prefetch behavior, and tensor-list NestedTensor conversion. (cherry picked from commit ef126e597835021c4dc1f8dd8dadf4279d8c8c72)
# 🐛 Bug Fix ## Re-prime current index across epoch boundaries - Include the current epoch-boundary index when resetting SFT prefetch order. - Re-prime on every crossed epoch so async prefetch does not wait for an index that was omitted from the worker queue. - Return immediately from PrefetchBuffer.wait_for when no prefetch thread has been started. --- # ✅ Tests ## Cover prefetch wait edge cases - Add regression coverage for async SFT prefetch epoch-boundary re-priming. - Add coverage for wait_for(timeout=None) before set_index_order starts a worker. (cherry picked from commit 9635f298dc42fc380f262b3b501d1ec73bfd1d70)
# 🐛 Bug Fix ## Synchronize sharded prepack runtime - Register and propagate RELAX_SFT_TQ_SHARDS to every Ray actor. - Recheck the final cached sample after the bounded prefetch worker exits. --- # ✅ Tests ## Cover rebase-sensitive SFT paths - Verify target-only fallback and per-node model preparation for remote producers. - Exercise producer re-priming across steps and all-shard Megatron consumption. - Cover typed shard configuration propagation and the worker-exit cache race. (cherry picked from commit c1640dc27e31362a91092f76032ef7e75629e77d)
# ✅ Tests ## Restore pytest-asyncio coverage - Replace per-call asyncio.run wrappers with native await expressions. - Keep sharded producer steps on one event loop and align with the target branch test style. (cherry picked from commit 5a8d24a178b8f343cf84dd2f34b40be9c23ba91f)
(cherry picked from commit 7744c284a7c685249fa0de2de6f6bfb269432bb4)
(cherry picked from commit 0c806f6f83144fbdeb453dcf2da58144117f1b52)
# 🐛 Bug Fix ## Allow Pillow to decode truncated image payloads - Enable Pillow's process-wide truncated image compatibility for multimodal loaders - Prevent SFT producers from failing on recoverable image tail truncation --- # ✅ Tests ## Cover truncated JPEG loading - Verify a JPEG with four missing trailing bytes is decoded successfully (cherry picked from commit 0ec114c897e4f17b3f06182f775d55ec624de511)
# ✨ Features
`_render_per_message_fallback` hardcoded ChatML (`<|im_start|>{role}\n` …
`<|im_end|>`). That is fine for Qwen, but any non-ChatML model dies on the very
first sample with
RuntimeError: could not locate 'user' message after cursor 0 in rendered
chat template output
because the scan finds no headers. Path 1 (`{% generation %}`) is not an escape
hatch either -- Qwen3's own template has no such marker, so ChatML models rely on
this fallback too.
Extract the delimiters into a `_Dialect` record and select it from what the
template actually rendered. Behaviour for ChatML is byte-for-byte unchanged.
## Added: gemma-4
Frames turns as `<|turn>model\n … <turn|>` (note: the assistant role renders as
`model`) and delimits reasoning with `<|channel>thought … <channel|>`. Unlike
ChatML's fixed-length `<think>\n` opener, the reasoning span has a closing
marker, so the mask resumes after `<channel|>` rather than after a fixed offset.
Mirrors THUDM/slime's `gen_multi_turn_loss_mask_gemma4`.
Tool messages are gated per dialect: gemma-4's tool-call framing differs and is
not implemented, so it raises rather than silently mis-masking.
# ✅ Verification
Against gemma-4-31B-it's stock template:
- single turn -- loss covers exactly `'The weather is nice today.<turn|>\n'`;
bos, `<|turn>user\n…`, and the `<|turn>model\n` header are all excluded
- multi turn -- `'a1<turn|>\na2<turn|>\n'`, both replies and neither prompt
- reasoning block detected and the mask resumes after `<channel|>`
Qwen3 regression: dialect still resolves to chatml, loss still covers
`'…</think>\n\nworld<|im_end|>\n'` with the user turn excluded.
An end-to-end 40-step SFT run on gemma-4-31B produced an identical loss curve to
the previous approach (4.3340 -> 1.1633 vs 1.1602, bf16 noise), confirming the
rendered training sequence is unchanged.
(cherry picked from commit c6ca8d385221170d37e914e32195678adc3c353c)
(cherry picked from commit 199b956a4b45553f849fde27828c7011efa3932f)
- Add FSDP2 full and LoRA training backends for Qwen-Image - Add native SGLang diffusion rollout, reward, and in-memory trajectory transport - Add transactional weight synchronization, checkpointing, offload, and resume support - Add launch recipes, data tools, evaluation assets, and bilingual documentation --- - Validate trajectory contracts and replay the final valid diffusion transition - Coordinate distributed failures, RNG persistence, checkpoint publication, and rotation - Preserve optional Megatron imports and existing text-RL defaults --- - Keep trajectories and local reward images in memory - Stream bucketed weights directly to rollout engines - Prune stale rollout objects and artifact directories --- - Add unit coverage for FSDP lifecycle, LoRA, checkpointing, rollout, rewards, and weight updates - Add SGLang patch and Qwen-Image replay contract tests (cherry picked from commit cff14b1057edb65c89051e912a9f6822b4dcfdbe)
Let a custom advantage function return one value per turn instead of a single scalar. SessionForest records each response node's token span, and the reward pipeline expands the per-turn values across those spans, so training receives dense rewards shaped exactly like the log-probs and needs no turn-level branch of its own. Co-Authored-By: wulumeng <wulumeng@xiaohongshu.com> (cherry picked from commit 76c540526f6e890982b6cad1d0b8d4cf266fe48d)
- Add --sft-loss-last-turn-only (ms-swift loss_scale=last_round): supervise only the final learnable round; applies to both the generation-marker and per-message fallback render paths
- Add --sft-ignore-empty-think (ms-swift ignore_empty_think): keep an empty <think></think> block fully out of the loss on the fallback path; warn once when unsupported on the generation-marker path
- Reproduce ms-swift/Megatron two-stage sample order in IndexManager: persistent HF dataset shuffle followed by per-epoch torch.randperm sampler permutation
- Add --sft-train-data-prefetch: while training step N, prefetch step N+1 raw TransferQueue payload on a CPU worker; collective agreement and GPU transfer stay on the main thread
- Split fetch_data_from_transfer_queue out of get_data_from_transfer_queue so the CPU-only read can run off-thread and be finalized in order
- Map --vision-dp-when-tp to bridge provider vision_dp_when_cp and patch Qwen3-VL vision CP all-gather for autograd.apply keyword-arg compatibility
- Propagate mtp_use_repeated_layer to the bridge provider so a shared physical MTP layer is reused across depths
- Add VL LoRA/MTP SFT launch scripts (128k, 128/256xGPU)
---
- Handle multi-depth MTP loss: compute per-depth losses and log mtp_{i}_loss instead of assuming a single scalar
- Shift MTP labels/masks one token in the VL/THD get_batch path before CP zig-zag slicing so targets align with valid-token masks
- Stop gathering a portable HF adapter on every save: rank 0 exceeded 1.4 TiB host RAM on the 128-rank Qwen3.5-397B run; native Megatron checkpoint already holds LoRA params
- Rewrite kill_for_ray.sh to a whitelist that only kills Relax/SGLang workers, guarding ray daemons; re-enable clusterwide cleanup in ray-job.sh
---
- Downcast fp32 pixel_values to bf16 before shared-memory IPC + TQ transfer, halving the per-rank deserialize wait losslessly (vision PatchEmbed casts to bf16 anyway)
---
- Add ms-swift last-round loss-mask test
- Cover MTP repeated-layer provider mapping and streaming dataset/TQ iterator changes
(cherry picked from commit 70261625270310d80f3fdcd221f27e985554c996)
A VL base trained text-only builds no vision tower, so the generator Bridge consumes never yields those tensors. Bridge lays shards out from the source index, so the shard that should hold them never completes and is written short -- for gemma-4-26B that meant 657 of the reference's 1013 tensors, a checkpoint whose config.json declares a vision_config the weights do not have, which from_pretrained rejects. Chain the reference's own copies onto that same generator. Every shard then completes, and because the layout comes from the source index the export lands byte-for-byte in the reference's shape rather than gaining a stray extra shard. The tower never trained, so a verbatim copy is the correct weight, not an approximation. Only rank 0 reads it: the other ranks merely drain the generator to stay in step on collectives, so making all eight read the tower would be pure waste. `strict` is deliberately untouched. It only governs what happens when something is missing, so supplementing makes it moot on success while leaving today's behaviour intact if the copy cannot be made -- a bridge with no safetensors source logs a warning and exports as before. Tightening it is a separate change. FP8 is excluded on purpose: the reference tower is BF16 and would not be listed in config.json's modules_to_not_convert, so a loader would decode it as FP8. Verified on 8xH20-3e (job raysubmit_FJ5N2UJ6WbRkVFZK). The export is now two shards named and sized exactly like the base, 1013 keys, 0 tensors in a differently-named shard, total_size and total_parameters identical, and all 356 vision tensors bit-identical to the base (matching sha256 over the group). from_pretrained loads it with no weight warnings and generates. Co-Authored-By: Claude <noreply@anthropic.com> (cherry picked from commit 658e1a1c3eee6c52533ff47d1bafc9bee31c49d6)
# ⭐ Feature ## Add Claude Code reasoning training - Add a pinned reasoning-gym recipe that drives Claude Code through native Anthropic Messages and Buffered SSE - Preinstall Claude Code and Bubblewrap in the Gym image and provide process-group cleanup and cleanup probes - Add data preparation, service launch, deterministic trial verification, and 8-GPU training entrypoints ## Add mixed-environment routing - Select the Gym environment and config from per-row metadata - Add a combined math and workplace recipe with environment-specific startup and cleanup - Forward Chat Completions, Responses, and Messages callbacks in JSON or Buffered SSE ## Keep training recipes self-contained - Move GSM8K and Workplace training parameters into their recipe scripts and remove the obsolete shared launcher - Increase the local R2E Gym concurrency default for multi-session training --- # ✅ Tests ## Update Gateway and recipe coverage - Cover protocol forwarding, callback isolation, streaming behavior, and per-row routing - Keep the mirrored NeMo Gym example tests aligned with the expanded recipe set (cherry picked from commit 1bd8a89bdb5de78d07feea92bcb7e360db033403)
# 🐛 Bug Fix ## Preserve multimodal token identity - Remove the Qwen VL patch that forces the legacy multimodal loader - Keep the standard SGLang processor path so token IDs stay aligned with the actual multimodal model inputs ## Keep routed-expert replay aligned - Preserve routed experts as an ndarray through Agentic artifact restoration - Add an export-boundary shape safeguard for Agentic R3 samples (cherry picked from commit b6a2335232120913c864483d1cc329ba81af0021)
# 🐛 Bug Fix ## Restore docs image builds - Shim the SFT runtime during offline OpenAPI generation - Avoid requiring NumPy in the lightweight documentation image (cherry picked from commit 1dcdfa17d5a3e225d0b4837f5f8ad35dbd90e85e)
- Preserve OpenHands validation and unknown-tool actions in conversation history so tool errors do not create multiple export leaves. - Make R2E-Gym test-directory setup idempotent, exclude test directories from generic moves, and remove only verified self-referential links. - Apply the test-layout patch to cached checkouts at service startup and to new checkouts through the setup hook. - Add installed OpenHands history verification and six test-directory regression cases. - Verify repeated initialization and recovery on an isolated Pyramid SIF: 825 collected tests instead of 33000, with unchanged baseline test outcomes. - Record the recursive test-collection failure and startup patch behavior in the R2E-Gym troubleshooting notes. (cherry picked from commit 24e99f063079d56374f892a5e5fcc30d970014c3)
# ⚡ Performance - Use two TP2/EP1 prefill engines and two TP2/EP1 decode engines on eight GPUs. - Size decode capacity for 256 resident sessions and use consistent session routing, a 600-second HTTP keepalive, and Mooncake transfer settings. - Keep the measured 2048-token turn and 32768-token trajectory budgets, provide service context headroom, and use BF16 gradient reduction. - Default the GRPO recipe to a 20-update run and support an ALFWorld venv. # 🐛 Bug Fix - Distinguish exhausted context from per-turn length limits and export episode stop-reason and turn-count metrics without changing the success reward. - Stop routers launched by this Controller during shutdown so they cannot survive the train entrypoint's os._exit() cleanup boundary. # ✅ Tests - Check Python/Bash syntax, PD YAML, and launch arguments with a mocked Ray CLI. - Exercise Controller shutdown with zero routers, stopped routers, and a cleanup exception; verify later S3 cleanup still executes. - Pass all pre-commit hooks against the exact staged tree in a remote worktree. - GPU integration was not rerun: this commit packages the previously measured recipe and its existing cleanup changes. (cherry picked from commit 2d635d27fccce665530b63a5da2a428664dbbcbe)
- New `--genrm-instances` CLI arg deploys N named judge models behind one
Serve deployment (`{route_key: {model_path, num_gpus, ...}}`), each with
its own GPU budget, engine config, and sampling config
- `GenRMClient.generate(..., route_key=...)` and the `/generate` HTTP
payload select which instance answers a call; omitting `route_key` (or
using the legacy `--genrm-model-path`) falls back to a single
`"__default__"` instance, fully backward compatible
- `/health` and `/metrics` report per-instance detail once multiple
instances are configured
- New `relax/distributed/ray/multi_instance_orchestrator.py`:
`start_multi_instance_managers` splits a shared placement group across N
instances at non-overlapping `bundle_offset`s (prefix-sum based, so
instances may have unequal GPU budgets)
---
- New `relax/distributed/ray/multi_engine_manager.py`: common engine-pool
lifecycle (parallel bring-up, health check, dead-engine
detection/retirement/recovery, offload/onload with idempotency) lifted out
of `GenRMManager`
- `TeacherManager` (mopd) migrated onto the same base class, gaining health
check and dead-engine recovery it never had before (previously a dead
engine failed the whole batched onload/offload instead of being isolated
and rebuilt)
- `opd_utils._start_managed_multi_teacher` now delegates its GPU-budget
carve-up to `start_multi_instance_managers`, corrected from
`idx * gpus_per_teacher` to a prefix-sum `bundle_offset` so the algorithm
generalizes to instances with unequal GPU budgets
- Controller/Actor offload-onload plumbing (`controller.py`, `actor.py`,
`backends/megatron/actor.py`) updated to fan out over a list of GenRM
managers instead of assuming a single handle
---
- `test_multi_engine_manager.py`: fanout failure isolation, offload/onload
idempotency, dead-engine retire/recover, owned-vs-borrowed placement
group cleanup
- `test_arguments_genrm_instances.py`: legacy `--genrm-model-path`
normalization, `--genrm-instances` priority, per-instance required
`num_gpus`, heterogeneous GPU budgets
- `test_opd_multi_teacher_orchestration.py`: bundle_offset prefix-sum
regression guard for the mopd multi-teacher path
- Updated `test_genrm_engine_pick.py` for the new per-route engine cache
---
- `docs/en(zh)/examples/generative-reward-model.md`: new Multi-Instance
GenRM section with the `--genrm-instances` reference, a two-judge
split-bundle example, and an agentic per-module routing example
- `docs/en(zh)/api/genrm.md` + regenerated `docs/public/openapi/genrm.json`:
document the `route_key` request field
- `examples/generate_reward_model/`: new
`run-qwen3-4B-8xgpu-dual-genrm-split.sh` (quality + safety judges on a
split 8-GPU layout) and `reward_dual_genrm_quality_safety.py` (combines
both judges into one training reward)
(cherry picked from commit 87bdf914ec41f3110d2c6faa5fdbe00489c6d71a)
# 🐛 Bug Fix - Collapse expanded image placeholders before reprocessing raw images in the fast loader. - Convert processor StopIteration to ValueError so asynchronous requests finish with an error instead of hanging. - Restrict asyncio noise filtering to the known chain-future AssertionError and preserve unexpected failures. --- # ⚡ Performance - Warm the HF checkpoint page cache in the Deepeyes training recipe. --- # ✅ Tests - Cover asyncio callback errors that must remain visible and the known assertion that should be suppressed. (cherry picked from commit 8d8ae468f35ed157bb2383c8f63dff224394ee88)
SigureMo
force-pushed
the
codex/sync-main-20260915
branch
from
September 15, 2026 08:48
b8916ab to
7ffac71
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate review findings remain before approval.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
docs/en/guide/agentic-rollout.md:422
- This newly updated multi-context documentation points users toward the custom-advantage path, but the contract later in this guide still documents only scalar context values and says turn-level signals must be reduced. The implementation accepts list values and expands them over response-node spans (
relax/agentic/pipeline/reward.py:129-138), so this public guidance omits the supported per-turn credit shape; update the linked contract/examples and warning as part of this documentation change.
examples/nemo_gym_agentic/recipes/reasoning-gym-cc/configs/claude_settings.json:44 - The recipe disables the sandbox and explicitly allows unsandboxed commands, so model-generated Bash can access the training host's filesystem, network, and credentials. The comments explain why this is needed on one kernel, but do not provide isolation; make this an explicit container-only opt-in or fail closed when sandboxing is unavailable.
relax/agentic/pipeline/reward.py:134 - This scalar path now accepts booleans (and any other value accepted by
float()), so a mistakenTrue/Falsecustom advantage is silently converted to1.0/0.0instead of being rejected. That changes the validation contract and can alter training without surfacing a bad callback result; reject booleans before coercion.
- Files reviewed: 126/319 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+898
to
+907
| prefix = f"{_AGENT_PREFIX}-{cache_key}-" | ||
| stale = [] | ||
| for item in ray.util.list_named_actors(all_namespaces=True): | ||
| if item["namespace"] == _AGENT_NAMESPACE and item["name"].startswith(prefix): | ||
| try: | ||
| stale.append(ray.get_actor(item["name"], namespace=_AGENT_NAMESPACE)) | ||
| except ValueError: | ||
| pass | ||
| if stale: | ||
| refs = [actor.finalize.remote("superseded-session") for actor in stale] |
Comment on lines
460
to
462
| # Generic env-var passthrough for overlay packages. Comma-separated list | ||
| # of env-var names the driver wants forwarded to every Ray actor. Each | ||
| # name is copied from the driver's os.environ; missing names are |
SigureMo
force-pushed
the
codex/sync-main-20260915
branch
from
September 15, 2026 08:54
1835a67 to
1574835
Compare
rai-studio-bot
suggested changes
Sep 15, 2026
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
建议修改后再合并：MTP 标签额外移位会训练错误目标，FSDP 多节点权重同步会构造不一致的进程组。具体证据及修复方向见行级讨论；checkpoint 恢复和子进程清理也有可复现的问题。
已复核 15748354。本地完成针对性 CPU 测试和最小复现；缺少所需硬件与运行环境，未运行 GPU/NPU 集成测试及镜像构建。当前 pre-commit 已通过，Python CI 矩阵仍在运行。
Powered by Nyanpasu with gpt-6-astra medium, please check the suggestions carefully.
| # already in the same next-token-aligned frame as the shifted label. | ||
| # Shifting it again would leave it leading the label by one token, so | ||
| # each MTP depth would supervise the position one past what it predicts. | ||
| mtp_labels_padded = F.pad(t_padded[1:], (0, 1), value=pad_token_id) |
There was a problem hiding this comment.
VL + THD + CP 开启 MTP 时，这里先把标签移成 x[t+1]，但 _attach_mtp_forward_kwargs 会原样传给 GPT _postprocess，当前 Megatron patch 又对 mtp_kwargs 标签移位一次，随后普通和 chunked process_mtp_loss 都会逐层再移位。首层 MTP 因此实际学习 x[t+3]，而应为 x[t+2]，会静默训练错误目标。
请统一初始移位的责任：此处分支继续传原始 token，或显式标记预对齐标签并仅对此分支跳过 GPT 初始移位；普通文本路径仍传原始 tokens，不能全局删除 GPT 的移位。建议补充贯穿数据准备、GPT patch 与首层 MTP 的标签对齐回归测试。
| block_ranks: List[int] = [] | ||
| for offset in range(tp): | ||
| gpu = int(base) + offset | ||
| u = str(torch.cuda.get_device_properties(gpu).uuid) |
There was a problem hiding this comment.
多节点训练时，base_gpu_id 是引擎所在节点的本地编号，这里却用当前训练 rank 所在节点的 get_device_properties() 解析每一个引擎。两节点都从 GPU 0 开始时，同一引擎在不同节点会得到不同的 block_ranks，随后 dist.new_group() 的成员集合不一致，权重同步可能挂起；缓存还可能被同编号的远端引擎覆盖，导致跨节点 CUDA IPC。
用实际方法模拟两节点、每节点两卡，节点 0 得到 [[0,1],[0,1]]，节点 1 得到 [[2,3],[2,3]]，两者最终都选择节点 1 的引擎。当前配置校验允许多节点。请结合引擎节点 ID 和 GPU UUID 构造所有 rank 一致的映射，或在实现前明确拒绝多节点 FSDP 配置。
| return None | ||
| from megatron.core import dist_checkpointing | ||
|
|
||
| common_state = dist_checkpointing.load_common_state_dict(str(checkpoint_dir)) |
There was a problem hiding this comment.
这个新增探测对所有 Megatron checkpoint 都执行，包括非 LoRA 的旧 --ckpt-format torch 格式。带 tracker 和 iter_0000007/mp_rank_00/model_optim_rng.pt 的目录会通过 _is_megatron_checkpoint，但这里的 load_common_state_dict() 要求 distributed checkpoint 元数据，固定版本 MCore 会直接抛出 is not a distributed checkpoint，普通加载器的旧格式分支因此永远无法执行。
已用实际探测函数及固定版本的 MCore 验证函数复现。请先判断是否为 distributed checkpoint，仅对此类目录读取 LoRA 元数据，其余继续交给原加载器。
| if not shard.is_file(): | ||
| raise FileNotFoundError(f"HF base checkpoint shard is missing: {shard}") | ||
| stat = shard.stat() | ||
| shard_stats.append((name, stat.st_size, stat.st_mtime_ns)) |
There was a problem hiding this comment.
将本地 st_mtime_ns 纳入基座身份，会使相同权重重新下载后被误判为另一模型。--save-lora-only 恢复时会严格比较此身份，而 S3 的 _download_one() 写入新缓存文件时不保留原 mtime；缓存清理、换节点或重新下载后，原 checkpoint 因此无法恢复。用字节完全相同、仅 mtime 不同的两个模型目录调用此函数，已确认身份不同。
请改用跨下载稳定的模型 revision、内容摘要或经过验证的远端对象身份，并保留对真正不同基座的检查。
| + return | ||
| + try: | ||
| + await asyncio.wait_for(proc.wait(), timeout=5) | ||
| + return |
There was a problem hiding this comment.
等待组长退出不能证明整个进程组已清理。若 Claude Code 收到 SIGTERM 后退出，但它启动的 Bash 子进程忽略 SIGTERM，此处立即返回，后面的 SIGKILL 不会执行；下方清理状态又只检查组长 returncode，会报告 clean=true，使 gateway 释放 trial 槽位并跳过强制清理。
已执行补丁中的原函数复现：组长返回 -15，子进程仍存活，但状态报告已清理。请独立检查进程组的存活成员并升级终止信号；入口处组长已退出的情况也需处理，不能只依据 proc.returncode 删除跟踪。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1574835403
ℹ️ 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".
Comment on lines
+580
to
+582
| transformer_dir = os.path.join(output_dir, subfolder) if base_model_path else output_dir | ||
| os.makedirs(transformer_dir, exist_ok=True) | ||
| _write_sharded_safetensors(renamed, transformer_dir, max_shard_size_bytes) |
There was a problem hiding this comment.
When --output-dir is reused, this writes the new checkpoint without removing weight files from the previous export. Switching between single-file and sharded layouts can therefore leave an old diffusion_pytorch_model.safetensors or index/shard set beside the new snapshot; model loaders may select the stale layout, while _verify_merged scans both and can still report success. Remove the managed weight files and index before writing, or require an empty output directory.
Useful? React with 👍 / 👎.
| requests = [record_to_request(r) for r in records] | ||
| if not requests: | ||
| return {} | ||
| manager = GenerativeRewardManager.local(args) |
There was a problem hiding this comment.
When the evaluator is invoked with --reward-runtime remote --reward-endpoint ..., this unconditionally constructs the local manager, so the endpoint is never contacted and the scorer model is loaded in the evaluator process instead. This makes the advertised remote mode ineffective and can fail on hosts without the reward model dependencies or resources; select GenerativeRewardManager(args) for the remote runtime as the training path does.
Useful? React with 👍 / 👎.
Comment on lines
+560
to
+564
| required = getattr(args, "reward_required_components", None) | ||
| weights = getattr(args, "reward_component_weights", None) | ||
| if required is not None and weights is not None: | ||
| missing = [c for c in required if c not in weights] | ||
| _fail(errors, bool(missing), f"reward component_weights missing required components: {missing}.") |
There was a problem hiding this comment.
For a normal FSDP generative run, the parser defaults reward_component_weights, reward_scorer_path, and reward_endpoint to None, but this validation only compares required components when both mappings already exist. Such a configuration passes preflight, performs an expensive rollout, and then necessarily fails in reward post-processing: local runtimes reject the missing scorer, remote rejects the missing endpoint, or combine_component_advantages rejects the empty weights. Validate nonempty weights and the runtime-specific scorer or endpoint here, with any intended debug-only exemption made explicit.
Useful? React with 👍 / 👎.
SigureMo
force-pushed
the
codex/sync-main-20260915
branch
from
September 15, 2026 09:09
1574835 to
940ffc8
Compare
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
examples/nemo_gym_agentic/recipes/multienv-math-workplace/run-qwen3-4B-8xgpu-nemo-gym-multienv.sh:88
SAVE_DIRis never initialized in this script, so the documented invocation expands this path to/nemo-gym/multienv/Qwen3-4Band may fail due to permissions; it also contradicts the README, which says this recipe does not save checkpoints or useSAVE_DIR. Remove these checkpoint arguments if the README is authoritative, or initialize and validateSAVE_DIRbefore constructingCKPT_ARGS.
examples/nemo_gym_agentic/recipes/reasoning-gym-cc/configs/claude_settings.json:44- With the sandbox disabled and unsandboxed commands allowed, Claude Code's shell actions run inside a container that also has host networking,
SYS_ADMIN, and an unconfined seccomp profile. A prompt or generated task that executes an unsafe command can therefore access more than the intended benchmark workspace and potentially reach host services. Keep the sandbox as a required prerequisite, or make this reduced-isolation mode an explicit opt-in with a least-privilege container.
- Files reviewed: 126/319 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
654
to
658
| def get_grpo_returns( | ||
| rewards: torch.Tensor, | ||
| rewards: torch.Tensor | list[float] | list[torch.Tensor], | ||
| kl: list[torch.Tensor], | ||
| ): | ||
| returns = [] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 940ffc869f
ℹ️ 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".
Comment on lines
+249
to
+252
| traj = _as_tensor(response["trajectory_latents"]) # [T+1, ...] | ||
| timesteps = _as_tensor(response["timesteps"]).float() | ||
| sigmas = timesteps / 1000.0 # flow-matching sigma space | ||
| sde_indices = _as_long(response["sde_indices"]) |
There was a problem hiding this comment.
When sde_indices or sde_pool includes the allowed final denoising index num_inference_steps - 1, the SGLang response contract supplies T timesteps for T+1 trajectory latents, so this stores no terminal sigma. _replay_logp later accesses sigmas[step_index + 1], causing the training step to fail with an out-of-bounds access. Append the terminal zero sigma or reject the final transition during validation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
补充固定版本的源码证据：SGLang v0.5.17 的 rollout_denoising_mixin.py 在循环结束后同时追加最终 latent 和 final_timestep；同文件 164–165 行将两者成对存入列表，188 行导出完整 timesteps。Relax 的 diffusion_engine.py:596 原样传递该字段。
因此完整轨迹的正常引擎输出包含 T+1 个 timesteps，不能仅依据本地 [T] 注释认定最后一步必然越界。建议区分注释/畸形响应的校验问题与真实引擎输出；如有实际只返回 T 项的响应，需要进一步核对其版本和轨迹筛选参数。
Comment on lines
+265
to
+266
| stat = shard.stat() | ||
| shard_stats.append((name, stat.st_size, stat.st_mtime_ns)) |
There was a problem hiding this comment.
When a lightweight LoRA checkpoint is resumed after the identical HF base has been freshly downloaded or copied, its local st_mtime_ns commonly changes even though all model bytes are unchanged. A new process recomputes this digest, _validate_lora_checkpoint_metadata sees a base_hf_checkpoint mismatch, and refuses the resume; this is especially reachable through the S3 loader, which recreates shard files without restoring remote timestamps. Use stable source metadata or content-derived identity rather than filesystem mtimes.
Useful? React with 👍 / 👎.
Comment on lines
+983
to
+987
| "choices": [ | ||
| { | ||
| "index": 0, | ||
| "message": copy.deepcopy(result["message"]), | ||
| "logprobs": result["logprobs"], |
There was a problem hiding this comment.
For Chat Completions requests containing n other than 1, _normalized_chat_request now ignores the field even though the backend performs only one generation, and this renderer always returns one choice. Clients requesting n=2 therefore receive a successful but incomplete response instead of the prior validation error; restore the n == 1 check or implement multiple generations.
Useful? React with 👍 / 👎.
SigureMo
force-pushed
the
codex/sync-main-20260915
branch
from
September 15, 2026 09:18
940ffc8 to
7923142
Compare
# 🐛 Bug Fix - Guard the SIGTERM regression's optional SGLang import and the MessagePack regression's optional msgspec import. - Resolve rollout lifecycle enums inside the test body so the existing missing-dependency skip marker can take effect. - Reuse the existing isolated arguments-module fixture for SFT validation tests, preserving the real functions and assertions. - Isolate media loader modules in the SFT error-wrapping test so its simulated failure does not require optional video codecs. - Keep the real missing-image loader in SFT streaming tests while isolating unused audio/video modules. - Expose configured attributes on the botocore Config test double for retry and timeout assertions. - Require the real TransferQueue sampler for its token-budget regression, with an explicit skip for the CPU CI import stub. --- # ✅ Tests - Run SFT and adjacent MTP argument tests without SGLang: 27 passed. - Run native diffusion engine tests without msgspec: 27 passed and one optional codec test skipped; verify that codec test passes with msgspec installed. - Reproduce and fix the rollout collection NameError; targeted checks finish with 2 passed and 39 expected dependency skips. - Run the full SFT engine test suite without torchvision: 130 passed, including error wrapping and all three missing-media batch modes. - Run S3 loader tests with an unused router import stub: 76 passed; one Linux /dev/shm expectation is unavailable on macOS. - Run changed utils tests with the public CI stub: 140 passed and 4 dependency skips; verify the sampler regression passes with the project's pinned TransferQueue implementation. - Run full pre-commit checks, including Gitleaks, plus duplicate-definition and Ruff F811 checks.
SigureMo
force-pushed
the
codex/sync-main-20260915
branch
from
September 15, 2026 09:23
7923142 to
aba6247
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aba62471a5
ℹ️ 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".
Comment on lines
+589
to
+590
| seed = f"{args.model_path}:{getattr(args, 'model_revision', None)}" | ||
| return hashlib.sha256(seed.encode("utf-8")).hexdigest() |
There was a problem hiding this comment.
In the newly added FSDP path, this hashes only the model path and optional revision string, not the base model contents. If a local model directory is overwritten or remounted at the same path between runs, _assert_resumable accepts the adapter-only checkpoint even though the newly loaded base weights differ, silently resuming a policy that was never trained; use an immutable source revision or a content-derived manifest.
Useful? React with 👍 / 👎.
| logger.warning(f"No RNG state for rank {_rank()} at {path}; resume RNG not restored.") | ||
| return | ||
| try: | ||
| state = torch.load(path, map_location="cpu") |
There was a problem hiding this comment.
In the shipped PyTorch 2.9 environment, torch.load defaults to weights_only=True, but _save_rng stores NumPy's general pickled RNG tuple in this file. Resuming a normal checkpoint therefore rejects the NumPy globals here, the broad exception handler skips all subsequent restoration, and training silently continues with different Torch, CUDA, Python, and NumPy RNG streams; explicitly load this trusted per-rank file with weights_only=False or serialize only safe primitives and tensors.
Useful? React with 👍 / 👎.
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
Synchronize 33 development commits into main, adding asynchronous SFT prepacking, native diffusion RL, Gemma 4 recipes, extended Agentic protocols, NeMo Gym recipes, and multi-instance generative reward models. The series also fixes elastic scale-in, checkpoint export, and multimodal rollout stalls.
Why
Publish the completed development changes while retaining the existing GitHub main history, including #309 and the public CI compatibility fixes.
How
bff0e51a3ea88ad6a61968d4a26a732d49ff79da.Testing
pre-commit run --all-filespasses, including Gitleaks./dev/shmfilesystem test passes in GitHub CI.58054a33834aadbcf76aacd6b1e32e25c030f2c9, without initializing its distributed clients.aba62471a51d4a3292e885f699e1c094c9db6374: Pre-commit Checks and Python 3.10, 3.11, and 3.12. CI run. Python 3.11 reports 1833 passed and 426 dependency/hardware skips.GPU/NPU integration tests and image builds were not run locally because the required hardware and runtime environments are unavailable. Patch checks validate syntax only, not application inside the target images.
Type of Change