-
Notifications
You must be signed in to change notification settings - Fork 413
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
Add half precision testing [1/n] #77
Conversation
Codecov Report
@@ Coverage Diff @@
## master #77 +/- ##
==========================================
+ Coverage 96.67% 96.89% +0.21%
==========================================
Files 148 74 -74
Lines 4632 2319 -2313
==========================================
- Hits 4478 2247 -2231
+ Misses 154 72 -82
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
can we rather use this function version? Lightning-AI/pytorch-lightning#6434 |
Of course :] |
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 !
tests/regression/test_mean_error.py
Outdated
@pytest.mark.skipif( | ||
not _TORCH_GREATER_EQUAL_1_6, |
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.
Not sure if we really should skip here or show a more meaningful error message.
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.
@SkafteNicki ^^
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.
@justusschock you got any proposal?
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.
@justusschock more meaningful message in test or the class itself? wouldn't be easier to test for half precision tensors in the base metric class and check if torch version >= 1.6
@SkafteNicki mind resolve conflicts... :] |
Before submitting
What does this PR do?
Issue #57
Adds testing interface for half precision and apply to few metrics.
Follow up PRs will add the testing to remaining metrics and add documentation to metrics where it will not work.
Note, that we cannot really do anything about it not working for some of the metrics as some pytorch operations (e.g
torch.log
) does not support half precision + cpu.PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