Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable sampling when temperature=0 #197

Closed
jppgks opened this issue Jan 19, 2024 · 2 comments · Fixed by #467
Closed

Disable sampling when temperature=0 #197

jppgks opened this issue Jan 19, 2024 · 2 comments · Fixed by #467
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@jppgks
Copy link

jppgks commented Jan 19, 2024

Feature request

As discussed, temperature=0 should be equivalent to disabling sampling

Motivation

As a way to get deterministic results

Your contribution

/

@tgaddair tgaddair added enhancement New feature or request good first issue Good for newcomers labels Jan 19, 2024
@akelch11
Copy link

akelch11 commented Jan 21, 2024

Hi @jppgks ! I'm a new contributor looking to get started with this issue. I had a few small questions about the scope of the changes before I get started.

First, to clarify I'm on the right track, sampling is referring to choosing the next token according to the appropriate logit/probability distribution. This makes me think the changes would go in the NextTokenChooser (and possibly HeterogeneousNextTokenChooser?) classes.

Secondly, would it be best to implement this by changing the setting of flags like sampling (Line 78, server/lorax_server/utils/tokens.py and the do_sample flags in Lines 255-261 (same file) to have logic checking for if temperature == 0 ?

Let me know if this seems like a good starting approach.

@tgaddair
Copy link
Contributor

Hey @akelch11, thanks for looking into this!

I believe you're on the right track. Here are few things off the top of my head we would want to change:

  1. I would change the temperature check in tokens.py to include a check for temperature != 1 or temperature != 0 when setting has_warpers.
  2. Just below line 78, If sampling and temperature == 0 then I would raise a warning saying that sampling will be disabled due to setting temperature == 0, then I would override sampling = False.
  3. Changing the checks for lines 255-261 is a good catch! I agree that would also need to be changed to check x != 1 or x != 0.
  4. The temperature validation check in validation.rs would need to change to temperature < 0.

There might be a few other gotchas, but those should be the main ones. Thanks for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants