Fast non-model CLI commands #1349
Conversation
…t don't need a model
These changes add a little complexity with the lazy and local imports, but
they also greatly improve the CLI's response for --help, list, and where.
Changes:
* Move `import torch` into function bodies that need them
* Use `importlib.metadata.version` to check the torch version rather than
torch.__version__
* Switch from using torch.inference_mode as a decorator to using it as a
context manager.
* I also removed it from convert_hf_checkpoint_to_tune since that does
not use torch at all
* In build_utils, wrap the dtype values in lambdas so they're lazily
fetched.
pytorch#1347
Branch: FasterCli-1347
Signed-off-by: Gabe Goodhart <[email protected]>
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchchat/1349
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 8ae1bc5 with merge base 4a7dab8 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
| raise RuntimeError( | ||
| f"You are using PyTorch {torch.__version__}. At this time, torchchat uses the latest PyTorch technology with high-performance kernels only available in PyTorch nightly until the PyTorch 2.4 release" | ||
| f"You are using PyTorch {torch_version}. At this time, torchchat uses the latest PyTorch technology with high-performance kernels only available in PyTorch nightly until the PyTorch 2.4 release" |
There was a problem hiding this comment.
We might defer that to commands that actually use torch, and then we can just use torch.version ? No point in raising a version issue when all we do is run help or similar non-model cli commands?
Wdyt @Jack-Khuu
There was a problem hiding this comment.
I like having it here, since we can catch it at the very beginning as opposed to having version checking in every file.
Doing it this way will speed up the interface by a lot. Right now --help and --command is super slow. It also bogs down CI.
There was a problem hiding this comment.
I could see it either way, but I think I agree with @byjlw. I don't think the non-running commands are valuable in-and-of-themselves, so I think the argument would be that this is really a property of the tool (there is no torchchat without torch) and doing the lazy imports is just a speed optimization.
If there were standalone commands that didn't need torch (i.e. if people start using download just to fetch models, but then run them with something else), then I could see making this a soft requirement and deferring it to the commands that need it.
There was a problem hiding this comment.
Thanks for adding this @gabe-l-hart!!
It's been on my list for a while
| raise RuntimeError( | ||
| f"You are using PyTorch {torch.__version__}. At this time, torchchat uses the latest PyTorch technology with high-performance kernels only available in PyTorch nightly until the PyTorch 2.4 release" | ||
| f"You are using PyTorch {torch_version}. At this time, torchchat uses the latest PyTorch technology with high-performance kernels only available in PyTorch nightly until the PyTorch 2.4 release" |
Description
Closes: #1347
Import torch lazily in all places used by the CLI that don't need a model
These changes add a little complexity with the lazy and local imports, but they also greatly improve the CLI's response for
--help,list, andwhere.Changes:
import torchinto function bodies that need themimportlib.metadata.versionto check the torch version rather than torch.version