Remove old code for PyTorch, Accelerator and tokenizers #37234

cyyever · 2025-04-03T03:57:47Z

What does this PR do?

Remove outdated conditions and comments for PyTorch and Accelerator.

Specifically, some code for PyTorch < 2.1 has been found and removed. As a result, the functions is_torch_bf16_cpu_available,is_torch_fx_available, is_torchdynamo_available and is_torch_compile_available are now equivalent to is_torch_available. Some tests are further simplified using this fact.

There is one change regarding to old Accelerator code.
And old tokenizers version check is removed.

github-actions · 2025-04-03T03:58:00Z

Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. The CI will be paused while the PR is in draft mode. When it is ready for review, please click the Ready for review button (at the bottom of the PR page). This will assign reviewers and trigger CI.

Rocketknight1

LGTM, with one comment: Are we raising an error anywhere when users use old versions of Torch? I think it's important that they get an error telling them what the problem is, or else we'll get lots of issues complaining about failing compilations and missing functions from people who don't understand what went wrong

cyyever · 2025-04-04T01:36:57Z

Added a warning message about unsupported PT versions.

Rocketknight1 · 2025-04-04T13:15:31Z

Hi @cyyever, sorry about this, my fault! We actually have a separate PR to bump the minimum torch version to 2.1 here: #37277

As a result, you can skip the check - sorry for suggesting it!

cyyever · 2025-04-04T13:18:44Z

@Rocketknight1 I will rebase after it's merged to see whether there are more to delete..

cyyever · 2025-04-08T16:11:13Z

@Rocketknight1 Lots of new commits since last review.

cyyever · 2025-04-08T16:12:36Z

@ydshieh Follow-up of your clean-up.

Signed-off-by: cyy <[email protected]>

ydshieh · 2025-04-08T16:22:51Z

Thanks! Will check. Could you refine the PR title and description 🙏 ? Seems this PR focus on is_torch_fx_available? A more detailed information regarding what a PR is doing is much appreciated.

cyyever · 2025-04-08T16:24:34Z

@ydshieh Ignore the branch name...

ydshieh

Having a few nit questions in the first round 🙏

src/transformers/utils/import_utils.py

src/transformers/models/code_llama/tokenization_code_llama_fast.py

src/transformers/trainer.py

Signed-off-by: cyy <[email protected]>

This reverts commit 0e75643.

src/transformers/utils/import_utils.py

tests/trainer/test_trainer.py

ydshieh · 2025-04-10T13:23:35Z

tests/trainer/test_trainer_fsdp.py

 class TestFSDPTrainer(TestCasePlus):
    @require_torch_multi_accelerator
    @require_accelerate
-    @require_fsdp


Could you explain this part?

require_fsdp = require_torch after this PR.

I might miss some details, but can't find the reason why this is the case.

(forgot to unresolved so you didn't see it)
this is my last question and we are good!

Because FSDP_MIN_VERSION = "1.12.0". Actually for PT2.0 fsdp has been supported for a long time. is_fsdp_available should always return True once PT is available. You can check its definition in src/transformers/utils/import_utils.py. I would like to remove is_fsdp_available but I can't since it is an exported symbol.

Understand, thank you. I was expecting there will be a change to is_fsdp_available just you did for other places, that is why I was a bit confused here. Do you think to change to

def is_fsdp_available(min_version: str = FSDP_MIN_VERSION): return is_torch_available()

Either way, I will merge, just wondering.

@ydshieh It's better to respect min_version in case someone passes "2.6" ...

tests/utils/test_modeling_utils.py

ydshieh · 2025-04-10T13:25:33Z

Almost ready 🚀 to go 👍

Signed-off-by: cyy <[email protected]>

cyyever · 2025-04-10T13:56:51Z

@ydshieh done

Signed-off-by: cyy <[email protected]>

ydshieh · 2025-04-10T18:54:02Z

Merge! Thank you for the contribution and iteration 🤗

…37234) * Remove unneeded library version checks Signed-off-by: cyy <[email protected]> * Remove PyTorch condition Signed-off-by: cyy <[email protected]> * Remove PyTorch condition Signed-off-by: cyy <[email protected]> * Fix ROCm get_device_capability Signed-off-by: cyy <[email protected]> * Revert "Fix ROCm get_device_capability" This reverts commit 0e75643. * Remove unnecessary check Signed-off-by: cyy <[email protected]> * Revert changes Signed-off-by: cyy <[email protected]> --------- Signed-off-by: cyy <[email protected]>

