Repository navigation
Conversation
…antile metrics
# ⭐ Feature
## Stop-reason distribution
- Add `compute_stop_reason_metrics` pure aggregation in
`relax/utils/metrics/metric_utils.py`: buckets each sample into exactly one
stop reason — explicit `metadata["rollout_stop_reason"]` (normalized via
`strip().lower()`) first, then `sample.status` (the 5-value `Sample.Status`
enum), then an explicit `unknown` fallback — and emits
`stop_reason/{reason}/count` plus `stop_reason/{reason}/frac`, whose fractions
sum to 1 over the whole batch (denominator = all samples, `unknown` included).
## Turn-count tail quantiles
- Add `compute_num_turn_metrics` pure aggregation reading
`metadata["rollout_turns"]` (missing key still counts as 1, preserving the
previous inline semantics): keeps `num_turn/mean|max|min` output-identical
and adds `num_turn/p50|p90|p95|p99` via `np.percentile` (default linear
interpolation) to surface long-tail trajectories.
## Wiring
- `compute_metrics_from_samples` now merges both helpers, replacing the three
inline `num_turn` lines; the rollout main loop, stop conditions and
scheduling flow are unchanged and no state fields were added.
# ✅ Tests
- New `tests/utils/test_metric_utils_stop_reason_turns.py` covering: empty
input returns `{}`, single-sample stats, mixed reasons with fractions
summing to 1, explicit reason taking precedence over status, empty /
non-string reason fallback, reason normalization, all 5 enum statuses
bucketed, missing status falling into `unknown`, linear-interpolation
percentile values pinned (p50=5.5, p90=9.1, p95=9.55, p99=9.91 for turns
1..10), missing `rollout_turns` defaulting to 1, and a megatron-gated test
asserting the new keys flow through `compute_metrics_from_samples`.
|
Hi maintainers 👋 This PR is from a first-time fork contributor, and the workflow runs are awaiting approval ( （首次从 fork 提交，CI 运行需要维护者批准，麻烦有空点一下，感谢！） |
怎么还要给我们总任务给 close 了呢？ |
SigureMo
changed the title
SigureMo
changed the title
SigureMo
changed the title
|
抱歉！已修正 PR 描述：移除了 |
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
Review 结论
方向与官方任务口径一致：纯聚合函数、不碰 rollout 主循环与停止条件、接入 compute_metrics_from_samples 的方式正确。我另外把改动函数从源码里抽出来跑了 3000 组随机输入做交叉验证：num_turn/mean|max|min 与原内联实现逐位一致，frac 之和恒为 1、count 之和恒等于样本数，分位数与任务示例（1..10 → 5.5 / 9.1 / 9.55 / 9.91）一致；空输入返回 {} 也成立。
但停止原因的回退链需要先修一处，另有两项与任务 issue #326 的约定不一致：
需要先修（P1）
_resolve_rollout_stop_reason只读rollout_stop_reason，读不到 agentic 路径真正写入的stop_reason，分布会退化成状态枚举桶（见行级评论）。
建议同批处理（P2）
compute_num_turn_metrics未归一化轮数，且metadata非 dict 时抛异常（见行级评论）。- 总入口对空输入仍会抛异常，与验收项 1 / #326 的要求不一致（见行级评论）。
非行级：PR 描述关联的 issue 指向错误（P3）
Closes #321 指向的是「开源贡献者计划第二期｜活动流程指南」这个长期活动总 Issue，合入后会被自动关闭（@SigureMo 已在 PR 下提出同一问题）；本任务自己的 issue 是 #326【No.9】Agentic Rollout 停止原因分布与轮数分位数指标。建议改为 Closes #326。
非行级：占比键名与任务 issue 约定不一致（P3）
#326 约定的键名是 stop_reason/{reason}/ratio（示例：rollout/stop_reason/unknown/ratio），本 PR 输出 .../frac。仓库里 repetition_frac 与 truncated_ratio 两种命名都有先例，但如果验收以 #326 为准，改名的成本现在最低，建议先和导师确认口径。
测试文件位置与 #326 建议的 tests/distributed/ray/test_rollout_metadata_metrics.py 不同，内容本身完整（空输入/单条/多类/缺失 metadata/归一化/枚举全覆盖/键序/线性插值精确值），这一条只作提示，不影响结论。
补充说明：megatron 门控的接入测试在本机无法执行（环境缺 ray/megatron），我只做了静态核对——compute_metrics_from_samples 触及的 _compute_zero_std_metrics、_compute_spec_metrics、_compute_prefix_cache_metrics、_compute_reward_cat_metrics、compute_mopd_metrics 对测试里的 SimpleNamespace 都走 getattr(...) 默认值，不会因缺属性失败；实际结论以 CI 为准。
| status = getattr(sample, "status", None) | ||
| if isinstance(status, Sample.Status): | ||
| return status.value | ||
| if isinstance(status, str) and status.strip(): | ||
| return status.strip().lower() | ||
| return "unknown" |
There was a problem hiding this comment.
回退链只读 rollout_stop_reason，漏掉了 agentic 路径真正写入的 stop_reason，这些配方下新增的分布会退化成状态枚举桶：
- 生产方写的是
stop_reason：examples/deepeyes_agentic/app/agent.py:69、examples/deepeyes_v2_agentic/app/agent.py:239、examples/on_policy_distillation/agentic_opd/*/app/agent.py返回的 metadata 里都是stop_reason；它经relax/agentic/session/service.py:2781（_merge_export_metadata(unit_metadata, output_record["metadata"])）合并、由relax/agentic/session/state.py:744进入sample.metadata。仓库里已有消费者按该键取值：examples/on_policy_distillation/agentic_opd/alfworld/reward_alfworld.py:51用metadata.get("stop_reason")做 one-hot。而rollout_stop_reason目前只有旧的examples/deepeyes/rollout.py:434在写。 - 影响：这些 agentic 配方下只会输出
stop_reason/completed|truncated|pending/...，拿不到env_done/max_turns/finish_length——正是用来区分「哪个预算在起作用」、把高truncated_ratio归因下去的那部分信息，也是本任务存在的理由。 - 影响面：
status兜底只在「没写原因字段」的路径上生效——旧 DeepEyes rollout 正常返回前必定调用_record_rollout_stats（examples/deepeyes/rollout.py:579-581），所以它基本只在拿不到真实原因时触发。另外stop_reason/truncated/frac与已有的rollout/truncated_ratio语义重叠且数值可能不一致，并和reward_alfworld.py产出的stop_reason/{reason}系列共用前缀、混入两套词表。
建议第二级改读兼容字段，两级都无有效值时归 unknown（官方任务口径同样是「缺失或空值归入明确的 unknown 类别」，而非按状态推导）：
compat_reason = sample.metadata.get("stop_reason")
if isinstance(compat_reason, str) and compat_reason.strip():
return compat_reason.strip().lower()
return "unknown"test_single_sample_falls_back_to_status、test_every_status_value_gets_its_own_bucket、test_mixed_reasons_fractions_sum_to_one、test_missing_status_counts_as_unknown 目前固化了状态兜底与两套词表混用，需要一并调整。
| """ | ||
| if not samples: | ||
| return {} | ||
| turns = [sample.metadata.get("rollout_turns", 1) for sample in samples] |
There was a problem hiding this comment.
rollout_turns 未做归一化，且 metadata 非 dict 时会直接抛异常：
sample.metadata = None→AttributeError: 'NoneType' object has no attribute 'get'。同一路径的其他消费方都做了防御：relax/utils/opd/opd_utils.py:1490用(s.metadata or {})，examples/deepeyes_v2_agentic/reward_deepeyes_v2.py:774、examples/on_policy_distillation/agentic_opd/alfworld/reward_alfworld.py也各做了or {}/isinstance判断。任务 issue 【No.9】Agentic Rollout 停止原因分布与轮数分位数指标 #326 明确要求「metadata 为None或字段缺失 → 使用已有单轮默认值1」。- 非法值会原样进入统计（实测）：
{"rollout_turns": -3}→num_turn/min=-3；True/False被当作 1/0（num_turn/max=True）；2.5参与均值与分位（test_float_rollout_turns_are_aggregated_as_is固化了该行为）。【No.9】Agentic Rollout 停止原因分布与轮数分位数指标 #326 的归一化表要求：非负整数保留（0有效）、数学上为整数的浮点转 int、None/布尔/字符串/小数/负数/NaN/±inf/容器一律回退1。
建议抽一个私有归一化助手（metadata 缺失同时给 _resolve_rollout_stop_reason 用上）：
def _normalize_rollout_turns(sample: Sample) -> int:
metadata = sample.metadata if isinstance(sample.metadata, dict) else None
value = metadata.get("rollout_turns") if metadata else None
if value is None or isinstance(value, bool) or isinstance(value, (str, list, tuple, dict)):
return 1
if isinstance(value, (int, np.integer)):
return int(value) if value >= 0 else 1
if isinstance(value, (float, np.floating)) and np.isfinite(value) and value >= 0 and float(value).is_integer():
return int(value)
return 1现有 test_float_rollout_turns_are_aggregated_as_is 与「非法轮数默认 1」的口径冲突，需要相应调整。
| log_dict["num_turn/mean"] = np.mean([s.metadata.get("rollout_turns", 1) for s in samples]).item() | ||
| log_dict["num_turn/max"] = np.max([s.metadata.get("rollout_turns", 1) for s in samples]).item() | ||
| log_dict["num_turn/min"] = np.min([s.metadata.get("rollout_turns", 1) for s in samples]).item() | ||
| log_dict |= compute_num_turn_metrics(samples) |
There was a problem hiding this comment.
两个聚合函数对空输入已返回 {}，但总入口在到达这两行之前就会抛异常：compute_statistics(response_lengths) 对空数组取 np.max，实测得到 ValueError: zero-size array to reduction operation maximum which has no identity（relax/utils/metrics/metric_utils.py:82-89 没有空值保护，在它前面的 _compute_min_mean_max_stats 反而有）。
这不是本 PR 引入的，但 _log_eval_rollout_data 的判断是 if (samples := data[key].get("samples")) is not None（relax/distributed/ray/rollout.py:4781），"samples": [] 会进入该分支，所以评估侧存在可触发路径；任务 issue #326 也把「总入口也需要在任何统计和参数读取前处理空输入」列进了实现范围，PR 描述里的验收项 1 同样写的是空输入不抛异常。
建议在 compute_metrics_from_samples 开头早退一行：
if not samples:
return {}
Nyanpasu 审查看板审查状态： 🚧 需要修改 审查版本： a0259a7 目标分支： main 已按 a0259a7 复查（维护者将 main 合入特性分支触发本轮）：逐位比对确认 PR 所属改动与已审的 0c5cc07 完全一致，合入仅带入 main #329 的 has_repetition 重构且与新增聚合函数无交叉；新头上 CI 复现且仅复现 F6——Unit Tests (H20, 4 GPUs) 唯一失败仍是新增用例末行的 flushed 断言（1 failed, 2953 passed, 30 skipped），其余检查全绿。F6（P1）与 F7（P3，PR 描述仍写已移除的 status 回退/strip().lower()/frac 键名并引用已删除的测试文件）维持未解决，结论与上一轮一致：修正该断言并同步 PR 描述即可，无需其他改动。本轮完成生产/测试必要性审计（见审查阶段表），无新增发现。
CI 异常检查版本： a0259a7 · 已分析
审查发现待处理
已解决或已取代提交范围 · 接收 4 · 建议移出 0 · 待确认 0接收 4 个文件 · 建议移出 0 个文件 · 待确认 0 个文件。移出与待确认部分暂停深审，不代表审查通过。
精简审查与验证依据
生产代码的必要性与替代方案
测试的必要性与替代方案
Powered by Nyanpasu with glm-5.3[1m] xhigh, please check the suggestions carefully.
|
这种回复……建议……还是手动回复吧，连这么基本的对话都直接用 Agent 回我有点担忧对题目的理解程度 |
yes sir ，不好意思 |
…uard with task redai-studio#326 # 🐛 Bug Fix ## Stop-reason fallback chain (review P1) - `_resolve_rollout_stop_reason` now reads the legacy `metadata["rollout_stop_reason"]` first and the agentic `metadata["stop_reason"]` (written by DeepEyes agentic apps via the session service merge) second, falling back to `unknown` only when neither holds a valid value. Stop reasons are no longer derived from `Sample.status`. - Valid values are non-blank strings; surrounding whitespace is stripped while case and inner content are preserved. An explicit `unknown` value no longer falls back. ## Turn normalization (review P2) - New `_normalize_rollout_turns`: non-negative Python/NumPy integers are kept (0 valid), finite non-negative integral floats convert to int, and None / bool / string / fractional / negative / NaN / inf / container values — plus missing or None metadata — fall back to the single-turn default of 1. - `compute_num_turn_metrics` computes the four quantiles from a single `np.percentile(turns, [50, 90, 95, 99])` call (linear interpolation). ## Entry-point empty guard (review P2) - `compute_metrics_from_samples` returns `{}` immediately for empty input, before any statistics or args reads; the partial-rollout staleness read now tolerates `metadata=None` (missing `start_rollout_id` keeps gap 0). ## Naming per task redai-studio#326 - Ratio keys renamed `stop_reason/{reason}/frac` -> `stop_reason/{reason}/ratio`; counts are emitted as float integers. # ✅ Tests - New `tests/distributed/ray/test_rollout_metadata_metrics.py` covering the issue's matrix: empty input (warnings-as-errors), single sample, the four-sample issue example with exact counts/ratios/quantiles, the turn normalization table, reason compatibility (precedence, fallback, whitespace, case, explicit unknown), denominator independence from reward/status, purity/identities, entry integration (megatron-gated) including partial-rollout None metadata and real training/eval logger prefix checks. - `tests/test_agentic_rollout.py`: SessionForest export now asserts `rollout_turns == agentic_trace.turn_count`, real `stop_reason` metadata passthrough and non-mutating aggregation (single- and multi-turn). - Superseded `tests/utils/test_metric_utils_stop_reason_turns.py` removed (coverage consolidated per the issue).
b3d4533 to
0c5cc07
Compare
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
Review 结论
已按 0c5cc07 复查（上一轮的目标版本 b3d4533 已被同内容、仅换行符不同的提交取代）：上一轮的三条行级问题均已修复。我按 #326 的可核对示例与轮数归一化表独立复算确认——四样本示例的 count/ratio 为 2/1/1 与 0.5/0.25/0.25，轮数 min/mean/max 为 1/2/4，P50/P90/P95/P99 为 1.5/3.4/3.7/3.94，与约定一致；stop_reason 兼容读取与优先级、大小写保留、unknown 兜底、空输入与 metadata=None 分支、以及 ratio 键名也都已对齐。
需要先修（P1）
- 新增测试中有一处断言与代码实际行为不符，会让 CI 的
Unit Tests (H20, 4 GPUs)变红，详见行级评论（含建议改法）。
测试位置与覆盖面已按 #326 的建议调整（新增 tests/distributed/ray/test_rollout_metadata_metrics.py，并复用 tests/test_agentic_rollout.py 的 SessionForest 用例补齐导出断言）。
补充说明：megatron 门控用例在本机无法执行（环境缺 megatron/ray），上述结论来自静态核对与对聚合函数的独立执行验证；该 job 在 CI 上会真实执行这些门控用例，上面那条断言问题正源于此。
| assert logged["eval/gsm8k/stop_reason/unknown/ratio"] == pytest.approx(0.5) | ||
| assert logged["eval/gsm8k/num_turn/p99"] == pytest.approx(8.94) | ||
| assert logged["eval/step"] == 0 | ||
| assert flushed == [0] |
There was a problem hiding this comment.
本行断言与代码实际行为不符，会让 CI 的 Unit Tests (H20, 4 GPUs) 变红。
_log_eval_rollout_data 结尾同样会调用 tracking_utils.flush_metrics（relax/distributed/ray/rollout.py:4801），所以第 343 行执行完之后 flushed 已经有两条记录：训练入口那次由第 339 行刚断言过（flushed == [0] 通过，说明第一次 append 确实发生），评估入口再 append 一次，此时 flushed == [0, 0]，本行必然抛 AssertionError。
该用例被 pytest.importorskip("megatron.core") 门控，本地会整条跳过（本机「22 passed, 2 skipped」覆盖不到它），但 CI 的 GPU 单测跑在 ghcr.io/redai-studio/relaxrl:latest 镜像里、megatron 可导入：上一个 head 的同一 job 中，同属门控的 test_compute_metrics_from_samples_reports_stop_reason_and_turn_quantiles 是真实执行的（junit 里 time=0.002、未被 skip），所以这条断言在 CI 一定会执行。
按任务要求「训练/评估均断言刷新调用」，建议把两次刷新都断言出来：
| assert flushed == [0] | |
| assert flushed == [0, 0] |
There was a problem hiding this comment.
CI 已在 0c5cc07d 上跑完，这一处已被复现：Unit Tests (H20, 4 GPUs) 的唯一失败项就是本用例。
tests/distributed/ray/test_rollout_metadata_metrics.py:348: in test_training_and_eval_loggers_publish_prefixed_metrics
assert flushed == [0]
E assert [0, 0] == [0]
E Left contains one more item: 0
整轮结果为 1 failed, 2749 passed, 18 skipped，其余用例（含同文件其余门控用例与 GPU 集成测试）均通过，所以当前只差这一处；按上面的 suggestion 改为断言两次刷新即可。
rai-studio-bot
left a comment
rai-studio-bot
left a comment
There was a problem hiding this comment.
Review 结论
0c5cc07 上 CI 已跑完：唯一失败项是上一轮提出的 F6（新增测试末行断言与代码行为不符），复现输出与改法见该行级讨论；pre-commit、Python 3.10–3.12、GPU 集成测试等其余检查均通过。因此结论不变：修掉该断言即可。
非行级：PR 描述与当前实现已不一致（P3）
Changes段对compute_stop_reason_metrics的说明仍写着strip().lower()归一化、回退sample.status（5 档枚举值）、键名frac。当前实现是去空白但保留大小写、只读metadata["rollout_stop_reason"]与metadata["stop_reason"]、键名为ratio；描述里保留的status回退恰好是任务 issue #326 明确排除的做法，容易被评审误读为未按验收实现。Changes与Test plan引用的tests/utils/test_metric_utils_stop_reason_turns.py（18 个用例）已在本次提交中删除，实际新增的是tests/distributed/ray/test_rollout_metadata_metrics.py；本机验证命令也应对应改为后者。
建议按当前实现改写这两处后再合入，避免评审者照描述核对验收项。
|
本赛题已经由 #330 合入并锁定，感谢参与～ |
4 participants
Task: 【No.9】Agentic Rollout 停止原因分布与轮数分位数指标（Relax 开源贡献者计划第二期，认领记录见 #321 评论区）
Summary
在现有 rollout 指标中补充停止原因分布（计数 + 占比）与轮数分位数（P50/P90/P95/P99），用于识别长尾轨迹和异常停止。全部逻辑为纯聚合函数，只读取输入 samples 并返回指标字典，不修改 sample、不依赖全局状态，不改变 rollout 主循环、停止条件或调度流程。
调研备注：verl / OpenRLHF / TRL / rllm / slime / AReaL / SkyRL / ROLL 八个框架中，停止原因全分布仅 rllm 有（
batch/termination_reason/{reason}，含unknown桶），轮数分位数无一家输出——本 PR 填补该空白，键名与口径对齐生态惯例（小写斜杠、统计量末段、np.percentile默认线性插值）。Changes
relax/utils/metrics/metric_utils.pycompute_num_turn_metrics(samples)：读metadata["rollout_turns"]（缺失按历史默认计 1），保留num_turn/mean|max|min（输出与原内联实现逐位一致），新增num_turn/p50|p90|p95|p99。compute_stop_reason_metrics(samples)：每条 sample 恰落一桶——metadata["rollout_stop_reason"]（非空字符串，strip().lower()归一化；DeepEyes rollout 已在写入该字段）优先，回退sample.status（5 档枚举值），再回退unknown。输出stop_reason/{reason}/count与stop_reason/{reason}/frac，分母为全部 sample，占比之和恒为 1。_resolve_rollout_stop_reason(sample)：上述三级归因链的私有助手。relax/distributed/ray/rollout.pycompute_metrics_from_samples中原三行内联num_turn统计替换为对上述两函数的合并调用；truncated_ratio等既有指标不动。tests/utils/test_metric_utils_stop_reason_turns.py（新增）18 个用例：空输入返回
{}、单条样本、多原因混合（占比和=1）、显式原因优先于 status、空/非字符串原因回退、原因归一化、5 档枚举全覆盖、status 缺失归unknown、float 轮数语义、键序确定性、线性插值分位数精确值锁定（turns 1..10 → p50=5.5/p90=9.1/p95=9.55/p99=9.91）、rollout_turns缺失默认 1、以及 megatron 门控的接入测试（新键经compute_metrics_from_samples正常上报）。Acceptance criteria mapping
num_turn/p50|p90|p95|p99，经compute_metrics_from_samples正常上报num_turn/min|mean|max（提取口径与输出逐位一致）；不改主循环/停止条件/调度；无新增状态字段或控制分支Test plan
pytest tests/utils/test_metric_utils_stop_reason_turns.py tests/utils/test_metric_utils_rloo.py→ 22 passed, 2 skipped（skipped 均为 megatron 门控用例，CI 全量环境执行）num_turn/mean|max|min与原内联实现逐位一致；分位数与手写线性插值公式独立复核一致