Conversation
# 🐛 Bug Fix ## Reject --ref-load under preference objectives - Standard DPO snapshots the frozen reference from the initialized policy and rebuilds it from --hf-checkpoint on resume; --ref-load was silently ignored on that path while also rerouting the bridge mode policy-init fallback, so validate_preference_args now fails fast when it is set (RFC redai-studio#208: --ref-load is not the v1 DPO reference source) - Drop the misleading --ref-load from the DPO recipe; behavior is unchanged because the bridge fallback already resolves to the same HF checkpoint --- # ♻️ Refactor ## Deduplicate the preference-mode predicate - Route data.py get_data_iterator and loss.py loss_function through is_preference_mode() instead of inline loss_type/sft_objective checks, keeping objective dispatch on a single source of truth --- # ✅ Tests ## Cover --ref-load rejection - Parametrize standard and reference-free DPO rejection cases in test_preference_runtime.py
# ♻️ Refactor ## Remove unreachable DPO loss fallback - Drop the dead `0 * logits.sum()` branch in dpo_loss_function: build_preference_pair_indices already raises on an empty micro-batch ## Attach stable reason codes at preference raise sites - Introduce _PreferenceRowError carrying an explicit reason_code from every normalization/split/truncation rejection; message matching in _classify_preference_error remains only as a fallback for errors raised outside the module, so reworded messages can no longer silently degrade rejections to "schema" --- # ⚡ Performance ## Merge DP pair-count collectives - Reduce _get_preference_data_iterator from three all_reduces to two by riding MIN/MAX on a single MAX all_reduce over [count, -count] ## Skip redundant same-tag model restores - _switch_model now returns early when the target tag is already active, eliminating the duplicate full-weight CPU->GPU restore after the ref-forward finally block; paths that deliberately dirty weights already clear the tag first (covered by the injected-loader-failure reference integrity test) --- # 📝 Documentation ## Note probe bitwise-determinism prerequisite - en/zh DPO guides now state that the reference probe digest assumes an identical GPU/driver/image/kernel stack on resume, and that a probe mismatch signals environment drift rather than data corruption ## Justify the checkpoint-save barrier - Comment that the post-sidecar barrier is functional (peers must not pass before rank 0 persists the identity file), and use an explicit gloo group for the rank-0 check per distributed code rules
# 🐛 Bug Fix ## Preserve standard DPO likelihood semantics - Reject non-unit or non-finite rollout temperatures before actor construction - Replace CUDA-to-Python condition checks with device-side asynchronous assertions - Consolidate finite-value validation on the final DPO logits ## Validate preference data preparation inputs - Validate message objects, roles, and string content on both preference branches - Classify non-object source rows as schema rejections --- # ✅ Tests ## Cover validation regressions - Test finite and non-unit temperature failures - Verify CUDA conditions avoid Python boolean conversion - Cover malformed chosen and rejected message schemas
# 🐛 Bug Fix ## Pin standard DPO frozen reference provenance - Resolve the reference from the declared repository and revision in the configured HF checkpoint directory - Require Hugging Face local metadata to verify the pinned snapshot without downloading during actor startup - Rebuild the frozen reference from that verified directory for both fresh starts and resumes - Fail clearly when the configured checkpoint is unavailable, unverified, or resolves elsewhere --- # 📝 Documentation ## Document pinned local model preparation - Show the fixed-revision hf download command used by the public DPO recipe --- # ✅ Tests ## Cover local reference resolution - Verify repository, revision, local directory, and local-only cache resolution - Verify missing metadata and mismatched resolved directories fail before model loading
# 🐛 Bug Fix ## Require pinned local snapshot metadata - Require an immutable 40-character commit SHA for standard DPO references - Verify Hugging Face local-dir tree metadata before accepting the configured checkpoint - Reject copied or incomplete local snapshots that lack pinned provenance metadata --- # ✅ Tests ## Cover metadata validation - Exercise the pinned tree lookup and missing-metadata rejection paths - Document the full commit-SHA requirement for the DPO recipe
# 🐛 Bug Fix ## Validate pinned Hugging Face local snapshots - Parse the per-file metadata format used by pinned huggingface_hub 1.7.2 - Require every reference file to match the configured commit revision - Verify regular Git files with Git blob SHA-1 and LFS files with SHA-256 - Stream large-file hashing to keep reference validation memory bounded --- # ✅ Tests ## Cover reference provenance and integrity - Exercise Git and LFS ETag validation - Reject missing or mismatched local metadata - Reject replaced weights even when their original mtime is restored
# 🐛 Bug Fix ## Enforce local reference completeness - Require a supported single-file weight or standard Transformers weight index - Validate non-empty index weight maps, safe shard paths, and format-specific suffixes - Reject indexes that reference missing model shards --- # ✅ Tests ## Cover missing reference weights - Accept a complete multi-shard safetensors index - Reject snapshots without a supported weight entry point - Reject a deleted shard even when its local metadata is also removed
# 🐛 Bug Fix ## Move preference scheduling to the CPU control plane - Replace CUDA count reductions and scalar readbacks with one DP-Gloo control gather - Preserve pair-row, global-denominator, and micro-batch-count agreement across ranks - Require Gloo process groups for preference objectives and enable them in the DPO recipe --- # ⚡ Performance ## Remove hot-path GPU synchronization - Eliminate all item calls and control tensors from the preference iterator - Avoid GPU-to-CPU synchronization for train, reference, and evaluation iterators --- # ✅ Tests ## Guard preference iterator contracts - Cover unequal DP pair-row rejection through the Gloo control path - Assert the iterator contains no item or all-reduce scalar readback path
# ♻️ Refactor ## Keep one reference identity path - Remove the unused helper that reads the already-backed-up ref weights - Preserve rebuild validation and backup ordering without duplicate digest work --- # ✅ Tests - Pass the full pre-commit suite - Compile the Megatron actor module
|
Nit: Please avoid committing data JSON files directly to this PR. Consider adding a download link instead. |
|
Thanks for the suggestion. I removed the checked-in UltraFeedback manifest from this PR, updated the DPO documentation to use the reproducibility evidence bundle, and updated the evidence bundle linked in the PR description to include the manifest. |
# 🐛 Bug Fix - Merge current upstream main while preserving the original DPO history. - Reuse the shared SFT dataset factory and pair-aware queue payload path. - Preserve async SFT producers, sequence classification and MTP behavior; reject those unsupported combinations for preference objectives. - Add deterministic preference split/resume interfaces and propagate the actual global pair count to scheduler and metric normalization. # ✅ Tests - Focused CPU suite: 234 passed, 7 skipped (Megatron/CUDA unavailable). - Exercise the Docker-pinned TransferQueue pair sampler without Megatron. - pre-commit run --all-files --show-diff-on-failure passed. - GPU training and distributed save/resume were not rerun. # 📝 Documentation - Document the v1 compatibility boundaries in English and Chinese. Co-authored-by: Codex <noreply@openai.com>
Nyanpasu 审查看板审查状态: 🚧 需要修改 审查版本: 已复查 aab0c72:仅删除 iterator 源码字符串断言及 inspect 导入,保留 pair batching、DP 分配和分母校验行为测试,未发现新增问题。生产代码未变,F1、F2 仍未解决,F3 已解决,维持既有需要修改结论。本轮未重复运行测试;本地缺少 Megatron/CUDA,未执行 GPU/多节点验证。当前 CI 仍在运行。
Powered by Nyanpasu with gpt-6-astra medium, please check the suggestions carefully.
|
rai-studio-bot
left a comment
There was a problem hiding this comment.
需要修改:合法数据在跨 epoch 组 batch 时可能终止训练,详见行级意见;另请处理多轮 prompt 的 completion mask 和 CPU CI 的 sampler 测试问题。
已审查当前提交的数据、pair batching/loss、reference 恢复与 checkpoint 链路。本地聚焦测试共 74 passed、8 skipped;扩大 SFT 测试范围时因缺少 tensordict 无法收集,未执行本地 GPU/多节点训练。当前 GPU 检查通过,Python 3.10–3.12 检查失败,具体原因见测试行级意见。
| encoded_pair_ids = [ | ||
| int.from_bytes(hashlib.sha256(pair.pair_id.encode()).digest()[:8], "big") >> 1 for pair in pairs | ||
| ] | ||
| if len(set(encoded_pair_ids)) != len(encoded_pair_ids): | ||
| raise ValueError("preference pair ID hash collision within batch") |
There was a problem hiding this comment.
请区分数据源 pair ID 和本次采样的 pair occurrence ID。现有 IndexManager.get_next_indices() 会跨 epoch 补齐 batch,因此合法数据也可能在同一个 batch 中再次采到同一行;这里却将其判为 hash collision 并终止生产。当前提交用 10 个唯一 pair、seed=42、batch size=4,即可在第 3 个 batch 复现 [pair-2, pair-3, pair-7, pair-2] 后抛错。这会使不能整除 batch size 的普通数据集在跨 epoch 时中断训练。请为每次采样分配独立的训练 pair identity(同时保留源 ID),并补充跨 epoch 的打包/训练回归;仅删除这里的检查仍会触发下游重复 chosen/rejected branch 检查。
| chosen_tokens, chosen_mask = render_with_loss_mask( | ||
| chosen_sample, | ||
| tokenizer=self.tokenizer, | ||
| apply_chat_template_kwargs=self.apply_chat_template_kwargs, | ||
| ) |
There was a problem hiding this comment.
这里两个 render 调用需要显式只监督最终 completion。对于包含 {% generation %} 的模板,render_with_loss_mask() 默认的 assistant-mask 路径不会读取 CanonicalMessage.learn=False,而会标记所有历史 assistant 回答。共享 prompt 为 user → assistant → user 的合法 pair 因此在 _split_branch() 抛出 completion mask must be one contiguous suffix;使用本文件测试中的 tokenizer 即可复现。请在 chosen/rejected 渲染时排除历史回答(例如适用时传 last_turn_only=True),并测试带历史 assistant 的共同前缀。
| transfer_queue = pytest.importorskip("transfer_queue") | ||
| sampler_type = transfer_queue.SeqlenBalancedSampler | ||
| source_path = Path(inspect.getsourcefile(sampler_type) or "") | ||
| normalized_source = source_path.read_bytes().replace(b"\r\n", b"\n") | ||
| assert hashlib.sha256(normalized_source).hexdigest() == ( | ||
| "dc6c2db50df4b9448d4845ccacef67a400517db892b5cd55de2e22f6baf6888b" | ||
| ) |
There was a problem hiding this comment.
pytest.importorskip("transfer_queue") 无法识别 CPU CI 中的占位模块:现有 .github/workflows/ci.yml 会生成含 SeqlenBalancedSampler 的 stub,导入成功后这里必然对 stub 源码做真实 sampler 的 hash 校验。当前 Python 3.12 检查 已在这里失败(实际 hash 为 30cf6bec…),3.10/3.11 检查也失败。请在运行该兼容性测试的任务中安装 Docker 固定版本的真实 TransferQueue,或明确跳过已识别的 CI stub 并保留真实依赖环境中的兼容性门禁。
There was a problem hiding this comment.
指的是这个 PR 新增的 test_pinned_seqlen_sampler_consumes_pair_costs_and_keeps_equal_dp_groups:先比较 sampler 整个源码文件的 SHA-256,再验证 pair 分组行为。CPU CI 导入的是占位模块,因此之前还没执行行为断言就失败了。
这里比较整个源码 hash 确实很脆弱;依赖版本应由安装配置固定,测试重点应是 pair 原子性、分组数量和 cost 分配。我上一条指出的是实际 CI 失败,并不是建议保留这种源码 hash 测试。
There was a problem hiding this comment.
没让你回我……笨
你要是以后敢写出来这种测试就不给你 token 了
There was a problem hiding this comment.
4bc9ddad 已删除该测试,原先对 CI stub 做源码 hash 校验的失败原因已消除,本项按已解决记录。删除同时移除了真实 sampler 的行为断言,因此不将此次变更视为 sampler 兼容性验证通过;新一轮 CI 仍在运行。
# ✅ Tests - Mark the CPU CI TransferQueue import stub explicitly and skip only the real-sampler integration check when that stub is present. Keep the pinned source digest and pair-assignment assertions for actual installations. - Install the missing-media warning capture before shuffle starts prefetch workers, avoiding a race in the logging assertion. - Reproduce the exact CI stub hash failure before the fix. - Validate 67 tests against real TransferQueue and 66 passed / 1 expected skip against the workflow-generated CPU stub. - pre-commit run --all-files --show-diff-on-failure passes. Co-authored-by: Codex <noreply@openai.com>
# 🔩 Chore - Revert 21d2c22 as requested before removing the pinned sampler test. - Restore the original CI stub and test files. - pre-commit run --all-files --show-diff-on-failure passes. Co-authored-by: Codex <noreply@openai.com>
# ✅ Tests - Delete the fixed-source-hash TransferQueue sampler test as requested. - Remove its now-unused imports without adding a CI-specific skip path. - Fix the independent prefetch logging race by installing the capture hook before starting the worker thread. - Focused CPU regression: 66 passed with the CI TransferQueue stub. - pre-commit run --all-files --show-diff-on-failure passes. Co-authored-by: Codex <noreply@openai.com>
| def test_preference_iterator_has_no_device_scalar_readback(): | ||
| source = inspect.getsource(data_module._get_preference_data_iterator) | ||
| assert ".item(" not in source | ||
| assert "all_reduce(" not in source |
# ✅ Tests - Remove the source-string test that forbids item and all_reduce calls. - Remove the unused inspect import. - Keep the existing pair batching and denominator behavior tests. - All pre-commit hooks pass. Co-authored-by: Codex <noreply@openai.com>
What
本 PR 在现有离线 SFT 链路中加入 pair-aware Direct Preference Optimization(DPO)能力。
SeqlenBalancedSampler、DP 分配、动态 batching 和 micro-batch packing 全链路保持 preference pair 原子性。本 PR 不增加 Reward Model scalar head。Reward Modeling 和最终的共享偏好评测器位于后续 PR #258。
Why
Relax 当前主要覆盖在线 RL 工作流。Task 31 要求在不复制训练框架的前提下,复用 SFT trainer 建立标准的离线偏好优化链路。
实现遵循 RFC #208 和Task 31 要求。
Relates to #208.
Follow-up: #258.
How
本次更新合并主线
0651812093e3cd730302709b3db35ce0bb199e4b,保留作者历史;主线集成提交为14ccc2a。_create_sft_train_dataset/_prepare_sft_tq_payload接入偏好数据,保留主线 SFT producer/shard/prepack 路径。--sft-async-prepack、task_type=seq_cls和 MTP-only 组合;CPU 数据预取仍可用。每个 TransferQueue row 表示一个完整 preference pair,并通过
custom_meta携带 chosen/rejected 的合并 sequence cost。固定版本的SeqlenBalancedSampler因而可以在不拆分 pair 的前提下形成等大小 DP 分组。动态 packer 先分配完整 pair,仅在构造模型输入时展平 chosen/rejected branch。DPO loss 消费显式的 pair/branch identity,使用 completion-only mask,按全局 pair 数归一化,同时支持标准 reference 和 reference-free 公式。空 mask、非有限值、prompt prefix 不一致、截断后两侧相同、超大 pair,以及不一致的 global pair count 均 fail closed。
标准 DPO fresh 训练将初始 actor 快照为 frozen reference;resume 时则从固定 HF repository/revision 执行 model-only reference 重建。Canonical parameter、源文件 manifest、optimizer master/state 和固定 log-prob probe digest 可防止 reference 切换静默修改 actor 或 optimizer。验证后的 identity 会写入 checkpoint 旁的
relax_dpo_reference.json。Testing
删除源码字符串断言(
aab0c72)test_preference_iterator_has_no_device_scalar_readback及其inspect导入,保留 pair batching、DP 分配和分母校验的行为测试。CI 测试修正(
4bc9dda)test_pinned_seqlen_sampler_consumes_pair_costs_and_keeps_equal_dp_groups及固定源码 SHA-256 检查。主线集成提交
14ccc2a的本地验证pre-commit run --all-files --show-diff-on-failure:通过。pytest tests/,未重跑 GPU 训练、DP=2 或 distributed checkpoint save/resume;最新 CI 以本 PR checks 为准。作者提供的历史 GPU 验证
以下日志、曲线和证据包对应旧集成提交,保留用于追溯,不代表本次合并后的 GPU 复测结果。主线已更新 shuffle 行为,重新验证时应比较新版本的连续训练与恢复训练。
在最终堆叠集成 head
b57a1818f2f80d28d0ad650b314eb4d1c82df3aa的标准容器中执行:固定 TransferQueue 兼容性测试覆盖 pair-row 原子性、等大小 DP 分组、row-aligned
total_lengthscost 消费,以及无丢失、无重复的完整分配。数据准备脚本的测试覆盖 manifest:4,096 个 train pair、512 个互不重叠的 eval pair、确定性选择、拒绝原因计数和派生文件 hash。GPU 验证固定使用相同的模型/data revision 和 global batch size 32 pairs:
SeqlenBalancedSampler with dp_size=2,两张 A100 均有实际活动。Type of Change
Screenshots / Logs
本 PR 附带 DPO DP=1/DP=2 训练曲线和脱敏后的可复现性证据包。证据包包含:
公开附件不包含 checkpoint、optimizer state、模型权重、完整数据集、凭据或机器专属标识。
测量环境使用
ghcr.io/redai-infra/relaxrl:dev-20260723-8cc1e8fd;证据采集时从 GHCR 解析的 manifest digest 为sha256:8dc39af377a570e6cd7ec88c8b7fcd44c1eb820111e9d2069f1c7c3024b2ea23。Transformer Engine 无法导入顶层flash_attn_interface,并明确回退到 native attention。因此 DP 对比描述的是相同 native-attention 环境,不是 FlashAttention 性能结果。task31-pr1-dpo-evidence-public-v2.tar.gz