-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[BNB] Fix bnb dockerfile for latest version #1291
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.
Makes a lot of sense to me, thanks a lot for fixing! 🚀
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. |
Co-authored-by: Younes Belkada <49240599+younesbelkada@users.noreply.github.com>
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, thanks. Only a quick comment.
@@ -82,6 +82,13 @@ jobs: | |||
pip install -e . --no-deps | |||
pip install pytest-reportlog pytest-cov parameterized datasets scipy einops | |||
mkdir transformers-clone && git clone https://github.com/huggingface/transformers.git transformers-clone | |||
if [ "${{ matrix.docker-image-name }}" == "huggingface/peft-gpu-bnb-latest:latest" ]; then | |||
cd transformers-clone | |||
transformers_version=$(pip show transformers | grep '^Version:' | cut -d ' ' -f2 | sed 's/\.dev0//') |
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.
This assumes that there is never going to be a .dev1
, right? I wonder if something along the lines of git checkout $(git tag --list --sort=-v:refname | head -n 1)
would be more robust, but there might be a reason to do it this way that's eluding me, so no blocker.
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.
Normally, we should not even get a .dev0
since we are just installing the latest version on pip. As for the tag, I thought about getting the latest tag but I wasn't certain whether the tags had additional uses beyond versioning. Let's see if this holds and, if we came across issues, we will switch to your command.
What does this PR do ?
This PR fixes the nightly docker file for the tests on the latest transformers. Right now, we are installing the latest transformers but we are launching the tests that are on the main branch. We need to checkout the corresponding branch (e.g. v4.36.3 if it is the version of transformers that is installed).