Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Unittest tolerance handling improvements #18694

Merged
merged 61 commits into from
Jul 19, 2020
Merged

Conversation

DickJC123
Copy link
Contributor

@DickJC123 DickJC123 commented Jul 11, 2020

Description

This PR moves to consolidate and standardize the way our data comparison routines in test_utils.py handle tolerances. Over time, the number of these routines has grown and the approaches have diverged. For example, two of the most frequently used routines are:

def assert_almost_equal(a, b, rtol=None, atol=None, …):
    rtol = get_rtol(rtol)
    atol = get_atol(atol)
    ….

and

def check_consistency(sym, …,  tol=None, … ):
    if tol is None:
        tol = {np.dtype(np.float16): 1e-1,
               np.dtype(np.float32): 1e-3,
               np.dtype(np.float64): 1e-5,
               …}
…

As show, assert_almost_equal() offers separate arguments for specifying relative and absolute tolerances (by default rtol, atol = 1e-5, 1e-20), but these do not vary with dtype. In contrast, check_consistency() only has one tol argument, which it uses for both atol and rtol, but the tol default is dtype-dependent.

This PR unifies these approaches by having both check_consistency() and assert_almost_equal() support rtol and atol, drawing upon the same set of dtype-dependent defaults. The goal here is for the testing framework to make sensible tolerance choices for our unittest data comparisons when possible, rather than burden each test developer to do so. In effect, this PR addresses the current codebase TODO in test_utils.py:

def get_rtol(rtol=None):
    """Get default numerical threshold for regression test."""
    # _TODO: get from env variable, different threshold might
    # be needed for different device and dtype
    return 1e-5 if rtol is None else rtol

While I believe setting tolerances via env var is not appropriate, test tolerances should be a function of the dtype and device (i.e. context). A first application for this concept is in support for the newly announced A100 GPU with its TensorFloat-32 (TF32) mode of computation. The A100 will by default round the mantissa of float32 GEMM and Convolution inputs to a float16 precision. Therefore, although operator i/o’s might be float32, the “effective dtype” for the purposes of tolerance selection is float16. By consolidating the tolerance handling logic, this PR can now easily incorporate an effective_dtype(dat) routine based on context, so that unittests can run 32-bit models and seamlessly apply the appropriate tolerances for each context, be it A100 or non-A100.

This PR was developed on an in-house CI system that runs on many GPU architectures, including the A100. As part of the PR development, some unittests were modified so that now all tests pass on the A100. These tests had typically explicitly hard-coded a tolerance appropriate for a float32 test. The modification for those tests involved dropping the fixed tolerance, allowing this PR’s adaptive context- and dtype-dependent tolerance selection logic to take over.

Thanks to @Kh4L for supplying the GEMM-flag adaptations for TF32 that are part of this PR.
Thanks also to @drivanov for his prior efforts to adapt assert_almost_equal() to work directly on ndarrays, thereby enabling context-dependent default tolerance selection.

@ptrendx @eric-haibin-lin @marcoabreu

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • [X ] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • [ X] Changes are complete (i.e. I finished coding on this PR)
  • [ X] All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • [ X] Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • [X ] To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

DickJC123 and others added 30 commits July 10, 2020 15:45
Signed-off-by: Serge Panev <spanev@nvidia.com>
Signed-off-by: Serge Panev <spanev@nvidia.com>
Signed-off-by: Serge Panev <spanev@nvidia.com>
Signed-off-by: Serge Panev <spanev@nvidia.com>
Copy link
Contributor

@xidulu xidulu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM regarding the changes to test_gluon_probability

@szha
Copy link
Member

szha commented Jul 19, 2020

@DickJC123 thanks for the change and for fixing the issues you found along the way. Remember to mark the issues this PR resolve in the description.

@szha szha merged commit 146b49e into apache:master Jul 19, 2020
@szha
Copy link
Member

szha commented Jul 19, 2020

Thanks for the fixes, @DickJC123. They are really helpful. I did a code review and examined the analysis included in each of them as well as the specific fixes.

@DickJC123
Copy link
Contributor Author

DickJC123 commented Jul 19, 2020

Thanks @szha! You probably saw that I've been struggling to get a passing CI- running into and fixing many issues unrelated to my PR along the way.

@szha szha added this to the v1.8.0 milestone Aug 23, 2020
DickJC123 added a commit to DickJC123/mxnet that referenced this pull request Sep 15, 2020
* Add sm arch 80 to Makefile

* Add TF32 to cuBLAS GEMMs

Signed-off-by: Serge Panev <spanev@nvidia.com>

* Add CUDA version guards

Signed-off-by: Serge Panev <spanev@nvidia.com>

* Remove useless TF32 for double and old CUDA version

Signed-off-by: Serge Panev <spanev@nvidia.com>

* Factorize VERSION_ADJUSTED_TF32_MATH

