update codecarbon #35243

nhamanasu · 2024-12-12T16:19:20Z

What does this PR do?

Related to #390 Add Python 3.11 support mlco2/codecarbon#391, I found we need to update codecarbon to run test_trainer.py properly in Python 3.11 or above. In current main branch, "codecarbon==1.2.0" was set, so I rewrote this to "codecarbon>=2.8.1". It properly worked.
test_trainer.py generates some output directories which could be staged through git. I added them in .gitignore

Before submitting

This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
Did you read the contributor guideline,
Pull Request section?
Was this discussed/approved via a Github issue or the forum? Please add a link
to it if that's the case.
Did you make sure to update the documentation with your changes? Here are the
documentation guidelines, and
here are tips on formatting docstrings.
Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

trainer: @muellerzr and @SunMarc

nhamanasu · 2024-12-12T17:34:08Z

The failure of tests_generate will be resolved in this PR: skip Fuyu from test_generate #35246
The failure of tests_torch will be resolved in this PR: set supports_gradient_checkpointing = False in SwitchTransformer #35249

Rocketknight1 · 2024-12-12T18:01:57Z

cc @ydshieh @muellerzr @SunMarc

ydshieh

LGTM (don't know the reason why it was pinned to 1.2.0 previously)

CircleCI jobs don't install codecarbon so no need to build and check before merge. We can see next dailiy CI next week.

Thanks

ydshieh · 2024-12-13T13:54:00Z

will let at least one of @muellerzr or @SunMarc to approve too.

SunMarc

LGTM ! Left a comment

SunMarc · 2024-12-13T15:10:28Z

.gitignore

+
+# folders generated in test_trainer.py
+/test
+/regression
+/None
+/examples/checkpoint-*


Do you know which test is causing this. ? We try to put everything that are saved in temp folder

Thank you for your comment! Let me see...

I've searched the lines maybe related to:

".test"

transformers/tests/trainer/test_trainer.py

Line 1193 in bc6ae0d

args = TrainingArguments("./test", report_to="none")

transformers/tests/trainer/test_trainer.py

Line 1454 in bc6ae0d

"./test",

transformers/tests/trainer/test_trainer.py

Line 1476 in bc6ae0d

"./test",

transformers/tests/trainer/test_trainer.py

Line 1508 in bc6ae0d

"./test", learning_rate=1e9, logging_steps=5, logging_nan_inf_filter=False, report_to="none"

transformers/tests/trainer/test_trainer.py

Line 1516 in bc6ae0d

"./test", learning_rate=1e9, logging_steps=5, logging_nan_inf_filter=True, report_to="none"

transformers/tests/trainer/test_trainer.py

Line 1579 in bc6ae0d

args = TrainingArguments("./test", report_to="none", dataloader_persistent_workers=False)

transformers/tests/trainer/test_trainer.py

Line 1623 in bc6ae0d

"./test",

transformers/tests/trainer/test_trainer.py

Line 1682 in bc6ae0d

"./test",

"./regression":

transformers/tests/trainer/test_trainer.py

Line 555 in bc6ae0d

output_dir = kwargs.pop("output_dir", "./regression")

transformers/tests/trainer/test_trainer.py

Line 738 in bc6ae0d

args = TrainingArguments("./regression", learning_rate=0.1, report_to="none")

transformers/tests/trainer/test_trainer.py

Line 990 in bc6ae0d

args = TrainingArguments("./regression", report_to="none")

transformers/tests/trainer/test_trainer.py

Line 1009 in bc6ae0d

"./regression",

"./examples"

transformers/tests/trainer/test_trainer.py

Line 3188 in bc6ae0d

training_args = TrainingArguments(output_dir="./examples", use_cpu=True, report_to="none")

transformers/tests/trainer/test_trainer.py

Line 3206 in bc6ae0d

output_dir="./examples",

transformers/tests/trainer/test_trainer.py

Line 3240 in bc6ae0d

args = RegressionTrainingArguments(output_dir="./examples", max_steps=4)

transformers/tests/trainer/test_trainer.py

Line 3255 in bc6ae0d

args = RegressionTrainingArguments(output_dir="./examples")

transformers/tests/trainer/test_trainer.py

Line 3282 in bc6ae0d

args = RegressionTrainingArguments(output_dir="./examples")

"./None"

I couldn't find any directly specified part, but I've seen "./None" folder is made every time I run test_trainer. I suspect that there are some lines which don't pass output_dir to TrainingArguments, or the variables passed to TrainingArguments are occasionally None.

I'm not sure all of these are the main cause, but when I ran test_trainer.py, it surely made the folders I wrote in .gitignore.

I found 117 lines which use with tempfile.TemporaryDirectory() as tmp_dir: and pass tmp_dir to TrainingArguments, so most of the test experiments properly utilizes temp dir, and the lines I picked above seem to be irregular.

I found another non-tmp-dir specified in test_trainer.py

"./generation"

transformers/tests/trainer/test_trainer.py

Line 786 in bc6ae0d

"./generation",

transformers/tests/trainer/test_trainer.py

Line 801 in bc6ae0d

"./generation",

transformers/tests/trainer/test_trainer.py

Line 883 in bc6ae0d

"./generation",

transformers/tests/trainer/test_trainer.py

Line 898 in bc6ae0d

"./generation",

nhamanasu · 2024-12-13T16:11:03Z

I replaced "./test", "./examples", "./generation", and "./regression" with tmp_dir.

The only part left we need to find is the causes of producing "./None" dir, I think.

nhamanasu · 2024-12-13T16:14:52Z

Maybe I did something unnecessary...

New failure FAILED tests/trainer/test_trainer.py::TrainerIntegrationWithHubTester::test_push_to_hub - AssertionError: '__DUMMY_TRANSFORMERS_USER__/tmpnlv7o37x' != '__DUMMY_TRANSFORMERS_USER__/tmp_mcwovml' may be related to the part I replaced.

Testing push_to_hub requires the checkpoint dir which is not temporary I guess?

I'm ready to revert the last commit and just leave .gitignore intact.

I want your review @SunMarc .

ydshieh · 2024-12-13T16:18:58Z

Hi, the last commit of changing won't work as the tmp_dir is created within the scope of

    with tempfile.TemporaryDirectory() as tmp_dir:

outside the with block, the directory is removed automatically and the sequential code blocks won't be able to access it.

nhamanasu · 2024-12-13T16:21:59Z

It's my trivial mistake. Thank you for pointing it out.
I'll fix them.

ydshieh · 2024-12-13T16:30:16Z

I guess it's good to just revert that commit , keep this PR as doing what it intends to do. For the temp. directory, we can address them in another PR (if eventually we really wish )

This reverts commit 310a6d9.

nhamanasu · 2024-12-13T16:51:44Z

Thank you for your comments! I reverted the last commit, and gonna make another PR to fix tmp_dir issue.

→ I split the PR as @ydshieh suggested: #35266

ydshieh · 2024-12-13T18:12:10Z

.gitignore

+
+# folders generated in test_trainer.py
+/test
+/regression
+/None
+/examples/checkpoint-*


let's also revert this one in this PR :-)

Thanks, I reverted it!

.gitignore

nhamanasu · 2024-12-15T13:31:14Z

@ydshieh cc. @SunMarc
Thank you for your last commit to fix .gitignore!

I wonder what kind of step is left to finally merge this PR.
If there's something l still have to do, I'd be happy if you could let me know 🙇

ydshieh · 2024-12-16T09:26:18Z

@ArthurZucker We need an approval from you :-) 🙏

