Skip to content

fix: apply temperature scaling before top-p filtering in logits_to_probs#1150

Open
Mr-Neutr0n wants to merge 1 commit intofishaudio:mainfrom
Mr-Neutr0n:fix/logits-to-probs-temperature-order
Open

fix: apply temperature scaling before top-p filtering in logits_to_probs#1150
Mr-Neutr0n wants to merge 1 commit intofishaudio:mainfrom
Mr-Neutr0n:fix/logits-to-probs-temperature-order

Conversation

@Mr-Neutr0n
Copy link

Summary

  • In logits_to_probs(), temperature scaling was applied after top-p filtering, causing the cumulative probability threshold to operate on the raw (unscaled) logit distribution rather than the temperature-adjusted one
  • Moved the temperature division (logits / temperature) to before the top-p sorting and cumulative sum, so that top_p filters based on the correct probability distribution
  • With temperature < 1.0, the previous order made top-p effectively more restrictive than intended; with temperature > 1.0, it was less restrictive

Details

The standard sampling pipeline order is:

  1. Repetition penalty
  2. Temperature scaling
  3. Top-p (nucleus) filtering
  4. Final softmax → sample

Previously, the code ran top-p filtering on the raw logits and only applied temperature afterwards. Since top-p computes softmax → cumsum to decide which tokens to keep, it needs to see the temperature-adjusted logits to make the right filtering decisions. Otherwise the top_p parameter doesn't behave as users expect when temperature != 1.0.

Test plan

  • Verify that with temperature=1.0, behavior is unchanged (temperature scaling is a no-op)
  • Verify that with temperature < 1.0, top-p retains fewer tokens than before (correctly more peaked distribution)
  • Verify that with temperature > 1.0, top-p retains more tokens than before (correctly flatter distribution)

@github-actions
Copy link
Contributor

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Mar 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant