Pass missing arguments to SeamlessM4Tv2ConformerEncoderLayer.forward() when gradient checkpointing is enabled

[anferico]

What does this PR do?

Fixes a bug for which if gradient checkpointing is enabled, SeamlessM4Tv2ConformerEncoderLayer.forward() is called with some missing arguments.

Fixes #31028

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?

@ylacombe

[ylacombe]

LGTM ! Thanks for fixing this!
Could you also verify that this doesn't happen in the v1 of SeamlessM4T ?

[anferico]

@ylacombe turns out the same problem existed in v1 too 🤓 Just pushed a new commit fixing that

[ylacombe]

Great!
@amyeroberts, the PR is ready for a review. Thanks in advance!

[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.

[amyeroberts]

@anferico Thanks for adding this fix!

This should be caught in tests. Could you check the test suit for this model to make sure the forward pass with gradient checkpoint is tested?

[anferico]

@amyeroberts The gradient checkpointing-related tests are currently being skipped for both SeamlessM4T v1 and v2.

Aside from that, this issue kinda calls for more robust tests, because the current test suite for gradient checkpointing only checks if gradients are computed for each parameter, but it could never have spotted the edge case where the model.forward() method behaves differently depending on whether GC is enabled or not. I understand it might be a big effort, but ideally we would require every function that is affected by GC be tested twice: once with GC enabled and once with GC disabled, making sure that the outputs are exactly the same given certain inputs. What do you think?

[amyeroberts]

@anferico OK, sounds like the work is a bit more involved. Could you open an issue detailing that the GC tests should be updated? This way we keep track of the work and we can merge this without being blocked

[anferico]

@amyeroberts done! #32063

[amyeroberts]

@anferico Awesome - thank you!