You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/08/10 22:53:53 UTC
[GitHub] [tvm] mehrdadh opened a new pull request #8712: [Test] Increase atol for CI
mehrdadh opened a new pull request #8712:
URL: https://github.com/apache/tvm/pull/8712
Fixing this issue: https://github.com/apache/tvm/issues/8690
cc @junrushao1994 @tqchen
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8712: [Test] Increase atol for CI
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8712:
URL: https://github.com/apache/tvm/pull/8712#issuecomment-896469614
Yeah @comaniac I mean using 1e-4 for every parametrization for this particular test case, so we don't actually have disagreement here :-)
I don't think we need to be really specific to target though. Whatever device it is, the amount of computation (MADs) is fixed and determined by the shape, so the margin of numeric error should be sort of similar
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] comaniac commented on a change in pull request #8712: [Test] Increase atol for CI
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #8712:
URL: https://github.com/apache/tvm/pull/8712#discussion_r686382635
##########
File path: tests/python/topi/python/test_topi_conv2d_nchw.py
##########
@@ -142,7 +142,7 @@ def test_conv2d_nchw(
if "int" in dtype:
tol = {"atol": 0, "rtol": 0}
elif dtype == "float32":
- tol = {"rtol": 1e-4, "atol": 1e-5}
+ tol = {"rtol": 1e-4, "atol": 1e-4}
Review comment:
Why we need to release the tol for float32? If it's only for a specific target, then we should release the tol for that target instead of all cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] comaniac commented on pull request #8712: [Test] Increase atol for CI
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #8712:
URL: https://github.com/apache/tvm/pull/8712#issuecomment-896431270
> I think we can change everything to 1e-4, given that we do expect precision issues if the compute order is different
I'm a bit concern to use 1e-4 everywhere even compute order is indeed an issue, because the error may propagate and accumulate. If we allow every op to have 1e-4 then it's possible to hurt the accuracy eventually especially when the graph is large. That's why I would prefer to know the specific case that we really have no way but can only use 1e-4. The flaky issue can be workaround by other approaches like fixing the random seed IMHO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #8712: [Test] Increase atol for CI
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #8712:
URL: https://github.com/apache/tvm/pull/8712#issuecomment-896835262
see additional comments in #8690 on zero. BTW. given there are a lot of actual zeros in y but not in x, the simplest approach could be swap x and y in this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8712: [Test] Increase atol for CI
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8712:
URL: https://github.com/apache/tvm/pull/8712#issuecomment-896487641
looks like we are experiencing more flakiness :-(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8712: [Test] Increase atol for CI
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8712:
URL: https://github.com/apache/tvm/pull/8712#issuecomment-896391212
I think we can change everything to 1e-4, given that we do expect precision issues if the compute order is different
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #8712: [Test] Increase atol for CI
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #8712:
URL: https://github.com/apache/tvm/pull/8712#issuecomment-897176809
Thanks! Let's get it merged ASAP
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jroesch merged pull request #8712: [Test] Increase atol for CI
Posted by GitBox <gi...@apache.org>.
jroesch merged pull request #8712:
URL: https://github.com/apache/tvm/pull/8712
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #8712: [Test] Increase atol for CI
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #8712:
URL: https://github.com/apache/tvm/pull/8712#discussion_r686405803
##########
File path: tests/python/topi/python/test_topi_conv2d_nchw.py
##########
@@ -142,7 +142,7 @@ def test_conv2d_nchw(
if "int" in dtype:
tol = {"atol": 0, "rtol": 0}
elif dtype == "float32":
- tol = {"rtol": 1e-4, "atol": 1e-5}
+ tol = {"rtol": 1e-4, "atol": 1e-4}
Review comment:
Yeah this particular case is causing a lot of flakiness
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org