Repository navigation
Conversation
# ⭐ Feature - Scan every response window with a 10,000-character window and 5,000-character stride, retaining the strict compression threshold and boolean metric API. - Report suspicious intervals, per-window and maximum compression ratios, and union coverage without changing responses, generation, or rewards. - Add streaming JSONL and CPU Torch dump diagnostics with atomic JSON reports. # ♻️ Refactor - Reuse the existing rollout metric aggregation in a CPU-importable module and preserve its distributed entry point; defer Ray-only utility imports. # ✅ Tests - Add fixed middle-repeat/normal-suffix regression and real dump/CLI roundtrips. - Validate the production aggregation and boundary, Unicode, strict-threshold, overlap-union, and full-coverage cases. - Record 24 reproducible CPU benchmark cases through one million characters. # 📝 Documentation - Add English and Chinese usage guides and benchmark methodology/results. Relates to redai-studio#321 (contributor task No.6).
|
辛苦 merge 下最新 main,新增了两条流水线 |
|
# 🔩 Chore - Merge upstream main at 0651812 to incorporate the GPU integration workflow and local device-action references. - Preserve the full-response repetition diagnostics changes. # ✅ Tests - Run the full pre-commit suite with Python 3.12 for the legacy docformatter hook. - Run repetition detection, dump reader, CLI, and production metrics regression tests.
Nyanpasu 审查看板审查状态: ✅ 已通过 审查版本: 已针对修复提交 `f2babd93` 完成复审:上一轮的 1 项阻塞 + 2 项非阻塞全部修复并经证据核验,本轮未发现新的阻塞或非阻塞问题。新 head 上 7 项检查全绿,其中此前失败的 GPU Unit Tests 由 `6 failed, 2779 passed` 转为 `2794 passed, 18 skipped, 0 failed`;三位发现分别通过 CI、本地复现与新增回归测试确认关闭。
Powered by Nyanpasu with deepseek-v4.1-flash-ali medium, please check the suggestions carefully.
|
rai-studio-bot
left a comment
There was a problem hiding this comment.
审查结论:请求修改
已针对 961d092a 完成审查。相比上一版 cda2ff1d,本 PR 仅合入了上游 CI 变更,检测器、离线 CLI、dump 读取与指标模块代码逐字节一致,结论不受影响。
阻塞(1 项)
relax/utils/misc.py把is_port_available的导入下沉到函数体内,移除了relax.utils.misc的模块属性,会让既有测试tests/distributed/ray/test_weight_sync_port_hardening.py的 6 处patch.object(misc, "is_port_available", ...)直接报AttributeError;在 CI 训练镜像下该文件不会被 skipif 跳过,pytest tests/会失败。修复方式与验证命令见该线程。
非阻塞(2 项)
repetition_frac依赖的has_repetition现在没有窗口上限,且非重复样本无法提前退出,训练侧每步的检测成本随响应长度线性上升(已给出实测数据)。- 离线报告由
NamedTemporaryFile+os.replace发布,权限固定为0600且会静默改写既有报告的权限。
以上三项的证据、复现与建议都在对应行级线程中,此处不再重复。
已核实无问题
窗口覆盖完整(含未对齐末段、无重复窗口)、has_repetition 与 analyze_repetition 判定一致、阈值严格大于、covered_chars 并集计算正确、迁移后的聚合函数与旧实现逐行等价且指标名/分母保持、weights_only 默认安全且无静默回退到 pickle、CLI 在无 torch/numpy/ray 的解释器下可端到端跑通并保持原子发布、benchmarks/repetition/windows-python313.json 的 24 个用例判定可用本仓库检测器复现、中英文文档一致且链接可达。
P1 修复后(并确认 pytest tests/distributed/ray/test_weight_sync_port_hardening.py 通过)即可再次请求审查;P2/P3 可在本 PR 或后续跟进。
|
|
||
|
|
||
| def get_free_port(start_port=10000, consecutive=1, max_port=None): | ||
| from relax.utils.http_utils import is_port_available |
There was a problem hiding this comment.
把 is_port_available 的导入下沉到函数体内,会让 relax.utils.misc 不再具备该模块属性,直接打断既有测试 tests/distributed/ray/test_weight_sync_port_hardening.py(该文件第 125、137、150、155、166、183 行均为 patch.object(misc, "is_port_available", ...))。mock.patch.object 默认 create=False,目标属性不存在时在进入上下文时即抛错:
AttributeError: <module 'relax.utils.misc'> does not have the attribute 'is_port_available'
(已用等价的最小模块复现验证。)
该文件由 pytestmark = pytest.mark.skipif(not HAS_DEPS, ...) 保护,而 HAS_DEPS 取决于 relax.distributed.ray.rollout 能否导入——在 CI 训练镜像(ghcr.io/redai-studio/relaxrl:latest 配合 requirements.txt 中的 ray/sglang)下为真,因此不会被跳过。.github/workflows/unittest.yml:86 执行 pytest tests/,这会让必需检查变红。本 PR 描述写的是「完整测试套件未跑(缺少训练依赖)」,正是这类只在有 Ray/SGLang 环境里才暴露的问题。
还有两点需要注意:
- 只把模块级导入加回去、同时保留函数体内这一行,测试仍然会失败。函数体内的
from ... import每次调用都会重新读取relax.utils.http_utils,而patch.object(misc, ...)只修改misc的属性,get_free_port依旧调用真实的端口探测,于是assert port == 21000(第 139 行)之类的断言照样不成立。要让 patch 生效,函数体内必须改回读取全局名。 - 这处下沉的收益很有限:
relax/utils/http_utils.py只依赖httpx与relax.utils.env,不引入 ray/torch;而relax/utils/misc.py仍在模块级import torch,导入开销由 torch 主导。真正需要延迟的是同一改动里的import ray(那处保留即可),is_port_available这处可以还原。
建议:恢复 relax/utils/misc.py 模块级的 from relax.utils.http_utils import is_port_available,删除 get_free_port 内的同名导入,保留 get_current_node_ip 内的 import ray;改完用 pytest tests/distributed/ray/test_weight_sync_port_hardening.py(需 ray/sglang 环境)确认恢复。
There was a problem hiding this comment.
CI 已在本 head 上确认该问题:GPU Unit Tests(961d092a)在训练镜像内执行 pytest tests/,结果为
6 failed, 2779 passed, 18 skipped, 99 warnings in 383.96s (0:06:23)
失败的 6 条正是 tests/distributed/ray/test_weight_sync_port_hardening.py::TestBoundedPortWindow 的全部用例,报错原文(对应上文列出的 6 个 patch.object 调用点):
E AttributeError: <module 'relax.utils.misc' from '/workspace/Relax/relax/utils/misc.py'> does not have the attribute 'is_port_available'
- 运行:https://github.com/redai-studio/Relax/actions/runs/35319444050 (job
105526169816,即必需的 "Unit Tests (H20, 4 GPUs)" 检查,退出码 1) - 该 run 的
head_sha就是961d092a;同一 head 上 Pre-commit Checks、Tests (Python 3.10/3.11/3.12) 与两个 GPU 集成用例均通过,所以这是当前唯一的红灯。
补充两点实测结论:
- 本 PR 自带的
tests/utils/test_repetition*.py在同一个训练镜像里 84 条用例全部 PASSED,检测器、dump 读取与 CLI 部分没有问题,红灯完全来自这一处导入下沉。 - 因此上文建议的修法不变:还原
relax/utils/misc.py模块级的from relax.utils.http_utils import is_port_available、删除get_free_port内的同名导入,保留get_current_node_ip内的import ray。
改完重跑该 workflow(或本地 pytest tests/distributed/ray/test_weight_sync_port_hardening.py)确认转绿后,即可再次请求审查。
There was a problem hiding this comment.
已在 f2babd93 验证修复,F1 关闭。
relax/utils/misc.py:13恢复模块级from relax.utils.http_utils import is_port_available,get_free_port改回读取全局名(import ray的延迟保留),与建议一致;http_utils只依赖env/logging_utils,不存在循环导入。- 此前失败的必需检查 GPU Unit Tests 在新 head 上通过:2794 passed, 18 skipped, 0 failed(run 35323216487),
tests/distributed/ray/test_weight_sync_port_hardening.py全部用例转绿。 - 新增的
tests/utils/test_misc_ports.py用真实 socket 覆盖了模块属性导出与端口占用两条路径,同样通过。
| log_dict |= _compute_prefix_cache_metrics(args, samples) | ||
| log_dict |= _compute_reward_cat_metrics(args, reward_category_samples) | ||
| log_dict |= compute_mopd_metrics(args, rewarded_samples) | ||
| log_dict["repetition_frac"] = np.mean([int(has_repetition(s.response)) for s in samples]).item() |
There was a problem hiding this comment.
这一行在每个 rollout step 对整批样本各调用一次 has_repetition(调用链:_log_rollout_data → compute_metrics_from_samples,relax/distributed/ray/rollout.py:4774,4813)。新实现没有窗口上限,窗口数随响应长度线性增长(relax/utils/repetition.py:107-124,10k 窗口 / 5k 步长),且每个窗口都做一次 level 9 压缩;旧实现对每个样本固定压缩一次末尾 10k 窗口。关键在于:非重复样本无法提前退出,所以训练中最常见的路径从常数成本变成了 O(响应长度)。
用本仓库真实检测器实测(非重复文本,纯 CPU,200 次均值):
| 响应长度 | 旧实现 | 新实现 | 倍数 |
|---|---|---|---|
| 10,000 字符 | 不检测 | 0.19 ms | 新增覆盖 |
| 60,000 字符 | 0.19 ms | 2.48 ms | 12.8× |
| 120,000 字符 | 0.18 ms | 5.04 ms | 28.4× |
绝对量级不大(PR 自带的 benchmarks/repetition/ 也显示 1M 字符约 18 ms),通常远小于一个训练 step,所以我把它标为 P2 而非阻塞项。但 benchmarks/ 只刻画了独立检测器,没有覆盖训练侧「每步 × 全批」的聚合开销;对 agentic rollout 这类响应可达十万字符量级的场景,建议在 PR 描述里明确这个权衡。
如果希望保留布尔路径的固定成本,可以给 has_repetition 单独设窗口数上限、或复用已扫描窗口的缓存;如果认为当前成本可以接受,也请把「指标路径不做窗口上限」这一点写进 PR 描述或 docs/*/guide/repetition-diagnostics.md,避免后续被当成回归。
There was a problem hiding this comment.
已按建议在文档中显式说明该权衡,F2 关闭。
docs/en/guide/repetition-diagnostics.md 与 docs/zh/guide/repetition-diagnostics.md 的「快速开始」新增段落,说明训练指标同样使用无窗口上限的全文布尔扫描、非重复样本需压缩每个窗口、CPU 成本随批次内响应字符总数线性增长,并明确检测器基准不覆盖整批聚合与训练吞吐。
实现按 PR 原设计保留完整窗口覆盖,这一点现在对使用者是显式可见的,不再需要后续以「无上限」为由重新讨论。
| with tempfile.NamedTemporaryFile( | ||
| mode="w", encoding="utf-8", dir=output.parent, prefix=f".{output.name}.", suffix=".tmp", delete=False | ||
| ) as stream: | ||
| temporary = Path(stream.name) |
There was a problem hiding this comment.
tempfile.NamedTemporaryFile 以固定的 0600 创建临时文件(不受 umask 影响),第 143 行的 os.replace 会把这个权限原样发布给最终报告。于是:
- 在 umask 022 下生成的报告是
0600,与仓库其它 writer 的行为不一致(例如relax/utils/training/train_dump_utils.py:233的open(path, "w")遵循 umask); - 更值得注意的是,若已有报告是
0644,重跑一次会被静默改成0600,作者不会收到任何提示。
本机复现(umask 022):
$ python -m relax.entrypoints.repetition tests/fixtures/repetition/middle_repetition.jsonl --output /tmp/perm.json
$ stat -c '%a' /tmp/perm.json # 600
$ chmod 644 /tmp/perm.json
$ python -m relax.entrypoints.repetition tests/fixtures/repetition/middle_repetition.jsonl --output /tmp/perm.json
$ stat -c '%a' /tmp/perm.json # 600(被静默改回)该 CLI 的典型用法是把报告写进共享的 rollout 结果目录,其他账号或后续采集进程可能因此读不到报告。建议在 os.replace 之前显式设定权限,例如按 umask 计算、或在目标已存在时沿用其原有 mode:
current_umask = os.umask(0)
os.umask(current_umask)
os.chmod(temporary, 0o666 & ~current_umask)There was a problem hiding this comment.
已在 f2babd93 验证修复,F3 关闭。
relax/entrypoints/repetition.py:143-144 在 os.replace 前按既有报告的 st_mode & 0o777 设定临时文件权限。原始复现场景(umask 022):
$ python -m relax.entrypoints.repetition tests/fixtures/repetition/middle_repetition.jsonl --output /tmp/perm2.json
$ stat -c '%a' /tmp/perm2.json # 600(新报告私有,文档已说明)
$ chmod 644 /tmp/perm2.json && python -m relax.entrypoints.repetition ... --output /tmp/perm2.json
$ stat -c '%a' /tmp/perm2.json # 644(保留,不再是此前的静默降级)
- 新增
tests/utils/test_repetition_report_permissions.py覆盖 0600/0640/0644/0664 的保留、新报告私有、失败时原文件与权限不变且无残留临时文件;本地与 CI 均通过。 - 中英文文档同步说明了新报告默认
0600、替换时保留读/写/执行位、以及所有者、ACL 与扩展属性不复制。
# 🐛 Bug Fix - Restore the module-level is_port_available export used by get_free_port and weight-sync callers; retain the lazy Ray import. - Preserve existing POSIX report permission bits during atomic replacement and document private new-report defaults. # ✅ Tests - Add CPU port tests using real sockets and POSIX report permission regression tests. - Windows targeted validation: 86 passed, 30 skipped (24 training-environment tests and 6 POSIX cases). - All-file pre-commit compatibility checks pass with Python 3.12 for docformatter. # 📝 Documentation - Explain per-batch linear CPU cost of full-response training metrics and retain complete window coverage.
rai-studio-bot
left a comment
There was a problem hiding this comment.
审查结论:通过
已针对 f2babd93 复审。上一轮的 3 项发现均已修复并经证据核验,本轮未发现新的阻塞或非阻塞问题。
- F1(P1,已关闭):
relax/utils/misc.py:13恢复is_port_available的模块级导入、get_free_port改回读取全局名,import ray仍保持延迟;此前失败的 GPU Unit Tests 在新 head 上由6 failed, 2779 passed转为 2794 passed, 18 skipped, 0 failed,tests/distributed/ray/test_weight_sync_port_hardening.py全部转绿。 - F2(P2,已关闭):中英文文档说明了训练指标全文扫描的成本随批次字符数线性增长、检测器基准不覆盖整批聚合;实现按原设计保留完整窗口覆盖。
- F3(P3,已关闭):
relax/entrypoints/repetition.py:143-144在os.replace前按既有报告的权限位设定临时文件;本地复现确认既有0644报告重跑后保持644,新增回归测试覆盖多种权限与失败路径。
各发现的详细证据与复现命令已在对应行级线程中,此处不再重复;新 head 上 7 项检查全部通过。
|
第六题已由 #329 完成,本 PR close,感谢参与~ |
What
Implement No.6 — 长响应全文重复检测与离线诊断 (full-response repetition detection and offline diagnostics) from the second Relax contributor program.
This detects highly repetitive text anywhere in
Sample.responseand helps locate suspicious regions in saved rollouts. Compression is used only as a statistical measurement; this feature does not summarize or compress context/memory, rewrite responses, stop generation, or change rewards. Tool observations remain part of the original response.Deliverables:
has_repetition(...) -> booland the existingrepetition_fracsample-level metric.Why
Relates to #321, specifically task No.6. Claim: #321 (comment) (bugkeep / Shuan-xx). This PR covers No.6 only; the team's separately claimed No.8 is outside its scope. No RFC is required for No.6.
Previously,
has_repetitiononly inspected the last 10,000 characters and only when the response was longer than 10,000 characters. A response with a severely repetitive middle and an ordinary ending therefore produced a false negative. The committed fixturetests/fixtures/repetition/middle_repetition.jsonldemonstrates this: the old suffix check stays below the threshold while the full scan finds the middle window.Reference: radixark/miles#3115. Unlike that PR's later fixed-window-count optimization, this implementation never widens the stride or drops intermediate windows to cap cost.
How
Detection semantics
nullmaximum. Nonempty short responses and exactly-one-window responses are checked once. This intentionally expands the historical metric's coverage, including short responses.Integration and offline use
The existing production
compute_metrics_from_samplesimplementation is moved unchanged intorelax/utils/metrics/rollout_metrics.pyand re-exported by the distributed rollout module. The Ray import inget_current_node_ipis deferred;misc.is_port_availableremains a module-level export used byget_free_port. This lets CPU tests exercise the same production aggregation without starting inference engines; metric names and sample-level denominators are retained.Training metrics scan every nonrepetitive response without a window limit. With fixed window/stride sizes, per-step CPU cost grows with the total response characters across the batch. Detector-only benchmarks do not establish whole-batch aggregation cost or training throughput.
On POSIX, atomic report replacement preserves existing read/write/execute permission bits. New reports default to private
0600; ownership, ACLs and extended attributes are not copied. These rules are documented in both usage guides.JSONL processing streams one record at a time and needs no Torch/Ray/SGLang dependencies. Torch loads on CPU with restricted deserialization by default; trusted legacy object dumps require explicit
--trusted-torch. Records preserve physical source/position and available rollout/sample/group/dataset identifiers. Invalid records fail with a location instead of disappearing silently. Reports are published only after a successful complete scan.Usage:
docs/en/guide/repetition-diagnostics.mdanddocs/zh/guide/repetition-diagnostics.md.Testing
Includes upstream
mainat0651812.Review-fix validation: 86 passed, 30 skipped on Windows, covering the three repetition suites plus real-socket port tests, POSIX report-permission tests, and the existing weight-sync port-hardening suite. Skips comprise 23 weight-sync tests and one logger test requiring unavailable training dependencies, plus six POSIX-only cases. WSL has no pytest installed; POSIX cases await Linux CI. Full-file pre-commit compatibility checks passed with Python 3.12 for docformatter. The prior GPU run at
961d092had 2779 passes and exactly six failures involving the removedmisc.is_port_availableexport; the new commit restores that export. Its GPU integration workflows passed both Qwen training cases. The new commit requires fresh CI results.91 passed, 2 skipped. Tests cover beginning/middle/end repetition, the fixed middle-repeat/normal-tail fixture, empty/short/exact-window responses, overlap boundaries, unaligned suffixes, Chinese/emoji/combining-mark offsets, controls, threshold equality, maximum calculation, union coverage, invalid input/configuration, and legacy compression compatibility. Integration tests call the real production sample aggregation and actual dump writers, reader, and CLI, without replacing the detector or aggregation with mocks.
The two skips require the unavailable Ray/SGLang/Megatron training environment (the optional distributed logger wiring check and an existing eval-logger test). No GPU/multinode training run or full
pytest tests/pass is claimed.pre-commit run --all-filescompatibility run passes (all hooks executed; details below)pytest tests/) — targeted results above; full training dependencies unavailable locallyLocal pre-commit was run over all files in an isolated Windows worktree. A temporary, uncommitted configuration selected existing Python 3.12 for the original pinned docformatter hook (Python 3.13 removed
lib2to3). Windows checks out upstream symlinks as plain placeholder files; after the EOF hook normalized those placeholders, the all-file rerun passed. Those 12 unrelated placeholder-only changes were excluded and restored. All changed-file hooks also pass. No hooks were disabled or project hook configuration changed; upstream Linux CI remains authoritative.Type of Change
Screenshots / Logs
Reproduce the actual detector benchmark:
benchmarks/repetition/windows-python313.jsonrecords 24 cases: 10k/100k/1m characters, nonrepetitive control / middle repetition / full repetition / Chinese middle repetition, detailed and boolean APIs. Each case uses a fresh subprocess and seven timing batches. Full results and methodology:benchmarks/repetition/README.md.Detailed scan of the nonrepetitive control on Windows 11, Python 3.13.9, zlib 1.3.1, 12 logical CPUs:
Native process peaks include interpreter/imports, input creation, warmup and native zlib allocations, rather than claiming incremental scanner memory. Separate
tracemallocscan peaks, raw timings, and environment metadata are included. Timing excludes input generation/imports and memory tracing; these measurements are not end-to-end training throughput.Required CI and
rai-studio-botreview must pass before requesting the community's manual mentor review. This PR does not claim those pending checks are complete.