ArthurZucker

thanks for the update! 🤗

update codecarbon

d911458

nhamanasu mentioned this pull request Dec 12, 2024

Added RAdamScheduleFree support | updated codecarbon to run test_trainer.py properly #35214

Closed

5 tasks

ydshieh approved these changes Dec 13, 2024

View reviewed changes

SunMarc approved these changes Dec 13, 2024

View reviewed changes

replace directly-specified-test-dirs with tmp_dir

310a6d9

Revert "replace directly-specified-test-dirs with tmp_dir"

f1d4f56

This reverts commit 310a6d9.

nhamanasu mentioned this pull request Dec 13, 2024

Fix all output_dir in test_trainer.py to use tmp_dir #35266

Merged

5 tasks

ydshieh reviewed Dec 13, 2024

View reviewed changes

revert the change of .gitignore

cc1a01e

ydshieh reviewed Dec 13, 2024

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

Update .gitignore

b0c81fe

ydshieh requested a review from ArthurZucker December 16, 2024 09:25

ArthurZucker approved these changes Dec 20, 2024

View reviewed changes

ArthurZucker merged commit 34ad1bd into huggingface:main Dec 20, 2024
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update codecarbon #35243

update codecarbon #35243

nhamanasu commented Dec 12, 2024

nhamanasu commented Dec 12, 2024 •

edited

Loading

Rocketknight1 commented Dec 12, 2024

ydshieh left a comment

ydshieh commented Dec 13, 2024

SunMarc left a comment

SunMarc Dec 13, 2024

nhamanasu Dec 13, 2024 •

edited

Loading

nhamanasu Dec 13, 2024

nhamanasu Dec 13, 2024 •

edited

Loading

nhamanasu Dec 13, 2024

nhamanasu commented Dec 13, 2024

nhamanasu commented Dec 13, 2024 •

edited

Loading

ydshieh commented Dec 13, 2024

nhamanasu commented Dec 13, 2024

ydshieh commented Dec 13, 2024

nhamanasu commented Dec 13, 2024 •

edited

Loading

ydshieh Dec 13, 2024

nhamanasu Dec 13, 2024

nhamanasu commented Dec 15, 2024

ydshieh commented Dec 16, 2024

ArthurZucker left a comment

update codecarbon #35243

update codecarbon #35243

Conversation

nhamanasu commented Dec 12, 2024

What does this PR do?

Before submitting

Who can review?

nhamanasu commented Dec 12, 2024 • edited Loading

Rocketknight1 commented Dec 12, 2024

ydshieh left a comment

Choose a reason for hiding this comment

ydshieh commented Dec 13, 2024

SunMarc left a comment

Choose a reason for hiding this comment

SunMarc Dec 13, 2024

Choose a reason for hiding this comment

nhamanasu Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

".test"

"./regression":

"./examples"

"./None"

nhamanasu Dec 13, 2024

Choose a reason for hiding this comment

nhamanasu Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

nhamanasu Dec 13, 2024

Choose a reason for hiding this comment

"./generation"

nhamanasu commented Dec 13, 2024

nhamanasu commented Dec 13, 2024 • edited Loading

ydshieh commented Dec 13, 2024

nhamanasu commented Dec 13, 2024

ydshieh commented Dec 13, 2024

nhamanasu commented Dec 13, 2024 • edited Loading

ydshieh Dec 13, 2024

Choose a reason for hiding this comment

nhamanasu Dec 13, 2024

Choose a reason for hiding this comment

nhamanasu commented Dec 15, 2024

ydshieh commented Dec 16, 2024

ArthurZucker left a comment

Choose a reason for hiding this comment

nhamanasu commented Dec 12, 2024 •

edited

Loading

nhamanasu Dec 13, 2024 •

edited

Loading

nhamanasu Dec 13, 2024 •

edited

Loading

nhamanasu commented Dec 13, 2024 •

edited

Loading

nhamanasu commented Dec 13, 2024 •

edited

Loading