-
Notifications
You must be signed in to change notification settings - Fork 31.7k
Cheap CLI #42447
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
Cheap CLI #42447
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| try: | ||
| is_available, torch_version = _is_package_available("torch", return_version=True) | ||
| parsed_version = version.parse(torch_version) | ||
| if is_available and parsed_version < version.parse("2.2.0"): | ||
| logger.warning_once(f"Disabling PyTorch because PyTorch >= 2.2 is required but found {torch_version}") | ||
| return is_available and version.parse(torch_version) >= version.parse("2.2.0") | ||
| except packaging.version.InvalidVersion: | ||
| return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to revert this; I ran into an edge-case when uninstalling torch where it would still show up with is_available yet would have 'N/A' as a version.
This is a bit out of scope of that PR so happy to revert it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just saw that, isn't that caused by the lru_cache maybe?
| app.command(name="chat", cls=ChatCommand)(Chat) | ||
| app.command()(download) | ||
| app.command()(env) | ||
| app.command()(run) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? If yes, can the run.py be entirely removed since it's not used anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And if removed, would be good to mention it in PR description + migration guide.
Apart from that the PR looks good to me (haven't run it locally though)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Thanks @Wauplin
molbap
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems faster and fixes transformers add-new-model-like on fresh envs, thanks!
|
|
||
| def stream_response(streamer, _request_id): | ||
| # Temporary hack for GPTOS 2: filter out the CoT tokens. Full solution here implies defining new output | ||
| # Temporary hack for GPT-OSS 2: filter out the CoT tokens. Full solution here implies defining new output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # Temporary hack for GPT-OSS 2: filter out the CoT tokens. Full solution here implies defining new output | |
| # Temporary hack for GPT-OSS: filter out the CoT tokens. Full solution here implies defining new output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fix 2 out of 3 for the GPT OSS family of models :)
| try: | ||
| is_available, torch_version = _is_package_available("torch", return_version=True) | ||
| parsed_version = version.parse(torch_version) | ||
| if is_available and parsed_version < version.parse("2.2.0"): | ||
| logger.warning_once(f"Disabling PyTorch because PyTorch >= 2.2 is required but found {torch_version}") | ||
| return is_available and version.parse(torch_version) >= version.parse("2.2.0") | ||
| except packaging.version.InvalidVersion: | ||
| return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just saw that, isn't that caused by the lru_cache maybe?
Wauplin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Co-authored-by: Lucain <[email protected]>
Better guard the CLI commands, so that
transformers add-new-model-likeworks without the serving dependencies (bug) and so thattransformers chatdoes not importtorchunnecessarily (improvement)The
transformers run(previouslytransformers-cli run) is an artefact of the past, was not documented nor tested,and isn't part of any public documentation. Removing it for now and writing about it in the migration guide.