-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Topi][Testing] Float16 unittests for dense, conv2d, depthwise conv2d #8529
Conversation
Potential reviewers: @masahi (implemented primarily for vulkan float16 support), @tmoreau89 (float16 progress) |
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.
In general I'm afraid of flaky tests due to the rounding errors. Can we at least fix the random seed in the test to reduce the possibility?
Good call, especially with the much larger relative error of float16. Change has been made. |
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
e249fd5
to
1b5363b
Compare
df89fd9
to
97a86a7
Compare
Bumping to restart CI after #8656 . |
97a86a7
to
b86342b
Compare
- Use input dtype for dilate/conv2d accumulate in python impl. Previously, the python implementations of dilation and conv2d would use numpy default dtype in some cases, rather than the input data's dtype. - Added fallback for datatypes not supported by scipy.signal.convolve2d (e.g. float16). - Refactored to avoid duplication, use common get_pad_tuple functionality.
Intended to avoid flaky test failures later due to rounding errors.
- ref_data must be a test fixture, not acquired through request.getfixturevalue, in order to have the random_seed be known. - dilate_python's return value didn't follow `out_dtype`. - The test_topi_conv3d tests had the reference results computed in float64, due to dilate_python() not respecting the input data type. With the correct dtype, the tolerances needed to be slightly widened.
b86342b
to
e061b0d
Compare
Rebased to main to restart CI. Looks like it was a timeout in the macOS compilation. |
…apache#8529) * [Topi][Testing] Minor cleanup for python reference implementations - Use input dtype for dilate/conv2d accumulate in python impl. Previously, the python implementations of dilation and conv2d would use numpy default dtype in some cases, rather than the input data's dtype. - Added fallback for datatypes not supported by scipy.signal.convolve2d (e.g. float16). - Refactored to avoid duplication, use common get_pad_tuple functionality. * [Topi][UnitTests] Added float16 tests to test_topi_dense.py * [Topi][UnitTests] Added float16 to test_topi_conv2d_nchw.py * [Topi][Float16] Added float16 tests for depthwise conv2d. * [UnitTests] Explicitly set seed for float16 tests Intended to avoid flaky test failures later due to rounding errors. * [UnitTests] Fixed a few failing unit tests. - ref_data must be a test fixture, not acquired through request.getfixturevalue, in order to have the random_seed be known. - dilate_python's return value didn't follow `out_dtype`. - The test_topi_conv3d tests had the reference results computed in float64, due to dilate_python() not respecting the input data type. With the correct dtype, the tolerances needed to be slightly widened. Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
…apache#8529) * [Topi][Testing] Minor cleanup for python reference implementations - Use input dtype for dilate/conv2d accumulate in python impl. Previously, the python implementations of dilation and conv2d would use numpy default dtype in some cases, rather than the input data's dtype. - Added fallback for datatypes not supported by scipy.signal.convolve2d (e.g. float16). - Refactored to avoid duplication, use common get_pad_tuple functionality. * [Topi][UnitTests] Added float16 tests to test_topi_dense.py * [Topi][UnitTests] Added float16 to test_topi_conv2d_nchw.py * [Topi][Float16] Added float16 tests for depthwise conv2d. * [UnitTests] Explicitly set seed for float16 tests Intended to avoid flaky test failures later due to rounding errors. * [UnitTests] Fixed a few failing unit tests. - ref_data must be a test fixture, not acquired through request.getfixturevalue, in order to have the random_seed be known. - dilate_python's return value didn't follow `out_dtype`. - The test_topi_conv3d tests had the reference results computed in float64, due to dilate_python() not respecting the input data type. With the correct dtype, the tolerances needed to be slightly widened. Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
…apache#8529) * [Topi][Testing] Minor cleanup for python reference implementations - Use input dtype for dilate/conv2d accumulate in python impl. Previously, the python implementations of dilation and conv2d would use numpy default dtype in some cases, rather than the input data's dtype. - Added fallback for datatypes not supported by scipy.signal.convolve2d (e.g. float16). - Refactored to avoid duplication, use common get_pad_tuple functionality. * [Topi][UnitTests] Added float16 tests to test_topi_dense.py * [Topi][UnitTests] Added float16 to test_topi_conv2d_nchw.py * [Topi][Float16] Added float16 tests for depthwise conv2d. * [UnitTests] Explicitly set seed for float16 tests Intended to avoid flaky test failures later due to rounding errors. * [UnitTests] Fixed a few failing unit tests. - ref_data must be a test fixture, not acquired through request.getfixturevalue, in order to have the random_seed be known. - dilate_python's return value didn't follow `out_dtype`. - The test_topi_conv3d tests had the reference results computed in float64, due to dilate_python() not respecting the input data type. With the correct dtype, the tolerances needed to be slightly widened. Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
This commit adds fp16 test cases to the conv2d NHWC TOPI schedules for `arm_cpu`. Following the example of apache#8529, the numpy reference conv2d output is computed in fp32 instead of fp16, while the absolute tolerance varies for each test case according to the size of the summed axis and the output's largest element.
…17007) This commit adds fp16 test cases to the conv2d NHWC TOPI schedules for `arm_cpu`. Following the example of #8529, the numpy reference conv2d output is computed in fp32 instead of fp16, while the absolute tolerance varies for each test case according to the size of the summed axis and the output's largest element.
The goal of this PR is to add float16 test cases to topi schedules used for ResNet50. This PR is split up into independent commits, as described below, for ease of review.
[Topi][Testing] Minor cleanup for python reference implementations
Use input dtype for dilate/conv2d accumulate in python
impl. Previously, the python implementations of dilation and conv2d
would use numpy default dtype in some cases, rather than the input
data's dtype.
Added fallback for datatypes not supported by scipy.signal.convolve2d (e.g. float16).
Refactored to avoid duplication, use common get_pad_tuple functionality.
[Topi][UnitTests] Added float16 tests to test_topi_dense.py
[Topi][UnitTests] Added float16 to test_topi_conv2d_nchw.py
[Topi][Float16] Added float16 tests for depthwise conv2d.