-
Notifications
You must be signed in to change notification settings - Fork 49
fix-43: Refactoring import statements for faster startup time #50
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
Conversation
* Also moving type hinting to use `TYPE_CHECK` to avoid importing torch at startup.
jakelorocco
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.
lgtm; that's a really good improvement! do we also want to fix our pyproject packages as a part of this? ie should we thin out the packages that we download now depending on the user request? Maybe by default we install ollama and openai, but force users to specify huggingface?
I could move them to optional dependencies, so the user would have to do something like uv pip install "mellea[hf]" |
Maybe. The I think the more important reason for dependency groups is the numpy / transformers / etc. versioning woes we're always risking by virtue of having so many "low-level" libraries imported. I suggest we go ahead with this merge because it's a massive QoL improvement, and open an issue for dependency group stuff. We should also let the alora and watsonx sdk folks know that they should slim up their imports. I think part of @avinash2692 's RCA was an observation that a lot of the stuff they're importing isn't necessary. |
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.
🚀 46s -> 2s 🚀
Thanks for the thorough RCA on this.
@avinash2692 please feel free to open 2 new issues for the stuff above and then squash-merge this.
* Moves watsonx imports into a conditional * moves transformers imports into a conditional * moves alora stuff into a condition * Also moving type hinting to use `TYPE_CHECK` to avoid importing torch at startup.
Summary
Initial startup time for an import statement was taking too long for a fresh install as shown in #43 and also here:
After the refactor, we have a faster startup time:
Related Issue
Changes
TYPE_CHECKto avoid importing torch at startup.