Signed-off-by: Serge Panev <spanev@nvidia.com>

* Add TF32 considerations to test_util.py:check_consistency()

* Bypass test_gluon_gpu.py:test_large_models if gmem >32GB

* Default tols in assert_almost_equal() now a function of dtype and ctx

* Expand types listed by default_tols()

* Fix pylint

* All with_seed() tests to waitall in teardown

* Elevate MXNET_TEST_SEED logging to WARNING

* Revert test_gluon_gpu.py:test_rnn_layer to default tols

* Fix test_gluon_model_zoo_gpu.py::test_inference and test_operator_gpy.py::test_np_linalg_{solve,tensorinv}

* test_numpy_interoperability.py to not fix seed for rest of CI

* Further fix to test_np_linalg_tensorinv

* Fix test_gluon_data.py:test_dataloader_context when run on 1-GPU system.

* Fix test_operator_gpu.py::test_embedding_with_type

* Fix test_operator_gpu.py::{test_*convolution_large_c,test_np_linalg_tensorsolve}

* Remove unneeded print() from test_numpy_interoperability.py

* Unify tol handling of check_consistency() and assert_almost_equal().  Test tweeks.

* Add tol handling of assert_almost_equal() with number args

* Add tol handling of bool comparisons

* Fix test_numpy_op.py::test_np_random_rayleigh

* Fix test_operator_gpu.py::test_batchnorm_with_type

* Fix test_gluon.py::test_sync_batchnorm in cpu selftest

* Improve unittest failure reporting

* Add to robustness of test_operator_gpu.py::test_embedding_with_type

* Check_consistency() to use equal backward gradients for increased test robustness

* Fix test_operator_gpu.py::test_{fully_connected,gemm}.  Add default_numeric_eps().

* test_utils.py fix for numeric gradient calc

* Reinstate rtol=1e-2 for test_operator.py::test_order

* Remove auto-cast of check_consistency() input data to least precise dtype (not needed)

* Fix test_operator.py::test_{reciprocol,cbrt,rcbrt}_op

* Expand default float64 numeric_eps for test_operator_gpu.py::test_sofmin

* Fix segfault-on-error of @Retry decorator. Add test isolation.

* assert_almost_equal() to handle a,b scalars

* Fix test_operator_gpu.py::test_gluon_{mvn,mvn_v1} race

* Fix test_operator_gpu.py::test_flatten_slice_after_conv via scale

* Remove test_utils.py:almost_equal_ignore_nan()

* Fix sample vs. pop variance issue with test_numpy_op.py::test_npx_batch_norm

* Expose test_utils.py:effective_dtype() and use to fix test_operator_gpu.py::test_np_linalg_svd

* Fix true_divide int_array / int_scalar -> float_array to honor np_default_dtype

* Try test_elemwise_binary_ops serial to avoid pytest worker crash

* Fix (log_)softmax backward on empty ndarray

* Temporarily log all CI seeds to troubleshoot seed non-determinism

* Revert "Temporarily log all CI seeds to troubleshoot seed non-determinism"

This reverts commit f60eff2.

* Temp log all CI seeds to troubleshoot unwanted seed determinism

* Revert "Add sm arch 80 to Makefile"

This reverts commit f9306ce.

* Same fix of sample vs. pop variance issue, now with test_operator_gpu.py::test_batchnorm

* Revert "Temp log all CI seeds to troubleshoot unwanted seed determinism"

This reverts commit ff328ef.

* Marking test_sparse_dot_grad with garbage_expected after teardown error

* Fix flakiness of test_gluon_probability{_v1,_v2}.py::test_gluon_kl{_v1,}

* Temp skip of test_aggregate_duplication on gpu

* Add seeding to test_{numpy,}_contrib_gluon_data_vision.py.  Make created files unique.

* Add ndarray module isolation to help debug test_bbox_augmenters worker crash

* Marking test_sparse_square_sum serial after pytest worker crash

* Fix flakiness of test_gluon_probability{_v1,_v2}.py::test_half_cauchy{_v1,}

