[WIP] FEAT Decorator to purge accelerate env vars #3252
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In some circumstances, calling certain classes or functions can result in accelerate env vars being set and not being cleaned up afterwards. As an example, when calling: TrainingArguments(fp16=True, ...) The following env var will be set: ACCELERATE_MIXED_PRECISION=fp16 This can affect subsequent code, since the env var takes precedence over TrainingArguments(fp16=False). This is especially relevant for unit testing, where we want to avoid the individual tests to have side effects on one another. Decorate the unit test function or whole class with this decorator to ensure that after each test, the env vars are cleaned up. This works for both unittest.TestCase and normal classes (pytest); it also works when decorating the parent class. In its current state, this PR adds the new decorator and tests it, but the decorator is not yet applied to potentially problematic functions or classes.
|
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. |
muellerzr
approved these changes
Nov 22, 2024
5 tasks
|
@muellerzr how much will this still be needed if huggingface/transformers#34886 is merged? |
|
None, but it's not bad to over protect imo |
SunMarc
approved these changes
Nov 25, 2024
BenjaminBossan
deleted the
feat-decorator-to-purge-modified-accelerate-env-vars
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
In some circumstances, calling certain classes or functions can result in accelerate env vars being set and not being cleaned up afterwards. As an example, when calling:
TrainingArguments(fp16=True, ...)The following env var will be set:
ACCELERATE_MIXED_PRECISION=fp16This can affect subsequent code, since the env var takes precedence over
TrainingArguments(fp16=False). This is especially relevant for unit testing, where we want to avoid the individual tests to have side effects on one another. Decorate the unit test function or whole class with this decorator to ensure that after each test, the env vars are cleaned up. This works for bothunittest.TestCaseand normal classes (pytest) and standalone functions; it also works when decorating the parent class and in conjunction with other decorators.Notes
In its current state, this PR adds the new decorator and tests it, but the decorator is not yet applied to potentially problematic functions or classes.
Personally, I'm not super happy with the implementation, as it turned out to be more complicated than I initially thought (mainly because of the subclassing). I tested this decorator on the initial test failure in PEFT and it solved it. However, it requires to decorate each and every class/function that could potentially have side effects. It is thus easy to miss something.
Before submitting
Pull Request section?
Was this discussed/approved via a Github issue or the forum? Please add a linkInternal discussion on Slackto it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.