Skip to content

test(functional): relax grpo_topp_topk gen_kl_error bound 0.03 -> 0.032#3341

Merged
terrykong merged 1 commit into
mainfrom
terryk/relax-topp-topk-gen-kl-error
Jul 24, 2026
Merged

test(functional): relax grpo_topp_topk gen_kl_error bound 0.03 -> 0.032#3341
terrykong merged 1 commit into
mainfrom
terryk/relax-topp-topk-gen-kl-error

Conversation

@terrykong

@terrykong terrykong commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

What

Relax the grpo_topp_topk functional-test convergence bound:

- 'max(data["train/gen_kl_error"]) < 0.03' \
+ 'max(data["train/gen_kl_error"]) < 0.032' \

Why

tests/functional/grpo_topp_topk.sh asserts max(train/gen_kl_error) < 0.03. gen_kl_error measures the mismatch between the logprobs vLLM used at generation time and the ones the training policy recomputes for the same tokens. Top-p/top-k truncated sampling produces a larger and noisier mismatch than greedy decoding, so the metric sits right at the 0.03 bound and flakes.

Observed failure: fast_L1_Functional_Tests_GRPO_3 failed at 0.03014 — barely over — and needed a rerun:

Data

Mining gen_kl_error for this test across recent GRPO_3 runs:

stat value
passing runs range ~0.0093 – 0.0253
passing-run max 0.0253
the flake 0.0301

0.03 has essentially no margin over the passing distribution. 0.032 clears the observed flake (0.0301) while keeping the bound tight so it stays sensitive to a genuine regression. (The bound has been 0.03 since the test was added in #2053 and was never tuned against observed variance.)

🤖 Generated with Claude Code

@terrykong
terrykong requested a review from a team as a code owner July 24, 2026 21:16
@copy-pr-bot

copy-pr-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

The top-p/top-k GRPO functional test asserts max(train/gen_kl_error) < 0.03,
but top-p/top-k truncated sampling produces a larger, noisier generation-vs-
training logprob mismatch than greedy, so the metric sits right at the bound
and flakes.

Observed on a recent run: the check failed at 0.03014 (barely over 0.03),
requiring a rerun (fast_L1_Functional_Tests_GRPO_3, run 30111867247).
gen_kl_error across recent GRPO_3 runs ranges ~0.009-0.025 on passing runs
with the flake at ~0.030. Bump to 0.032 to clear the observed flake while
keeping the bound tight (regression-sensitive).

Signed-off-by: Terry Kong <terryk@nvidia.com>
@terrykong terrykong changed the title test(functional): relax grpo_topp_topk gen_kl_error bound 0.03 -> 0.05 test(functional): relax grpo_topp_topk gen_kl_error bound 0.03 -> 0.032 Jul 24, 2026
@terrykong
terrykong force-pushed the terryk/relax-topp-topk-gen-kl-error branch from a122c60 to bb191ba Compare July 24, 2026 21:21
@terrykong terrykong added the CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) label Jul 24, 2026
@terrykong

Copy link
Copy Markdown
Collaborator Author

/ok to test bb191ba

@terrykong
terrykong requested a review from yfw July 24, 2026 21:42
@terrykong
terrykong enabled auto-merge (squash) July 24, 2026 21:42
@terrykong
terrykong merged commit 2d3bb79 into main Jul 24, 2026
81 checks passed
@terrykong
terrykong deleted the terryk/relax-topp-topk-gen-kl-error branch July 24, 2026 23:15
macandro96 pushed a commit that referenced this pull request Jul 25, 2026
…32 (#3341)

Signed-off-by: Terry Kong <terryk@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants