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

Replace deprecated pytorch methods #1814

Merged
merged 4 commits into from
Dec 16, 2024
Merged

Conversation

lipeng31
Copy link
Contributor

  • torch.cuda.amp.GradScaler(...) => torch.amp.GradScaler("cuda", ...)
  • torch.cuda.amp.autocast(...) => torch.amp.autocast("cuda", ...)

- torch.cuda.amp.GradScaler(...) => torch.amp.GradScaler("cuda", ...)
- torch.cuda.amp.autocast(...) => torch.amp.autocast("cuda", ...)
@csukuangfj
Copy link
Collaborator

could you make it backward compatible?

@lipeng31
Copy link
Contributor Author

I read the PyTorch docs and found the first version that provides both torch.cuda.amp.autocast and torch.cuda.amp.GradScaler is 1.6.0, and the first version that provides torch.amp.autocast("cuda", ...) and torch.amp.GradSclaer("cuda", ...) is 1.10.0 which was released 3 years ago, so the compatibility problem happens only for the versions of [1.6.0,1.10.0). I'm not sure if this icefall is supposed to support those versions of PyTorch. Additionally, maintaining compatibility with those versions of PyTorch would require numerous changes throughout the codebase, potentially making the code less clean. Could you reconsider the necessity of this compatibility? Thanks.

@lipeng31
Copy link
Contributor Author

Last commit fails some checks. I'll fix it soon.

Along with reformatting to pass black lint.

- egs/libritts/ASR/zipformer/train.py
- egs/libritts/CODEC/encodec/encodec.py
- egs/ljspeech/TTS/vits/vits.py
- egs/wenetspeech4tts/TTS/valle/train.py
@lipeng31
Copy link
Contributor Author

lipeng31 commented Dec 4, 2024

Hi @csukuangfj , I have completed the code revision. There are some checks that still haven't passed, which I believe are unrelated to the changes I've made. Could you please review the pull request? Thank you.

Copy link
Collaborator

@csukuangfj csukuangfj left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

@csukuangfj
Copy link
Collaborator

@lipeng31 Could you resolve the conflicts?

@lipeng31
Copy link
Contributor Author

@lipeng31 Could you resolve the conflicts?

Conflicts resolved, please review it, thanks.

Copy link
Collaborator

@csukuangfj csukuangfj left a comment

Choose a reason for hiding this comment

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

Thank you!

@csukuangfj csukuangfj merged commit 3e4da5f into k2-fsa:master Dec 16, 2024
113 of 123 checks passed
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.

2 participants