-
Notifications
You must be signed in to change notification settings - Fork 7.2k
Make torch version check numeric #4285
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
Make torch version check numeric #4285
Conversation
411b1f8 to
106b293
Compare
datumbox
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! Will merge once the CI completes.
Summary: Co-authored-by: Vasilis Vryniotis <[email protected]> Reviewed By: NicolasHug Differential Revision: D30417204 fbshipit-source-id: 17ba279a691378c2f6ad9968bd7b9ece823173a6
| if torch.__version__ >= '1.5': | ||
| TORCH_MAJOR = int(torch.__version__.split('.')[0]) | ||
| TORCH_MINOR = int(torch.__version__.split('.')[1]) | ||
| if TORCH_MAJOR > 1 or (TORCH_MAJOR == 1 and TORCH_MINOR >= 5): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be using the newly introduced TorchVersion instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created an issue to track this down #4296
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to use the new torch.__version__ as a TorchVersion object , then just reverting this pr will do the job.
Or can I create a new pr for just one line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or can I create a new pr for just one line?
Yup :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Thanks Sir
Co-authored-by: Vasilis Vryniotis <[email protected]>
String comparison messes up in cases such as
torch.__version__ = 1.10.blah