Skip to content

Conversation

@LysandreJik
Copy link
Member

@LysandreJik LysandreJik commented Mar 19, 2025

This PR updates the dummy-creation process that had been in place up to now. Instead of defining dummies in the lib, they're now dynamically created when the required dependencies for an object aren't found.

See below for an extract from the documentation explaining the current updated approach.


While we strive for minimal dependencies, some models have specific dependencies requirements that cannot be
worked around. We don't want for all users of transformers to have to install those dependencies to use other models,
we therefore mark those as soft dependencies rather than hard dependencies.

The transformers toolkit is not made to error-out on import of a model that has a specific dependency; instead, an
object for which you are lacking a dependency will error-out when calling any method on it. As an example, if
torchvision isn't installed, the fast image processors will not be available.

This object is still importable:

>>> from transformers import DetrImageProcessorFast
>>> print(DetrImageProcessorFast)
<class 'DetrImageProcessorFast'>

However, no method can be called on that object:

>>> DetrImageProcessorFast.from_pretrained()
ImportError: 
DetrImageProcessorFast requires the Torchvision library but it was not found in your environment. Checkout the instructions on the
installation page: https://pytorch.org/get-started/locally/ and follow the ones that match your environment.
Please note that you may need to restart your runtime after installation.

Let's see how to specify specific object dependencies.

Specifying Object Dependencies

Filename-based

All objects under a given filename have an automatic dependency to the tool linked to the filename

TensorFlow: All files starting with modeling_tf_ have an automatic TensorFlow dependency.

Flax: All files starting with modeling_flax_ have an automatic Flax dependency

PyTorch: All files starting with modeling_ and not valid with the above (TensorFlow and Flax) have an automatic
PyTorch dependency

Tokenizers: All files starting with tokenization_ and ending with _fast have an automatic tokenizers dependency

Vision: All files starting with image_processing_ have an automatic dependency to the vision dependency group;
at the time of writing, this only contains the pillow dependency.

Vision + Torch + Torchvision: All files starting with image_processing_ and ending with _fast have an automatic
dependency to vision, torch, and torchvision.

All of these automatic dependencies are added on top of the explicit dependencies that are detailed below.

Explicit Object Dependencies

We add a method called requires that is used to explicitly specify the dependencies of a given object. As an
example, the Trainer class has two hard dependencies: torch and accelerate. Here is how we specify these
required dependencies:

from .utils.import_utils import requires

@requires(backends=("torch", "accelerate"))
class Trainer:
    ...

Backends that can be added here are all the backends that are available in the import_utils.py module.

@github-actions github-actions bot marked this pull request as draft March 19, 2025 14:59
@github-actions
Copy link
Contributor

Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. When it is ready for review, please click the Ready for review button (at the bottom of the PR page).

@LysandreJik LysandreJik force-pushed the dummy-removal branch 2 times, most recently from 907994d to 82203a3 Compare March 26, 2025 16:56
@LysandreJik LysandreJik marked this pull request as ready for review March 26, 2025 16:57
Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks to the Great Cleaner @LysandreJik

@LysandreJik LysandreJik force-pushed the dummy-removal branch 3 times, most recently from eafece1 to 86d19fb Compare March 28, 2025 10:15
@LysandreJik LysandreJik changed the base branch from main to tests-fetcher-test-all March 28, 2025 10:24
@LysandreJik LysandreJik force-pushed the dummy-removal branch 5 times, most recently from c2d9ac6 to a7d9a91 Compare March 28, 2025 14:56
@LysandreJik
Copy link
Member Author

@molbap will review this sometime early next week and then we can merge 🤗

@molbap
Copy link
Contributor

molbap commented Apr 7, 2025

Will finish this review tomorrow morning!

@molbap molbap self-requested a review April 7, 2025 16:16
Copy link
Contributor

@molbap molbap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, I read all modifications 👀
I tested:

  • Creating a new model (llava-like) that requires torch/torchvision. No issues there, inits are created correctly and imports work fine.
  • Imports of specific models do not seem to be slowed down. Specifically ;
    Time to import transformers on main (stats on 20 runs)
real: mean=1.985  std=0.126
user: mean=2.179  std=0.082
sys:  mean=0.326  std=0.020

on the branch:

real: mean=2.231  std=0.128
user: mean=2.259  std=0.097
sys:  mean=0.390  std=0.033

which is a very minor (but measurable) slowdown. For a specific model that requires backends,
Time to import LlavaForConditionalGeneration:
main:

real: mean=3.731  std=0.200
user: mean=3.259  std=0.174
sys:  mean=0.642  std=0.035

on the branch:

