-
Notifications
You must be signed in to change notification settings - Fork 390
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
Accelerate bug in specific versions #1007
Conversation
In versions 0.20.* and 0.21.0 (latest), there was/is a bug in unwrap_model with keep_fp32_wrapper=False. This results in forward not working after unwrapping the model, which can affect skorch users who use AccelerateMixin. The bug has been fixed in this PR: huggingface/accelerate#1838 However, there was no release yet. Therefore, I exclude the specific accelerate versions with that bug. Using a version <0.20 or a later release should work. To catch this type of issue sooner, I added a test that will trigger the bug.
Argh:
So the transformers version that's installed fails with earlier accelerate versions. We could now start excluding transformers versions with this error message, but I think it's getting too much. My proposal would be to leave this PR open for now and wait for the next accelerate release with the bugfix, then require that accelerate version. Update: The new accelerate v0.22 with the bugfix was released today, so tests are green now. |
requirements-dev.txt
Outdated
@@ -1,4 +1,4 @@ | |||
accelerate>=0.11.0 | |||
accelerate>=0.11.0,!=0.20.*,!=0.21.0 |
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.
At this point, what do you think of going with >=0.22
?
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.
True, it's best to be on the latest version anyway. And since accelerate is not a strict dependency, users can still use skorch with different accelerate versions if they want.
In versions 0.20.* and 0.21.0 (latest), there was/is a bug in
unwrap_model
when called withkeep_fp32_wrapper=False
. This results inforward
not working after unwrapping the model, which can affect skorch users who useAccelerateMixin
.The bug has been fixed in this PR:
huggingface/accelerate#1838
However, there was no release yet. Therefore, I exclude the specific accelerate versions with that bug. Using a version <0.20 or a future release should work.
To catch this type of issue sooner, I added a test that will trigger the bug.