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

fix pytorch division warning by using suggested torch.div rounding_mode #14577

Closed
wants to merge 1 commit into from

Conversation

mgoldey
Copy link
Contributor

@mgoldey mgoldey commented Nov 30, 2021

What does this PR do?

This PR removes a warning that is repeatedly thrown with the latest releases of transformers and pytorch.

example:

lib/python3.7/site-packages/transformers/models/hubert/modeling_hubert.py:803: UserWarning: __floordiv__ is deprecated, and its behavior will change in a future version of pytorch. It currently rounds toward 0 (like the 'trunc' function NOT 'floor'). This results in incorrect rounding for negative values. To keep the current behavior, use torch.div(a, b, rounding_mode='trunc'), or for actual floor division, use torch.div(a, b, rounding_mode='floor').

The fact that this occurs in multiple places suggests there's an opportunity for a shared function. Instead of a larger refactor, this PR touches a few specific places using // to avoid causing other side effects.

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.

No

Docs should be unaffected

  • Did you write any new necessary tests?

The test coverage should be unchanged.

@sgugger
Copy link
Collaborator

sgugger commented Dec 6, 2021

Note that this argument of torch.div was only introduced in PyTorch 1.8.0, so this fix is not compatible with all the versions of torch we support (1.4.0 and onward). I think we will need to write a function that uses the old syntax (//) for older versions and only switches to the newer syntax in more recent versions, then use that function.

@patrickvonplaten
Copy link
Contributor

Note that this argument of torch.div was only introduced in PyTorch 1.8.0, so this fix is not compatible with all the versions of torch we support (1.4.0 and onward). I think we will need to write a function that uses the old syntax (//) for older versions and only switches to the newer syntax in more recent versions, then use that function.

Agree! Let's put it in src/transformers/file_utils.py no? @mgoldey - would be interested in adapting the PR to include such a function? The function could look very similar to this syntax:

if version.parse(torch.__version__) < version.parse("1.4"):

@mgoldey
Copy link
Contributor Author

mgoldey commented Jan 3, 2022

I'd be happy to put forward something to that effect in the next few days

@sgugger
Copy link
Collaborator

sgugger commented Jan 10, 2022

Hi @mgoldey did you have time to work on this? It looks like it's going to be needed in the PR mentioned above as well, so we can take over on the implementation of the custom function Patrick mentioned you don't have time :-)

@mgoldey
Copy link
Contributor Author

mgoldey commented Jan 14, 2022

Hi @mgoldey did you have time to work on this? It looks like it's going to be needed in the PR mentioned above as well, so we can take over on the implementation of the custom function Patrick mentioned you don't have time :-)

Feel free to jump in if you have time. I'll notify if I wind up being free, but I had to switch directions from time-sensitive projects.

@sgugger
Copy link
Collaborator

sgugger commented Jan 17, 2022

Hi @mgoldey, I couldn't push to your branch, so I opened a fresh PR at #15180. I put you as co-author.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@mgoldey
Copy link
Contributor Author

mgoldey commented Feb 10, 2022

Moot - resolved by #15180

@mgoldey mgoldey closed this Feb 10, 2022
@mgoldey mgoldey deleted the mgoldey/FixDivWarning branch February 10, 2022 15:38
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