Skip to content

fix(opd): score teacher logprobs at rollout temperature, not 0 - #2085

Merged
zhuzilin merged 2 commits into
THUDM:mainfrom
EazyReal:opd-teacher-temperature
Aug 21, 2026
Merged

fix(opd): score teacher logprobs at rollout temperature, not 0#2085
zhuzilin merged 2 commits into
THUDM:mainfrom
EazyReal:opd-teacher-temperature

Conversation

@EazyReal

@EazyReal EazyReal commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Problem

slime/rollout/on_policy_distillation.py scores the teacher with max_new_tokens: 0 (logprobs of the tokens already in the sample, no generation). The payload hardcoded "temperature": 0.

OPD's KL is student (rollout sampling) vs teacher. Teacher logprobs at temperature 0 are a greedy distribution, so the penalty does not match the rollout policy whenever --rollout-temperature is not 0.

Change

"temperature": args.rollout_temperature,

max_new_tokens: 0 is unchanged. Unauthenticated / temp-0 rollouts still send 0.

Test plan

  • One OPD run with --rollout-temperature ≠ 0: teacher /generate payload should carry that temperature (not 0)

@EazyReal

Copy link
Copy Markdown
Contributor Author

@zhuzilin could you review this one? OPD teacher scoring was using temperature=0 while rollout samples use --rollout-temperature; this makes the teacher logprobs match the actual sampled distribution instead of silently scoring a greedy distribution.

@EazyReal
EazyReal force-pushed the opd-teacher-temperature branch from 382ef68 to ca2ea7f Compare June 30, 2026 08:39
EazyReal added a commit to EazyReal/slime that referenced this pull request Jul 7, 2026
@EazyReal

EazyReal commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@zhuzilin refreshed on latest main and checks are green. This scores OPD teacher logprobs at rollout temperature instead of forcing temperature 0, which keeps teacher scoring aligned with generation. Could you review when you have bandwidth?

@EazyReal
EazyReal force-pushed the opd-teacher-temperature branch 2 times, most recently from f1dec0f to a15928c Compare August 21, 2026 00:50
max_new_tokens=0 still only scores the existing sequence; temperature 0
was forcing a greedy distribution that does not match rollout.

Co-authored-by: Cursor <cursoragent@cursor.com>
@EazyReal
EazyReal force-pushed the opd-teacher-temperature branch from a15928c to ca2aa80 Compare August 21, 2026 01:03
@zhuzilin
zhuzilin merged commit 1da1bb1 into THUDM:main Aug 21, 2026
63 checks passed
@kkyyxhll

kkyyxhll commented Aug 24, 2026

Copy link
Copy Markdown

@zhuzilin @EazyReal 这个pr是一个no-op操作,并且这个会引起混淆?默认teacher只需要用到input_token_logprobs,sgl中对于input的logp是temperature=1.0,即保留原始logp。因此不是原来的temperature=0.0 或者 这个pr增加的"temperature": args.rollout_temperature

我新开了一个pr,#2319 ,辛苦review一下。非常感谢!

另外,sgl 的 SGLANG_RETURN_ORIGINAL_LOGPROB和temperature只影响output的logp,input的logp不受影响(opd在这,max_new_tokens=0)
下面是实际测试结果,不同temperature并不影响input的各种logp
起serve:

python -m sglang.launch_server \
  --model /root/Qwen3-0.6B \
  --reasoning-parser qwen3 \
  --tool-call-parser qwen25

temperature=1.0的payload

curl -s http://127.0.0.1:30010/generate -H 'Content-Type: application/json' -d '{
  "input_ids": [3838,374,279,6722,315,9625,30,576,6722,315,9625,374,12095],
  "sampling_params": {"temperature": 1, "max_new_tokens": 0},
  "return_logprob": true,
  "logprob_start_len": 0
}'

结果(temperature=1.0)
image

修改payload的temperature=2.0
结果如下(temperature=2.0)
image

修改payload的temperature=0.5
结果如下(temperature=0.5)
image

@EazyReal

Copy link
Copy Markdown
Contributor Author

Thanks @kkyyxhll for catching the SGLang side — you are right that sampling_params.temperature does not affect input log-probs, so the change in this PR is a no-op. However, pinning it to 1.0 also leaves the teacher at q_1 while the student is trained with p_T, which optimizes KL(p_T || q_1) rather than the matched objective KL(p_T || q_T). The real-temperature fix therefore needs the larger SGLang input-logprob change; I opened draft #2325 with that implementation and regression coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants