Encapsulate all forward passes of integration tests with "with torch.no_grad()"
Author: NielsRogge
Pull request: no
Labels: name: Good First Issue

Each model in the Transformers library has one or more corresponding integration tests, which make sure the implementation in Transformers returns the same exact logits/hidden states as the original implementation on the same input data.

e.g. the integration test for the Vision Transformer (ViT) can be found [here](https://github.com/huggingface/transformers/blob/df085d8ea8b823b85ecfe2e1968b5741983f98f3/tests/test_modeling_vit.py#L336). It verifies the shape of the logits as well as the first values on a cats image.

However, a lot of these integration tests don't leverage `with torch.no_grad()`, which make sure no gradients are computed (which isn't necessary when doing inference).

Hence, it would be great if someone could encapsulate the forward passes of the integration tests with `with torch.no_grad()`.

Comments:
Hey, I'd love to work on this!

Sounds good @itsTurner, feel free to open a PR :)

@itsTurner Are you still working on this issue?. If not then I would love to contribute to this issue. 

Feel free to open per-model PRs so that you may both contribute at the same time! (and so can other members of the community!)

@LysandreJik I have created this pr. Please let me know if this was the expected behaviour

Hey @NielsRogge, thanks for opening this issue and making it accessible for everyone!I have a few questions about this issue. 

1. Is `with torch.no_grad()` preferable over the `@torch.no_grad()` decorator? Personally I find the latter cleaner, and it removes the need to hunt for forward passes and open a `with` statement each time, especially when a single test case has multiple forward passes. 
2. PyTorch 1.9 introduced [`torch.inference_mode`](https://pytorch.org/docs/stable/generated/torch.inference_mode.html), which provides further optimizations on top of `torch.no_grad()`. Would this be preferable, or would it introduce backward compatibility issues with testing? I'm wondering if we can also create a wrapper that decides which context manager to use depending on the user's `torch` version. 

Appreciate your input in advance!

@LysandreJik Do you guys still need help with the tests ? I would love to open up a PR and work on this. Please let me know. 

Found some integration tests not leveraging `with torch.no_grad()`, 
I'll take on those and open PR :)

Hi @jaketae, that's a very good point. I'll cc @LysandreJik and @patrickvonplaten to decide on which option we use best for integration tests. In any case, an annotator is more clean than using a context manager imo.

No opposition from my side to use `@torch.no_grad()`, I find it clean as well, and this way we ensure that the whole method does not use gradients.

Ok for me too! 

@NielsRogge  @LysandreJik I believe everyone has agreed. Is this issue still open ? Can I work on this ?

cc'ing @ydshieh here as I'm not sure we can just add `torch.no_grad()`, due to the fact that torch needs to be available for that

This will fail if torch is not available. Even if I place `require_torch` in difference places and in different order with `torch.no_grad`. We can make a custom `no_grad` decorator if this is OK for everyone.

Hi @LysandreJik do you still need help with this issue? I'd love to open a PR on the RemBert and Camembert models!