Skip warning if tracing with dynamo

[angelayi]

What does this PR do?

Adds an additional check to skip the check for missing attention mask when tracing. Currently it checks if we're torch.jit.trace-ing or torch.fx.symbolic_trace-ing, so this adds a check to see if we're tracing with TorchDynamo.

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.

Tagging @hackyon who added this check previously

[hackyon]

Thanks for fixing this (I assume this might have broken a workflow somewhere).

The compilation is failing, I think we need to check if torch dynamo is available first using is_torchdynamo_available()? If so, it also "import torch._dynamo as dynamo" and so should be able to use dynamo.is_compiling() afterwards.

[ArthurZucker]

cc @fxmarty

[HuggingFaceDocBuilderDev]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

[fxmarty]

_dynamo is not visible in the torch namespace, so you need a import torch._dynamo. This PR breaks compatibility with torch<=1.13.1 though.

[angelayi]

Thanks for the quick review! I updated the diff with checking to see if the torch._dynamo module is importable and if it's available. Does this fix the issue with breaking compatibility with torch<=1.13.1?

[angelayi]

@fxmarty could I get a review on this PR?

[hackyon]

Thanks.

I figured I'd give my 2 cents even though I'm not a core reviewer. It'd probably be better to put the checking logic inside a helper method in the import_utils, next to is_torchdynamo_available():
https://github.com/huggingface/transformers/blob/main/src/transformers/utils/import_utils.py

There are other locations with logic to detect tracing, and the helper method will help us incorporate dynamo tracing to those locations as well.

[angelayi]

@ArthurZucker @fxmarty do you know who else would be free to review this?

[fxmarty]

LGTM, thank you! If I understand well this may allow to avoid some graph breaks and capturing irrelevant part of the code, so it is nice.

I am not sure whether dynamo tracing is tested anywhere though.

[angelayi]

@fxmarty thanks for the review! I noticed there's a "1 workflow awaiting approval" -- do you mind approving this too?

[ArthurZucker]

For now we have _create_and_check_torch_fx_tracing which checks symbolic_trace but it's not always tested. Would be nice to add a small test in test_warn_if_padding_and_no_attention_mask 😉

[angelayi]

@ArthurZucker Updated with a test that checks if torch compiling with dynamic shapes will run successfully and not result in a graph break.

[amyeroberts]

Thanks for adding this and a test!

Let's make sure @ArthurZucker is happy with the test before we merge in 🤗

[ArthurZucker]

Left a small nit but good for me! Thanks for contributing and bearing with me! 🤗

[angelayi]

Thanks for the review! I updated the test w/ a better name. Dumb question..how do I merge this 😅

[ArthurZucker]

I'll merge for you 😉