search

zeno-ml / zeno-build

Build, evaluate, understand, and fix LLM-based apps
MIT License
482 stars 33 forks source link

`max_new_tokens` instead of `max_tokens` #172

Closed zhaochenyang20 closed 1 year ago

zhaochenyang20 commented 1 year ago

https://github.com/zeno-ml/zeno-build/blob/31c4cbe5e7162d169ca04286a5ea84ce138fc162/zeno_build/models/chat_generate.py#L47

I think this parameter should be changed to max_new_tokens instead of max_tokens. In HuggingFace model.generate, the max_length means the input encoding together with the output decoding. And max_new_tokens means the max length of output decoding.

The actual meaning of max_tokens in zeno-bulid is the max length of generated tokens. So to be consistent with casual usage, max_new_tokens may be a better parameter name. 🤔

zhaochenyang20 commented 1 year ago

This seems like a problem created by OpenAI themselves.

https://platform.openai.com/docs/api-reference/completions#completions/create-max_tokens

The parameter name max_token defined by OpenAI for "the max tokens to generate" is confusing itself. 😂

neubig commented 1 year ago

I agree that this is somewhat confusing, but zeno_build follows the openai parameter naming conventions because these are now somewhat standard. Hopefully with the docstring this should not be too confusing.

I'll close this, but if there is a way the clarity could be further improved without changing the parameter names away from OpenAI conventions then I'd be happy to review a PR.