github-actions bot marked this pull request as draft April 3, 2025 03:57

cyyever force-pushed the is_torch_fx_available branch from 142363d to 93b4c9a Compare April 3, 2025 04:05

cyyever marked this pull request as ready for review April 3, 2025 04:09

github-actions bot requested review from ArthurZucker and Rocketknight1 April 3, 2025 04:09

Rocketknight1 approved these changes Apr 3, 2025

View reviewed changes

cyyever force-pushed the is_torch_fx_available branch from 93b4c9a to 7f51766 Compare April 4, 2025 01:36

cyyever force-pushed the is_torch_fx_available branch 2 times, most recently from ff855e6 to cdf9717 Compare April 6, 2025 04:05

cyyever changed the title ~~Remove unneeded PyTorch version checks~~ Remove unneeded version checks Apr 6, 2025

cyyever force-pushed the is_torch_fx_available branch 15 times, most recently from 66eeb32 to 200a951 Compare April 8, 2025 16:10

Remove unneeded library version checks

bf8b7bf

Signed-off-by: cyy <[email protected]>

cyyever force-pushed the is_torch_fx_available branch from 200a951 to bf8b7bf Compare April 8, 2025 16:16

cyyever changed the title ~~Remove unneeded version checks~~ Remove code for old PyTorch and Accelerator Apr 8, 2025

cyyever changed the title ~~Remove code for old PyTorch and Accelerator~~ Remove old code for PyTorch and Accelerator Apr 9, 2025

ydshieh reviewed Apr 9, 2025

View reviewed changes

Remove PyTorch condition

1b7fc82

Signed-off-by: cyy <[email protected]>

cyyever changed the title ~~Remove old code for PyTorch and Accelerator~~ Remove old code for PyTorch, Accelerator and tokenizers Apr 9, 2025

cyyever added 3 commits April 9, 2025 22:02

Remove PyTorch condition

ef38892

Signed-off-by: cyy <[email protected]>

Fix ROCm get_device_capability

0e75643

Signed-off-by: cyy <[email protected]>

Revert "Fix ROCm get_device_capability"

c0e5b3d

This reverts commit 0e75643.

ydshieh reviewed Apr 10, 2025

View reviewed changes

src/transformers/utils/import_utils.py Outdated Show resolved Hide resolved

ydshieh reviewed Apr 10, 2025

View reviewed changes

tests/trainer/test_trainer.py Show resolved Hide resolved

ydshieh reviewed Apr 10, 2025

View reviewed changes

tests/trainer/test_trainer.py Show resolved Hide resolved

ydshieh reviewed Apr 10, 2025

View reviewed changes

tests/utils/test_modeling_utils.py Outdated Show resolved Hide resolved

Remove unnecessary check

58c4659

Signed-off-by: cyy <[email protected]>

cyyever and others added 2 commits April 10, 2025 23:24

Revert changes

f10b666

Signed-off-by: cyy <[email protected]>

Merge branch 'main' into is_torch_fx_available

d10cb29

ydshieh merged commit 371c44d into huggingface:main Apr 10, 2025
18 checks passed

cyyever deleted the is_torch_fx_available branch April 11, 2025 00:54

Remove old code for PyTorch, Accelerator and tokenizers #37234

Remove old code for PyTorch, Accelerator and tokenizers #37234

Uh oh!

Conversation

cyyever commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Uh oh!

github-actions bot commented Apr 3, 2025

Uh oh!

Rocketknight1 left a comment

Choose a reason for hiding this comment

Uh oh!

cyyever commented Apr 4, 2025

Uh oh!

Rocketknight1 commented Apr 4, 2025

Uh oh!

cyyever commented Apr 4, 2025

Uh oh!

cyyever commented Apr 8, 2025

Uh oh!

cyyever commented Apr 8, 2025

Uh oh!

ydshieh commented Apr 8, 2025

Uh oh!

cyyever commented Apr 8, 2025

Uh oh!

ydshieh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ydshieh Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

cyyever Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

ydshieh Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

ydshieh Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

cyyever Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ydshieh Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

cyyever Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ydshieh commented Apr 10, 2025

Uh oh!

cyyever commented Apr 10, 2025

Uh oh!

ydshieh commented Apr 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cyyever commented Apr 3, 2025 •

edited

Loading

cyyever Apr 10, 2025 •

edited

Loading