Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Gradient checkpoining] Update Wav2Vec scripts #14036

Merged

Conversation

falcaopetri
Copy link
Contributor

What does this PR do?

This PR makes the Wav2Vec scripts compatible with the changes introduced in #13657 regarding the gradient_checkpointing feature/argument.

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?

@stas00, @LysandreJik, @patrickvonplaten

@stas00
Copy link
Contributor

stas00 commented Oct 16, 2021

FYI, this is a duplicate of #13964, but I think yours is better since mine doesn't change the flax example.

@falcaopetri
Copy link
Contributor Author

Hi and sorry for the duplicate. I checked for similar issues but forgot to search for PRs.

Besides the missed flax example, #13964 possibly runs into this warning: Passing gradient_checkpointing to a config initialization is deprecated and will be removed in v5 Transformers.

The present PR follows the recommendation from Performance#Gradient Checkpointing.

@stas00
Copy link
Contributor

stas00 commented Oct 17, 2021

No need to be sorry, I was just pointing to maintainers that there are 2 of the kind so it's easy to deal with them at once.

Further, #13877 moved wav2vec2 to supported examples, but for some reason these examples didn't get ported.

@falcaopetri
Copy link
Contributor Author

Well noted, the addition of examples/pytorch/speech-pretraining got me confused.

As I understood, run_wav2vec2_pretraining_no_trainer.py is equivalent to examples/research_projects/wav2vec2/run_pretrain.py, but uses accelerate instead of the Trainer API.

Nonetheless, there seems to be some duplicated work in, e.g., argument parsing, dataset setup, and model instantiation. It is also not clear whether the notes in examples/pytorch/speech-pretraining also apply to examples/research_projects/wav2vec2/ (they probably do, so it would be nice to have them together).

@huggingface huggingface deleted a comment from github-actions bot Nov 16, 2021
@stas00
Copy link
Contributor

stas00 commented Nov 16, 2021

@patrickvonplaten, we had 2 similar PRs. #13964 got merged

and this one has one more file covered that mine didn't.

I rebased it to incorporate the changes from the other PR.

@patrickvonplaten patrickvonplaten merged commit 7544efc into huggingface:master Nov 17, 2021
@patrickvonplaten
Copy link
Contributor

Thanks for updating the scripts!

Albertobegue pushed a commit to Albertobegue/transformers that referenced this pull request Jan 27, 2022
Co-authored-by: Stas Bekman <stas@stason.org>
@falcaopetri falcaopetri deleted the wav2vec_gradient_checkpointing branch April 1, 2024 19:08
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.

3 participants