Conversation
# ⭐ Feature ## Detect repetition anywhere in the response - Replace the suffix-only check with an overlapping window scan over the full text, so repetition in the head or middle of a response is no longer missed when the tail happens to be clean - Scan 10,000-character windows at a 5,000-character stride, keeping the existing zlib compression-ratio criterion and its threshold of 10 so verdicts stay comparable with previous runs - Cover short responses with a single window and always align a final window to the exact suffix, leaving no gap at the end of the text - Report per-window compression ratios, suspected character intervals and the maximum ratio via `scan_repetition`, while `has_repetition` keeps its boolean interface and short-circuits at the first hit for the online metric ## Add an offline diagnosis entrypoint - Add `python -m relax.entrypoints.diagnose_repetition` to diagnose dumped rollout data and emit a per-sample JSON report - Read `.jsonl` rollout results incrementally, importing `torch` lazily and only for the pickled `.pt` dumps written by `--save-debug-rollout-data` - Validate window parameters, thresholds and dump contents up front, and refuse to overwrite an input dump --- # ♻️ Refactor ## Split detection out of the metrics stack - Move the detection core into `relax/utils/repetition.py`, which imports only the standard library so offline diagnosis runs without torch or the metrics service stack - Re-export the public names from `relax.utils.metrics.metric_utils` so the historical import path and the `rollout/repetition_frac` call site are unchanged, and online and offline share one implementation and one threshold --- # ✅ Tests ## Cover the scan, the CLI and dump compatibility - Test repetition at the beginning, middle and end, clean negative controls, empty and short responses, and character-based offsets for CJK text - Test window bounds for stride alignment, gap-free coverage, the unaligned tail and rejection of invalid window parameters - Test threshold boundaries, early exit, union counting of overlapping hits and JSON serialization of the report - Diagnose dumps written by the real dump writer and assert the offline verdict matches the online rollout metric entry - Add `tests/utils/benchmark_repetition_scan.py` to reproduce the CPU time and peak-memory numbers quoted in the docs --- # 📝 Documentation ## Document repetition detection - Add bilingual guides covering the detection criterion, window parameters, the diagnosis workflow and report fields - Register both pages in the VitePress sidebars Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@rai-studio-bot Please review this PR. |
|
辛苦 merge 下最新 main,新增了两条流水线 |
|
已 merge 最新 main(#324 的两条 GPU 流水线),无冲突,新的流水线正在运行。 |
|
@SigureMo |
rai-studio-bot
left a comment
There was a problem hiding this comment.
审查结论
未发现阻塞问题。两条 P3 建议见行内评论:测试在缺少 torch/ray 时会失败而非跳过;has_repetition 可委托 scan_repetition 复用同一套遍历。
审查基于 292c03fd:该提交把 main 合入本分支,9 个改动文件与 883254ee 逐字节一致(仅带入 CI 文件),对两个版本都成立。
本轮独立核对过的几点:
- 检测能力是旧实现的超集:任意响应长度下,窗口列表都包含末尾那个
(len-10000, len)窗口,旧实现能命中的新实现都能命中,不存在漏报回归;窗口并集覆盖全文且不重复计数,已用暴力枚举核对边界与末段对齐。 - 成本:全文扫描按响应长度线性增长。Linux x86 实测干净文本 1 万字符 0.36 ms、100 万字符 68 ms(文档中的 24 ms 来自 macOS);按 512 样本、每样本约 1 万字符估算,每个 rollout step 新增约 0.2 s,属于可接受范围。
- 指标语义:
rollout/repetition_frac名称不变而判定范围变宽,新旧数值不可直接比较——PR 描述与文档已明确说明;仓库内该指标没有下游消费(无告警或阈值配置引用),因此不会改变训练行为。 - 本轮 7 项检查全部通过:pre-commit、Python 3.10/3.11/3.12、H20 单测,以及
Qwen3-4B-4xgpu-async、Qwen3-VL-4B-2xgpu两条 GPU 集成流水线。
# ♻️ Refactor ## Delegate the boolean predicate to the shared scan - Replace the duplicated window walk in `has_repetition` with a call to `scan_repetition(text, stop_at_first_hit=True)`, so traversal and the threshold comparison exist once instead of twice and the online metric cannot drift from the offline report --- # ✅ Tests ## Skip, rather than fail, without training dependencies - Import `torch` and `ray` through `pytest.importorskip` in the two remaining bare-import sites, matching the rest of the file - Without `torch`/`ray` the suite now reports 52 passed and 8 skipped instead of 1 failed, 52 passed and 4 errors; with them it still reports 60 passed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nyanpasu 审查看板审查状态: ✅ 已通过 审查版本: e361264 审查通过:F1–F6 均已解决,中英文开销说明已与只保存命中窗口的实现一致。本次仅修改两个文档文件,功能代码及测试未变,沿用此前 52 passed、8 skipped(缺少 torch/ray)和 84 组 JSON 等价性验证结果。当前提交尚无 CI 检查结果;本地无多节点 GPU 环境,未运行分布式集成测试。
Powered by Nyanpasu with gpt-6-astra medium, please check the suggestions carefully.
|
|
@rai-studio-bot F1、F2 均已在 |
rai-studio-bot
left a comment
There was a problem hiding this comment.
审查结论
未发现阻塞问题。上一轮两条 P3 建议在本提交均已采纳并核实解决,新增 1 条 P3 文档建议(见行内评论)。
上轮建议的处置(基于 7130c811 重新核对):
- 测试在缺少
torch/ray时优雅跳过 → 已解决:pytest tests/utils/test_repetition_detection.py -q在纯 CPU 环境为52 passed, 8 skipped(此前1 failed, 52 passed, 3 skipped, 4 errors)。 has_repetition委托scan_repetition→ 已解决:在线指标与离线报告现在共用同一趟扫描、同一套阈值;等价性已重新核对(60 例全部一致,其中 33 例命中),且「旧实现命中而新实现不命中」为 0 例,不存在漏报回归。
本轮新发现:docs/en/guide/repetition-detection.md:191-193 与 docs/zh/guide/repetition-detection.md:165-166 的「开销」段落仍写着布尔接口「不保存压缩比列表、辅助内存 O(window_size)」。委托之后 has_repetition 走的就是 scan_repetition,后者为每个已扫描窗口保留一个压缩比,因此该界应改为 O(window_size + K)(干净长文本下 K 为全部窗口数,100 万字符约 199 项、约 6 KB,影响可忽略,但表述已不准确)。这条文档漂移正是我上一轮的重构建议带来的,建议随本 PR 一并修正。
CI:7130c811 目前为 pending,尚无 check-run 结果,故本轮不对 CI 作结论。
# 📝 Documentation ## Describe the memory both entry points actually use - State that `has_repetition` delegates to `scan_repetition` with the early exit, so its auxiliary memory is `O(window_size + K)` rather than `O(window_size)`: the shared scan keeps one ratio per scanned window - Give both ends of `K`: 1 when a response repeats from the start, and the full window count for clean text (199 ratios, about 6 KB, at one million characters) - Apply the same correction to the English and Chinese guides Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rai-studio-bot
left a comment
There was a problem hiding this comment.
审查结论
审查通过,无阻塞项。
F1、F2 已在 7130c811 修复,F3(文档内存上界)已在 52235bdf 修复,三条均已核实关闭;详细证据见各自的行内讨论串。
- F1/F2:纯 CPU 环境
pytest tests/utils/test_repetition_detection.py -q为52 passed, 8 skipped(60 个用例无失败/报错);has_repetition委托scan_repetition后的等价性复核全部一致,无漏报回归。 - F3:中英文文档同批改正,改写后的表述与实现一致——提前退出下
K在开头即重复时为 1、纯净文本为全部窗口数;实测 100 万字符为 199 个窗口、压缩比列表约 6.3 KB,与文档所述「约 199 项、约 6 KB」相符。
本轮改动仅涉及两个文档文件(+11/-8),功能代码与 7130c811 逐字节一致;CI 目前尚无 check-run 结果,故本轮不对 CI 作结论。
|
@SigureMo 任务已完成,请求 review。 |
|
@rai-studio-bot 重点 review 是否过度设计,重点检查哪些代码是不必要的,可以参考 https://github.com/ShigureLab/nyanpasu/blob/main/AGENTS.md |
rai-studio-bot
left a comment
There was a problem hiding this comment.
针对设计复杂度的补充审查已完成:有局部可精简之处,未发现需要阻塞合并的问题,具体建议见行内评论。
整体拆分有实际需求支撑:标准库检测核心支持无训练依赖的离线运行,既有 dump 读取、真实指标入口集成测试和 CPU/内存基准均属于任务验收范围,建议保留。优先收窄报告状态和公共导出范围,删除无消费者的兼容别名,无需引入更多抽象。
基于 52235bdf;本地检测测试 52 passed、8 skipped(缺少 torch/ray),GitHub 7 项检查通过。本地无多节点 GPU 环境,未重跑分布式集成测试。此前 F1–F3 保持已解决。
# ♻️ Refactor ## Keep only the state the report is read for - Replace `RepetitionReport.compression_ratios` with a `num_windows_scanned` counter and an incrementally tracked maximum: the list was only ever read for its length and its maximum, and every hit window already carries its own ratio, so a clean one-million-character scan no longer holds 199 floats that nothing reads - Derive `has_repetition` from `hit_windows` instead of storing it alongside them, so the boolean cannot disagree with the list it summarizes - `to_dict()` is unchanged: the same seven fields with the same meanings ## Re-export only what an existing caller imports - Narrow the `metric_utils` re-export to `has_repetition` (the historical entry point `relax/distributed/ray/rollout.py` imports) and `scan_repetition` (shown in the guides); the constants, dataclasses and `repetition_window_bounds` are new in this PR with no caller going through `metric_utils`, so a second public import path for them maintained itself for nobody --- # ✅ Tests ## Drop the alias the benchmark invented for itself - Report mean wall time as `wall_mean_ms` in both the single-response and batch results, removing the duplicated `mean_ms` field that was annotated as an alias for "existing consumers" in a script this PR itself introduces - Update the guides' field description accordingly; CPU time, traced memory and independent-process RSS each measure something distinct and stay Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
8894e52 to
6119bfb
Compare
rai-studio-bot
left a comment
There was a problem hiding this comment.
6119bfbe 的精简修改已复核,F4–F6 已解决,未发现功能回归。仍有一处非阻塞文档遗漏:删除压缩比列表后,开销说明未同步,已在原 F3 讨论补充具体修改建议。
本地测试 52 passed、8 skipped(缺少 torch/ray);84 组新旧 JSON 报告一致,单响应和批次基准字段验证通过。当前 CI 仍在运行;本地无多节点 GPU 环境,未运行分布式集成测试。
# 📝 Documentation ## Match the cost section to the narrowed report - State that the boolean path retains nothing per window, so its auxiliary memory is `O(window_size)`, and that the detailed report keeps one entry per hit window, giving `O(window_size + H)` for `H` hit windows - Drop the "199 ratios, roughly 6 KB" figure: the scan no longer allocates a per-window ratio list, so clean text of any length now holds no window entries at all, while the scanned-window count itself is unchanged - Apply the same correction to the English and Chinese guides Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rai-studio-bot
left a comment
There was a problem hiding this comment.
审查通过。剩余开销文档问题已在 e3612640 修复,此前精简建议均已落实,无待处理审查问题。
本次仅改中英文文档,功能代码与已验证版本一致,沿用此前 52 passed、8 skipped(缺少 torch/ray)及 84 组 JSON 等价性验证结果。当前提交尚无 CI 检查结果;本地无多节点 GPU 环境,未运行分布式集成测试。
What
Extend repetition detection from the tail of a response to its full text, and add an offline diagnostic for dumped rollout data.
has_repetitionnow scans the whole response with overlapping windows, keeping the existing zlib compression-ratio criterion and its threshold of 10, and keeping its boolean interface. A newscan_repetitionadditionally reports the hit windows' character intervals, their compression ratios and the maximum ratio. A new entrypoint reads existing rollout dumps and writes a JSON report.Relates to #321, contributor program cohort 2, No.6 长响应全文重复检测与离线诊断. No RFC is required for this task.
Why
The previous check only looked at
text[-10000:], so a response that degenerates early and then recovers a clean ending was reported as non-repetitive. Therollout/repetition_fracmetric therefore undercounted exactly the failure mode it exists to track, and the metric alone could not say where in a response the repetition was.How
relax/utils/repetition.py, which imports only the standard library, so offline diagnosis runs on a plain CPU box withouttorch,numpyor the Ray/metrics service stack.relax.utils.metrics.metric_utilsre-exports the public names, so the historical import path and therollout/repetition_fraccall site are unchanged.text[start:end],startinclusive andendexclusive. Counting code points rather than bytes keeps offsets meaningful for CJK responses. The intervals mark suspected repetitive windows, not exact repetition boundaries.covered_charsas the union of the hit intervals, so overlapping windows are never counted twice.has_repetitionshort-circuit at the first hit, which is all the online metric needs. The offline diagnostic keeps every window so ratios are complete.python -m relax.entrypoints.diagnose_repetition, which reads.jsonlrollout results incrementally and importstorchlazily, on the pickled.ptpath alone. It emits per-sample identity (position,dataset,index,sample_index,group_index,rollout_id), hit intervals, ratios and an aggregaterepetition_fracmatching the online definition. Window parameters, thresholds and dump contents are validated up front, and the CLI refuses to overwrite an input dump.Scope kept minimal. The online call site in
relax/distributed/ray/rollout.pyis not modified:metric_utilsre-exports the detector, sorollout/repetition_fracpicks up full-text scanning through the existing import. Nothing in the rollout, metrics-service or reward paths is touched, which keeps the diff reviewable and the blast radius confined to one new standard-library module.Detection runs on
Sample.responseas-is. Roles are not distinguished, semantic repetition is not identified, and neither generation truncation nor reward is touched.Metric comparability. Thresholds and window parameters are unchanged in value, but two classes of response flip from non-repetitive to repetitive, both intended. Repetition outside the final window is now seen at all. Responses of 10,000 characters or fewer are now checked at all: the previous implementation guarded on
len(text) > 10000, so a fully repetitive 10,000-character response was unconditionally reported as clean.rollout/repetition_fracmay therefore rise on affected runs with no change in model behavior, and values from old and new runs should not be compared directly.Testing
pre-commit run --all-filespasses.pytest tests/utils/test_repetition_detection.py): 60 passed.tests/utils/was run on this branch and on its parent2a8d2ed: both report an identical 46 failed and 42 errors, all from unrelated modules whose optional dependencies are absent on this host, while passes go from 417 to 477 — exactly the 60 tests added here. No regressions.Fixed samples cover repetition at the beginning, in the middle with a clean ending, and at the end, plus a clean negative control. Other cases cover the empty string, a short response below one window, exactly one window, window boundaries, the unaligned final window, Chinese text with character-based offsets, a compression ratio exactly at the threshold (not a hit) and just above it (a hit), and union counting of overlapping hits.
Integration is verified through the real entrypoints rather than fixtures: four tests write dumps with the production writers (
save_rollout_result_jsonland the.ptdebug dump), read them back throughdiagnose_dumps, and assert the offline verdict matches therollout/repetition_fracentry produced by the real rollout metric path, for both dump formats and both train and eval dumps.No accelerator or distributed execution was run locally, because this macOS host has no accelerator cluster. The change is CPU-only string processing and does not touch distributed code paths.
Screenshots / Logs
tests/utils/benchmark_repetition_scan.pyreproduces the cost numbers quoted in the documentation. Measured on macOS 27.0 arm64, Python 3.14.7, five timing repeats, no early exit:Clean text is the worst case, since every window must be scanned: a one-million-character response costs about 23 ms of CPU. Traced peak memory stays near the size of one window regardless of response length. The documentation also records a batch benchmark of the online predicate and the measurement methodology.
Type of Change