You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The error log was pretty strange since the reported max rtol is smaller than the actual number. Perhaps it has something to do with fp 16. cc @Lunderberg since this might related to the recent changes you made
Hmm, that failing test is parametrized to include float16, but the failing case was using float32 values. It's also running with a fixed random seed to try to prevent any flakiness due to input data.
That particular parametrization was running with cudnn, added to the CI in #8383 and recently merged. I'm not able to reproduce the failure in my local ci_gpu docker instance, either before or after #8383. (Ran 100 iterations just no, no issues locally.)
Maybe the CI nodes have harder hardware that isn't compatible with this version of cudnn? The ci_gpu image uses cuda 11.0 + cudnn 8.0.5, which should be compatible with SM 3.5 or higher. What hardware are the CI nodes running on?
Trying to followup a bit on the issue. In this case most of the output of the conv2d becomes close to zero. We should look into the test-case generation.
Since approximating something that is super close to zero does not really test the functionality of the code. We should perhaps changes the way the input weight/data are generated.
It looks like the numpy results are returning identically zero for the values around the edge, while the cudnn results have non-zero values. The numpy results are correct due to the padding used for that value.
Something that might be complicating the investigation is that cudnn attempts to find the most efficient method on its own if nothing is specified. Last time I touched this file, I hoisted it up to be a configurable knob, but it still defaults to letting cudnn run its own benchmarks and select the fastest.
My current mental model is that in some cases, cudnn's benchmark selects an algorithm that is faster, but has approximations that result in slightly non-zero padding. If that cudnn implementation is selected, then the test fails in CI. Unfortunately, of the 18 options that cudnn supports, I only have hardware support to test out 4 of them, and all 4 of them keep the padding identically equal to zero.
I'm not sure that would make a difference. The total allowed tolerance is tol = atol + rtol*abs(y), and so that would only increase the total tolerance by a few percent.
Activity
[-][TEST][FLAKY] float16 conv2d [/-][+][TEST][FLAKY] float16 test_conv2d_nchw [/+]tqchen commentedon Aug 9, 2021
The error log was pretty strange since the reported max rtol is smaller than the actual number. Perhaps it has something to do with fp 16. cc @Lunderberg since this might related to the recent changes you made
Lunderberg commentedon Aug 9, 2021
Hmm, that failing test is parametrized to include
float16, but the failing case was usingfloat32values. It's also running with a fixed random seed to try to prevent any flakiness due to input data.That particular parametrization was running with cudnn, added to the CI in #8383 and recently merged. I'm not able to reproduce the failure in my local
ci_gpudocker instance, either before or after #8383. (Ran 100 iterations just no, no issues locally.)Lunderberg commentedon Aug 9, 2021
Maybe the CI nodes have harder hardware that isn't compatible with this version of cudnn? The
ci_gpuimage uses cuda 11.0 + cudnn 8.0.5, which should be compatible with SM 3.5 or higher. What hardware are the CI nodes running on?tqchen commentedon Aug 11, 2021
Trying to followup a bit on the issue. In this case most of the output of the conv2d becomes close to zero. We should look into the test-case generation.
Since approximating something that is super close to zero does not really test the functionality of the code. We should perhaps changes the way the input weight/data are generated.
Lunderberg commentedon Aug 11, 2021
It looks like the numpy results are returning identically zero for the values around the edge, while the cudnn results have non-zero values. The numpy results are correct due to the padding used for that value.
Something that might be complicating the investigation is that cudnn attempts to find the most efficient method on its own if nothing is specified. Last time I touched this file, I hoisted it up to be a configurable knob, but it still defaults to letting cudnn run its own benchmarks and select the fastest.
My current mental model is that in some cases, cudnn's benchmark selects an algorithm that is faster, but has approximations that result in slightly non-zero padding. If that cudnn implementation is selected, then the test fails in CI. Unfortunately, of the 18 options that cudnn supports, I only have hardware support to test out 4 of them, and all 4 of them keep the padding identically equal to zero.
Lunderberg commentedon Aug 11, 2021
And those same 5 are also not supported on kraken, either.
tqchen commentedon Aug 11, 2021
perhaps as a simplest resolution we can swap the numpy and cuddn results. Note that for 0 it matters less
Lunderberg commentedon Aug 11, 2021
I'm not sure that would make a difference. The total allowed tolerance is
tol = atol + rtol*abs(y), and so that would only increase the total tolerance by a few percent.tqchen commentedon Aug 11, 2021
@Lunderberg you are right. perhaps a better way is indeed to relax the atol for now then
mehrdadh commentedon Aug 11, 2021
@tqchen is the consensus to increase the
atol?Lunderberg commentedon Aug 11, 2021
I'd agree on relaxing the atol, since handling result values near zero is what it is intended to do.
mehrdadh commentedon Aug 11, 2021
@Lunderberg sounds good!
shingjan commentedon Aug 11, 2021
This test also happens to be flaky in the CI run of #8492 :
https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/PR-8492/53/pipeline/
https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/PR-8492/55/pipeline/
Lunderberg commentedon Aug 11, 2021
I ran into this same test on #8723 just now, and so I added the
atolupdate fortest_conv2d_nchwto it.tqchen commentedon Aug 15, 2021
close for now as the atol adjustment PR is now merged