-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Skip some Convolution2D
tests for older numpy versions
#8458
Conversation
I suggest avoid skipping but instead omit numerical checks (#7946 (comment)). |
83490a9
to
06d4e9e
Compare
Comment addressed, thanks a lot for the feedback. |
tests/chainer_tests/functions_tests/connection_tests/test_convolution_2d.py
Outdated
Show resolved
Hide resolved
06d4e9e
to
2bf1b9e
Compare
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.
Would you leave notes to refer the discussion?
tests/chainer_tests/functions_tests/connection_tests/test_convolution_2d.py
Show resolved
Hide resolved
tests/chainer_tests/functions_tests/connection_tests/test_convolution_2d.py
Show resolved
Hide resolved
pfnCI, test this please. |
Jenkins CI test (for commit 2bf1b9e, target branch master) succeeded! |
LGTM! The CI failure on Travis is not related to this change. |
Skip some `Convolution2D` tests for older numpy versions
Closes #7946
The reason behind the flakiness is the use of a numpy version < 1.17
1.17 fixed a bug in the float16 implementation.
What happens is that forward_expected is comparing all the backends against the cpu implementation using numpy. And when using CUDA the results in float16 might sightly differ in some corner cases dut to the cpu-gpu difference (numpy bug) in fp16 implementation. Other tests that rely on numpy for forward_expected are most likely to be affected by this.
This failure only happens with grouped convolutions.