Fixed PyTorch version check in sparse module
#1136
Conversation
|
Thank you for the PR! |
1 similar comment
|
Thank you for the PR! |
Codecov Report
@@ Coverage Diff @@
## main #1136 +/- ##
=======================================
Coverage 91.77% 91.77%
=======================================
Files 72 72
Lines 10485 10485
=======================================
Hits 9623 9623
Misses 862 862
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
heat/sparse/factories.py
Outdated
| @@ -94,7 +94,7 @@ def sparse_csr_matrix( | |||
| (indptr: tensor([0, 1, 3, 4]), indices: tensor([2, 0, 2, 2]), data: tensor([1, 1, 2, 3]), dtype=ht.int64, device=cpu:0, split=None) | |||
| """ | |||
| # version check | |||
| if int(torch.__version__.split(".")[1]) < 10: | |||
| if torch.__version__ < "1.10": | |||
There was a problem hiding this comment.
oh, I didn't know this was possible!
I had to go and time these two methods. We do many many checks, and those millionths of seconds pile up. 🤭
Turns out the string check (this PR) is about 10 times slower than the int check (previous solution) even after having to manually split the string. The int check is even faster for torch >=2, as the first condition is sufficient to make the decision.
Summary: I'd go for
if int(torch.__version__.split(".")[0]) <= 1 and int(torch.__version__.split(".")[1]) < 10solution in factories as well. What do you think?
There was a problem hiding this comment.
Interesting to know that it is soo much slower.
I have reverted back to the older method of version checking.
|
Thank you for the PR! |
|
Thank you for the PR! |
ClaudiaComito
changed the title
sparse module
Description
The earlier check for torch version fails in the new major version. This fix solves the issue.
Type of change
Bug fix
Does this change modify the behaviour of other functions? If so, which?
no