Skip to content

Fix torch version comparisons (helps with +cu*** or +cpu official builds)#18460

Merged
sgugger merged 1 commit intohuggingface:mainfrom
LSinev:fix_torch_version_comparisons
Aug 3, 2022
Merged

Fix torch version comparisons (helps with +cu*** or +cpu official builds)#18460
sgugger merged 1 commit intohuggingface:mainfrom
LSinev:fix_torch_version_comparisons

Conversation

@LSinev
Copy link
Contributor

@LSinev LSinev commented Aug 3, 2022

Comparisons like version.parse(torch.__version__) > version.parse("1.6")
are True for torch==1.6.0+cu101 or torch==1.6.0+cpu (which is not intended, I suppose).

So version.parse(version.parse(torch.__version__).base_version) comparisons are preferred (and used in pytorch_utils.py but not in other places).

What does this PR do?

  • Updated all comparisons to failsafe (when used in copypasting inspiration) version.parse(version.parse(torch.__version__).base_version)
  • added some often used patterns to pytorch_utils.py

Did not check if original version checks were eligible. Believe original authors just missed this version check caveat.
Only torch version check changed (not sure if other packages may be affected).

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?

As it is touching many parts of code: @patrickvonplaten, @LysandreJik, @sgugger

Comparisons like
version.parse(torch.__version__) > version.parse("1.6")
are True for torch==1.6.0+cu101 or torch==1.6.0+cpu

version.parse(version.parse(torch.__version__).base_version) are preferred (and available in pytorch_utils.py
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 3, 2022

The documentation is not available anymore as the PR was closed or merged.

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Amazing job, thanks a lot!
The tests were not properly triggered so launching them now. Will merge if all is green!

@sgugger sgugger merged commit 02b176c into huggingface:main Aug 3, 2022
@LSinev LSinev deleted the fix_torch_version_comparisons branch August 3, 2022 18:04
@patrickvonplaten
Copy link
Contributor

Wow very cool @LSinev !

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.

4 participants