Avoid emitting NaN / Inf logprobs out of inference

[tdene]

• I, the PR author, have personally reviewed every line of this PR.

What does this PR do?

There were several issues resolved in this PR.

• NaN logprobs were causing crashes when emitted through the API layer due to JSON parsing.
• Megatron Inference was treating top_p = 0.0 as greedy/default; NeMo RL was using top_p = 1.0.
• Some calculations were not being clamped, and were being sent to -inf.
• For processed logprobs, we were using the FlashInfer topp/topk filter kernel when not appropriate.

Issue tracking

For PRs from open-source community contributors:

• New features: a linked issue is required. Please open a feature request and reference it here before submitting the PR.
• Small updates (bug fixes, minor improvements): a linked issue is recommended and will accelerate the PR review process.

Linked issue:

Contribution process

Pre-checks

• I have added relevant unit tests
• I have added relevant functional tests
• I have added proper typing to my code Typing guidelines
• I have added relevant documentation
• I have run the autoformatter.sh on my PR

Code review

Feel free to message or comment @NVIDIA/mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!

All PRs start as draft. If you open a non-draft PR, it will be automatically converted to draft.

Step 1: Mark PR as "Ready for Review"

1. When your PR is ready, click Ready for Review.
2. An oncall reviewer is auto-assigned and expert reviewers are notified based on your changes.
   • Some PRs may jump straight to step 2. This is determined by .github/CODEOWNERS.

⚠️ Only mark as ready once merge-conflicts are resolved and the CI is passing.
Final Review might get declined if these requirements are not fulfilled.

Step 2: Final Review

For PRs that change megatron/core, once all expert reviewers have approved, the Final Review label is applied automatically and final reviewers are assigned.

For PRs outside megatron/core, this step is skipped.

Step 3: Approved

Once all required reviewers have approved, the Approved label is applied automatically.

Merge

Any member of mcore-engineers will be able to merge your PR.

[github-actions]

This PR has been automatically converted to draft because all PRs must start as drafts.

When you are ready for review, click Ready for Review to begin the review process. This will:

1. Add the oncall reviewer (optional reviewer)
2. Add required review teams based on your changes

See the contribution guide for more details.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[santhnm2]

Can we add specific links to vLLM code to document parity for the scalar constants? (MIN_SAMPLING_TEMPERATURE and JSON_SAFE_LOGPROB_FLOOR)

[tdene]

Can we add specific links to vLLM code to document parity for the scalar constants? (MIN_SAMPLING_TEMPERATURE and JSON_SAFE_LOGPROB_FLOOR)

Added the link for JSON_SAFE_LOGPROB_FLOOR.

MIN_SAMPLING_TEMPERATURE is handled different by vllm for processed_logprobs.

[santhnm2]

MIN_SAMPLING_TEMPERATURE is handled different by vllm for processed_logprobs.

Would it make sense to align these or is there no need?

[tdene]

MIN_SAMPLING_TEMPERATURE is handled different by vllm for processed_logprobs.

Would it make sense to align these or is there no need?

There is no need for two reasons:

• No one does low-temperature RL training, because that's counter-intuitive, and processed_logprobs is only used for RL training.
• vllm's solution is to change the temperature to 1.0 in places where it is smaller than the MIN, see here. This PR's choice to clamp it to MIN is more accurate than changing it to 1.0. Again, it doesn't matter though, and that's probably why vllm chooses to change it to 1.0.

[Phlip79]

Why not add this to SamplingParams?

[tdene]

If you mean the actual clamp, that is why not just have any tiny temperature value get automatically clamped in a __post_init__ whenever the request gets made, here are the reasons:

• temperature=0 means something special: greedy decode. We have no separate greedy flag (we probably should... follow-up PR). The OpenAI endpoints assume this "greedy is temperature 0" as they build the requests, and it would be strange for the sampling parameters to be modified away from it. If the first turn's response has different sampling parameters from the first turn's reply, external callers like Gym could get confused and call subsequent turns with non-greedy temperatures.
• There were more reasons, but now that I'm responding to your comment and thinking critically, all other reasons are minor.

So it's just that we lack an is_greedy flag, and instead we rely on temperature=0.
I would want to address this in a follow-up PR because is_greedy is a design choice, whereas this is just a quick bug-fix to enable NeMo RL.

But you're right. I've put this on my radar now for a follow-up clean-up PR.

[Phlip79]

Thanks for the explanation

[nemo-automation-bot]

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/33684159614

[nemo-automation-bot]

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/33686162520

[nemo-automation-bot]

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/33711118299