-
Notifications
You must be signed in to change notification settings - Fork 122
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
Modernbert install from git #508
Conversation
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.
PR Summary
This PR adds support for nomic-ai/nomic-embed-text-v1.5 model by updating dependencies and installing transformers from a specific git commit across multiple Dockerfiles.
- Adds temporary direct installation of transformers from GitHub commit 42865860ec in all Dockerfiles, bypassing Poetry's dependency management
- Adds einops as optional dependency in
pyproject.toml
for nomic-ai/nomic-embed-text-v1.5 support - Adds new
build-nvidia
target in Makefile but doesn't include it inbuild-all-docker
target - Version bump from 0.0.73 to 0.0.74 across infinity_client and infinity_emb packages
- Inefficient Docker layer caching due to git installation and cache cleanup in same layer
9 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile
# TODO: remove this line | ||
RUN apt-get install --no-install-recommends -y git && poetry run python -m pip install git+https://github.com/huggingface/transformers.git@42865860ec6dc135972d9555753cb7ee17f51fb4 && rm -rf ~/.cache/ /tmp/* |
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.
logic: Installing transformers directly from git with a specific commit hash bypasses poetry dependency management and could cause version conflicts. Consider adding this as a proper dependency in pyproject.toml instead.
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #508 +/- ##
=======================================
Coverage 80.00% 80.00%
=======================================
Files 43 43
Lines 3471 3471
=======================================
Hits 2777 2777
Misses 694 694 ☔ View full report in Codecov by Sentry. |
Related Issue
Checklist
Additional Notes
Add any other context about the PR here.