real: mean=3.740  std=0.184
user: mean=3.367  std=0.154
sys:  mean=0.587  std=0.039

No particular difference here.

New-model-addition correctly picks up on the init format (makes sense since for now it's copying it, modular will have to follow through

So the minor slowdowns may be due to the all being longer than it was, more modules to simply list even if they are not individually imported?

I've left one very minor comment or two but seems to hold up really well 🙌

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note, if torchvision is present but not torch (which may happen but isn't a runnable config) , then

    if is_torchvision_available():
         from torchvision import io as torchvision_io

and likely a few more things will fail at the import. Just flagging as torch is not a required dep

if __name__ == "__main__":
check_all_inits()
check_submodules()
# This entire files needs an overhaul
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😁

continue
if d not in direct_deps:
raise ValueError(f"KeyError:{d}. From {m}")
module_prefix = d.rsplit(".", 1)[0].replace(".", "/")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not familiar - so was common to get transformers.modeling_utils when we wanted e.g. transformers/modeling_utils.py ? good fallback then

Comment on lines +1899 to +1879
module_keys = set(
chain(*[[k.rsplit(".", i)[0] for i in range(k.count(".") + 1)] for k in list(module.keys())])
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just added a comment if I get it right

Suggested change
module_keys = set(
chain(*[[k.rsplit(".", i)[0] for i in range(k.count(".") + 1)] for k in list(module.keys())])
)
# collect all intermediate module paths from keys like "transformers.models.bart.modeling_bart", counting dots
module_keys = set(
chain(*[[k.rsplit(".", i)[0] for i in range(k.count(".") + 1)] for k in list(module.keys())])
)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha it is indeed unclear, it's so that when you import a module such as transformers.models.bart.modeling_bart.py, you can also import

transformers.models.bart.modeling_bart
transformers.models.bart
transformers.models
transformers

I'll add a comment

Copy link
Contributor

@molbap molbap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after discussion. Importing a model before:
image
Importing a model now:
image
Same time - the listing of all models is now explicit (all the tiny bars on the right).

Import * with torch, before:
image

Import * with torch, now:

image
Same measurement as before - roughly 200ms more for the import. Without torch, it takes 400ms.
LGTM!

@LysandreJik LysandreJik changed the base branch from tests-fetcher-test-all to main April 8, 2025 11:26
@LysandreJik LysandreJik force-pushed the dummy-removal branch 2 times, most recently from 4d04a39 to f6f1133 Compare April 9, 2025 14:55
@LysandreJik LysandreJik force-pushed the dummy-removal branch 3 times, most recently from 9ff165a to 45b8eb4 Compare April 11, 2025 07:11
@HuggingFaceDocBuilderDev

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.

@LysandreJik LysandreJik merged commit 54a123f into main Apr 11, 2025
19 of 21 checks passed
@LysandreJik LysandreJik deleted the dummy-removal branch April 11, 2025 09:08
@LysandreJik LysandreJik changed the title Dummies Simplify soft dependencies and update the dummy-creation process Apr 11, 2025
cyr0930 pushed a commit to cyr0930/transformers that referenced this pull request Apr 18, 2025
…gingface#36827)

* Reverse dependency map shouldn't be created when test_all is set

* [test_all] Remove dummies

* Modular fixes

* Update utils/check_repo.py

Co-authored-by: Pablo Montalvo <[email protected]>

* [test_all] Better docs

* [test_all] Update src/transformers/commands/chat.py

Co-authored-by: Joao Gante <[email protected]>

* [test_all] Remove deprecated AdaptiveEmbeddings from the tests

* [test_all] Doc builder

* [test_all] is_dummy

* [test_all] Import utils

* [test_all] Doc building should not require all deps

---------

Co-authored-by: Pablo Montalvo <[email protected]>
Co-authored-by: Joao Gante <[email protected]>
zucchini-nlp pushed a commit to zucchini-nlp/transformers that referenced this pull request May 14, 2025
…gingface#36827)

* Reverse dependency map shouldn't be created when test_all is set

* [test_all] Remove dummies

* Modular fixes

* Update utils/check_repo.py

Co-authored-by: Pablo Montalvo <[email protected]>

* [test_all] Better docs

* [test_all] Update src/transformers/commands/chat.py

Co-authored-by: Joao Gante <[email protected]>

* [test_all] Remove deprecated AdaptiveEmbeddings from the tests

* [test_all] Doc builder

* [test_all] is_dummy

* [test_all] Import utils

* [test_all] Doc building should not require all deps

---------

Co-authored-by: Pablo Montalvo <[email protected]>
Co-authored-by: Joao Gante <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants