Skip to content

[cleanup] Remove prefill logprobs recomputation path - #1192

Open
maocheng23 wants to merge 1 commit into
feat/true_on_policy_qwen_moe_spppfrom
feat/true_on_policy_qwen_moe_sppp_prefill_removal
Open

[cleanup] Remove prefill logprobs recomputation path#1192
maocheng23 wants to merge 1 commit into
feat/true_on_policy_qwen_moe_spppfrom
feat/true_on_policy_qwen_moe_sppp_prefill_removal

Conversation

@maocheng23

Copy link
Copy Markdown
Contributor

Stacked on top of #1088.

Summary

Removes the unused prefill-logprob recomputation path from Miles.

This was split out from the Qwen3 MoE SP+PP true-on-policy support PR so the functional SP+PP support and the controversial cleanup can be reviewed independently.

Main Changes

  • Remove the --recompute-logprobs-via-prefill CLI flag and validation.
  • Remove the rollout hooks that called SGLang prefill scoring after generation.
  • Delete the SGLang prefill scoring helper and its dedicated fast tests.
  • Remove test assertions that only existed to verify the flag was absent from true-on-policy launch plans.
  • Keep --sglang-enable-prefill-only-deterministic-inference available for any future model that genuinely needs prefill-only deterministic inference.

Validation

Local split checks:

The prior focused pytest run for the affected true-on-policy/script tests hung locally without producing failures, so I stopped it and did not use it as validation evidence.

Remove the unused --recompute-logprobs-via-prefill flag, rollout hooks, SGLang prefill scoring helper, and tests. The SGLang-level --sglang-enable-prefill-only-deterministic-inference flag remains available for any future model that needs prefill-only deterministic inference.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the --recompute-logprobs-via-prefill command-line argument and all associated logic, including the deletion of the utility module miles/rollout/generate_utils/prefill_logprobs.py and its corresponding unit tests. References to this functionality have been cleaned up across argument validation, rollout engines, and configuration tests. There are no review comments, so I have no feedback to provide.

@Zhichenzzz Zhichenzzz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks.

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.

2 participants