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

Add torch.no_grad, fix greedy_until bug #161

Merged
merged 3 commits into from
Apr 19, 2024
Merged

Add torch.no_grad, fix greedy_until bug #161

merged 3 commits into from
Apr 19, 2024

Conversation

OyvindTafjord
Copy link
Contributor

This was an unnecessary oversight spotted by @jjyang77, that we're not wrapping model calls with torch.no_grad(). Added here for the language_model.py model type. In a couple of experiments it doesn't seem to speed evaluation up by much, but does improve memory usage (was able to run Llama-3-8B on single GPU rather than needing two).

Also fixed a bug in the greedy_until method which causes crashes for some models (like Mistral) when primary_until is set as None, as noted by @dmh43.

primary_until = None
for tokenized_until in tokenizer(untils)["input_ids"]:
primary_until = tokenizer.eos_token_id
for tokenized_until in tokenizer(untils, add_special_tokens=False)["input_ids"]:
Copy link

Choose a reason for hiding this comment

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

what's this about?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The add_special_tokeens=False prevents the tokenizer from adding a <bos> token of sorts at the start of the string, which some tokenizers do, and would mess up the test that there's only one token (unfortunately, other tokenizers, like Llama/Mistral, also adds a "space" token when you tokenize "\n", so this is still not effective for those - will be improved in next iteration of model handling, more centrally)

@OyvindTafjord OyvindTafjord merged commit c3eb82e into main Apr 19, 2024
10 of 17 checks passed
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