Co-authored-by: Serge Panev <spanev@nvidia.com>
Co-authored-by: Bart Gawrych <gawrych.bartlomiej@intel.com>
szha pushed a commit that referenced this pull request Sep 17, 2020
…so test seeding (#18762). (#19148)

* Add sm arch 80 to Makefile

* Unittest tolerance handling improvements (#18694)

* Add sm arch 80 to Makefile

* Add TF32 to cuBLAS GEMMs

Signed-off-by: Serge Panev <spanev@nvidia.com>

* Add CUDA version guards

Signed-off-by: Serge Panev <spanev@nvidia.com>

* Remove useless TF32 for double and old CUDA version

Signed-off-by: Serge Panev <spanev@nvidia.com>

* Factorize VERSION_ADJUSTED_TF32_MATH

Signed-off-by: Serge Panev <spanev@nvidia.com>

* Add TF32 considerations to test_util.py:check_consistency()

* Bypass test_gluon_gpu.py:test_large_models if gmem >32GB

* Default tols in assert_almost_equal() now a function of dtype and ctx

* Expand types listed by default_tols()

* Fix pylint

* All with_seed() tests to waitall in teardown

* Elevate MXNET_TEST_SEED logging to WARNING

* Revert test_gluon_gpu.py:test_rnn_layer to default tols

* Fix test_gluon_model_zoo_gpu.py::test_inference and test_operator_gpy.py::test_np_linalg_{solve,tensorinv}

* test_numpy_interoperability.py to not fix seed for rest of CI

* Further fix to test_np_linalg_tensorinv

* Fix test_gluon_data.py:test_dataloader_context when run on 1-GPU system.

* Fix test_operator_gpu.py::test_embedding_with_type

* Fix test_operator_gpu.py::{test_*convolution_large_c,test_np_linalg_tensorsolve}

* Remove unneeded print() from test_numpy_interoperability.py

* Unify tol handling of check_consistency() and assert_almost_equal().  Test tweeks.

* Add tol handling of assert_almost_equal() with number args

* Add tol handling of bool comparisons

* Fix test_numpy_op.py::test_np_random_rayleigh

* Fix test_operator_gpu.py::test_batchnorm_with_type

* Fix test_gluon.py::test_sync_batchnorm in cpu selftest

* Improve unittest failure reporting

* Add to robustness of test_operator_gpu.py::test_embedding_with_type

* Check_consistency() to use equal backward gradients for increased test robustness

* Fix test_operator_gpu.py::test_{fully_connected,gemm}.  Add default_numeric_eps().

* test_utils.py fix for numeric gradient calc

* Reinstate rtol=1e-2 for test_operator.py::test_order

* Remove auto-cast of check_consistency() input data to least precise dtype (not needed)

* Fix test_operator.py::test_{reciprocol,cbrt,rcbrt}_op

* Expand default float64 numeric_eps for test_operator_gpu.py::test_sofmin

* Fix segfault-on-error of @Retry decorator. Add test isolation.

* assert_almost_equal() to handle a,b scalars

* Fix test_operator_gpu.py::test_gluon_{mvn,mvn_v1} race

* Fix test_operator_gpu.py::test_flatten_slice_after_conv via scale

* Remove test_utils.py:almost_equal_ignore_nan()

* Fix sample vs. pop variance issue with test_numpy_op.py::test_npx_batch_norm

* Expose test_utils.py:effective_dtype() and use to fix test_operator_gpu.py::test_np_linalg_svd

* Fix true_divide int_array / int_scalar -> float_array to honor np_default_dtype

* Try test_elemwise_binary_ops serial to avoid pytest worker crash

* Fix (log_)softmax backward on empty ndarray

* Temporarily log all CI seeds to troubleshoot seed non-determinism

* Revert "Temporarily log all CI seeds to troubleshoot seed non-determinism"

This reverts commit f60eff2.

* Temp log all CI seeds to troubleshoot unwanted seed determinism

* Revert "Add sm arch 80 to Makefile"

This reverts commit f9306ce.

* Same fix of sample vs. pop variance issue, now with test_operator_gpu.py::test_batchnorm

* Revert "Temp log all CI seeds to troubleshoot unwanted seed determinism"

This reverts commit ff328ef.

* Marking test_sparse_dot_grad with garbage_expected after teardown error

* Fix flakiness of test_gluon_probability{_v1,_v2}.py::test_gluon_kl{_v1,}

* Temp skip of test_aggregate_duplication on gpu

* Add seeding to test_{numpy,}_contrib_gluon_data_vision.py.  Make created files unique.

* Add ndarray module isolation to help debug test_bbox_augmenters worker crash

* Marking test_sparse_square_sum serial after pytest worker crash

* Fix flakiness of test_gluon_probability{_v1,_v2}.py::test_half_cauchy{_v1,}

Co-authored-by: Serge Panev <spanev@nvidia.com>
Co-authored-by: Bart Gawrych <gawrych.bartlomiej@intel.com>

* Fix test_gluon_data.py:test_dataloader_context when run on 1-GPU system.

* Remove pytest decorators introduced in error

* Fix test_forward.py:test_consistency

* Fix test_numpy_op.py tests

* Improve test seeding in test_numpy_interoperablity.py (#18762)

* Fix test_numpy_op.py:test_np_random_{beta,chisquare}

* Reduce problem sizes with test_optimizer.py:test_multilamb

* Skip test_gluon_gpu.py:test_fused_{lstm,gpu}_layer, fix test_rnn_cells, for fp16 contexts

* Trigger CI

Co-authored-by: Serge Panev <spanev@nvidia.com>
Co-authored-by: Bart Gawrych <gawrych.bartlomiej@intel.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants