Conversation
There was a problem hiding this comment.
Code Review
This pull request fixes the configuration for tau_pos and tau_neg to align with the project's YAML style guide by separating them. My review includes a suggestion to improve the new comment for tau_neg for better clarity and consistency, and also points out a related inconsistency in the comment for tau_pos that was introduced by this change.
| # negative tau for smoothing function in SAPO | ||
| tau_neg: 1.05 |
There was a problem hiding this comment.
Thanks for adding the comment and blank line to follow the file's style guide. The new comment for tau_neg is a bit sparse. It would be better to include the paper reference to be consistent with other parameters.
Also, a heads-up: the comment for tau_pos on lines 44-45 is now slightly misleading as it still mentions 'Positive and negative tau'. It would be great if you could update that as well to only refer to 'Positive tau' to avoid confusion.
Here's a suggestion for tau_neg:
# Negative tau for smoothing function in SAPO (https://arxiv.org/pdf/2511.20347)
tau_neg: 1.05
What does this PR do?
Checklist Before Starting
[{modules}] {type}: {description}(This will be checked by the CI){modules}includefsdp,megatron,sglang,vllm,rollout,trainer,ci,training_utils,recipe,hardware,deployment,ray,worker,single_controller,misc,perf,model,algo,env,tool,ckpt,doc,data,cfg,reward,like[megatron, fsdp, doc]{type}is infeat,fix,refactor,chore,test[BREAKING]to the beginning of the title.[BREAKING][fsdp, megatron] feat: dynamic batchingTest
API and Usage Example
# Add code snippet or script demonstrating how to use thisDesign & Code Changes
Checklist Before Submitting
Important
Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review.
pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=alwaysci-requestchannel in theverlSlack workspace. (If not accessible, please try the Feishu group (飞书群).)