Vec256 Test cases by quickwritereader · Pull Request #42685 · pytorch/pytorch

quickwritereader · 2020-08-06T17:31:38Z

Testing
Current list:

Notes on tests and testing framework

some math functions are tested within domain range
mostly testing framework randomly tests against std implementation within the domain or within the implementation domain for some math functions.
some functions are tested against the local version. ~~For example, std::round and vector version of round differs. so it was tested against the local version~~
round was tested against pytorch at::native::round_impl. for double type on Vsx vec_round failed for (even)+0 .5 values . it was solved by using vec_rint
~~complex types are not tested~~ After enabling complex testing due to precision and domain some of the complex functions failed for vsx and x86 avx as well. I will either test it against local implementation or check within the accepted domain
~~quantizations are not tested~~ Added tests for quantizing, dequantize, requantize_from_int, relu, relu6, widening_subtract functions
the testing framework should be improved further
~~For now -DBUILD_MOBILE_TEST=ON will be used for Vec256Test too~~
Vec256 Test cases will be built for each CPU_CAPABILITY

quickwritereader · 2020-08-06T17:49:07Z

For now AVX2 and VSX both fails on these tests

[ FAILED ] InverseTrigonometric/2.Asin, where TypeParam = at::vec256::(anonymous namespace)::Vec256<c10::complex >
[ FAILED ] InverseTrigonometric/2.ACos, where TypeParam = at::vec256::(anonymous namespace)::Vec256<c10::complex >
[ FAILED ] InverseTrigonometric/2.ATan, where TypeParam = at::vec256::(anonymous namespace)::Vec256<c10::complex >
[ FAILED ] InverseTrigonometric/3.Asin, where TypeParam = at::vec256::(anonymous namespace)::Vec256<c10::complex >
[ FAILED ] InverseTrigonometric/3.ACos, where TypeParam = at::vec256::(anonymous namespace)::Vec256<c10::complex >
[ FAILED ] InverseTrigonometric/3.ATan, where TypeParam = at::vec256::(anonymous namespace)::Vec256<c10::complex >

Additionally VSX fails on Multiplication because of precision
[ FAILED ] Arithmetics/2.Multiplication, where TypeParam = at::vec256::(anonymous namespace)::Vec256<c10::complex >

Failures above are precision related and tests can be checked within the domain and with low precision.
But ATan for both VSX and AVX has sign related error. As VSX complex is simply a translation of AVX codes for complex numbers:

[ RUN      ] InverseTrigonometric/2.ATan
Total Trial Count:131070
Domain:
{ -10, 10 }
Error epsilon: 1e-05
../../../aten/src/ATen/test/Vec256Test.h:491: Failure
Expected equality of these values:
  nearlyEqual(exp.real(), act.real(), absErr.real())
    Which is: false
  true
-1.5707963705062866 1.5707963705062866
atan: {
vec[(-5.22354,-4), (-4.52348,2.63532), (-5.75149,0.258563), (-0,-1.43798)]
vec_exp:vec[(-1.44969,-0.0913255), (-1.40576,0.0938578), (-1.39898,0.00757274), (-1.5708,-0.858373)]
vec_act:vec[(-1.44969,-0.0913255), (-1.40576,0.0938578), (-1.39898,0.00757276), (1.5708,-0.858373)]
}
[  FAILED  ] InverseTrigonometric/2.ATan, where TypeParam = at::vec256::(anonymous namespace)::Vec256<c10::complex<float> > (34 ms)
[----------] 1 test from InverseTrigonometric/2 (34 ms total)

[----------] 1 test from InverseTrigonometric/3, where TypeParam = at::vec256::(anonymous namespace)::Vec256<c10::complex<double> >
[ RUN      ] InverseTrigonometric/3.ATan
Total Trial Count:131070
Domain:
{ -10, 10 }
Error epsilon: 1e-05
../../../aten/src/ATen/test/Vec256Test.h:460: Failure
Expected equality of these values:
  nearlyEqual(exp.real(), act.real(), absErr.real())
    Which is: false
  true
1.5707963267948966 -1.5707963267948966
atan: {
vec[(-5,2.47906), (0,6.06085)]
vec_exp:vec[(-1.41065,0.0777398), (1.5708,0.166516)]
vec_act:vec[(-1.41065,0.0777398), (-1.5708,0.166516)]
}
[  FAILED  ] InverseTrigonometric/3.ATan, where TypeParam = at::vec256::(anonymous namespace)::Vec256<c10::complex<double> > (1 ms)

dr-ci · 2020-08-07T07:44:51Z

💊 CI failures summary and remediations

As of commit 14e2ef9 (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 153 times.

ezyang · 2020-08-07T16:20:12Z

@colesbury @VitalyFedyunin let me know if you need help finding other people to review

VitalyFedyunin · 2020-08-09T00:08:50Z

Thank you for separating and valuable find about precision, I will make review my highest priority at Monday.

Vec256Test.h are refactored

facebook-github-bot

@VitalyFedyunin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

VitalyFedyunin

Overall test structure and testing approach looks good to me, but it will take time to review tests one by one.

…st isn't supported

glaringlee

@quickwritereader @VitalyFedyunin
One thing want to mention here, I remembered that the TYPED_TEST_CASE is going to be deprecated and TYPED_TEST_SUITE is the new one to use, I believe no syntax change. I think we should use TYPED_TEST_SUITE if possible.

I will start to review the tests within this code tomorrow.

quickwritereader · 2020-08-12T11:25:04Z

@glaringlee I wanted to rename it, but gtest in the third_party folder is old

glaringlee

@quickwritereader
Sry for updating late, the code is a bit long here. I made some comments.
Let's keep TYPED_TEST_CASE for now (upgrading the googletest version for pytorch is not that trivial within facebook)

@VitalyFedyunin
std::cout is used within the test, should we comment all the std::cout lines?

glaringlee · 2020-09-10T15:58:59Z

@glaringlee thanks for clarifying. So maybe I should add those avx checks inside test too.
But still its strange as how cmake detects avx availability then.
I will see maybe I messed something

I don't think you need to add those AVX checks in the test. FB internal actually use a different mechanism to build pytorch, not CMake (It is similar to bazel but not the same, You can see some TARGET files, those are used internally). Pytorch rely on preprocessor defined macro to detected AVX (CPU_CAPABILITY_DEFAULT/VSX/AVX2 etc), and those flags are all undefined in fb internal test. If no such flags, all vec functions will fall back into vec256_base.h which uses std instead. This is as designed. So once we fix those overflow/precision loss issue in local_xxxx. I think we will be able to pass all the tests.

quickwritereader · 2020-09-10T16:05:29Z

I will try to compare against std then with the decreased domain.
But for asin acos It would be really tough to do. but I will see

glaringlee · 2020-09-10T16:11:55Z

I will try to compare against std then with the decreased domain.
But for asin acos It would be really tough to do. but I will see

Ah, haha, there might be some misunderstanding here. I think you don't need to decrease your domain.
How about change the local_abs and local_sqrt like what I did in my godbolt example? https://godbolt.org/z/f4xvn9
The problem here is overflow and precision loss due to floating type is not large enough, so how about use long double (or just use double and decrease your domain in this case) to store intermediate values (note that float * float will blow up float if input float is big, and samething for double)? And I think this will fix the errors in all the failures (abs, acos, asin, etc.)

quickwritereader · 2020-09-10T16:16:09Z

well, you replicating std behavior. which I chose to tests against local Pytorch complex avx behavior to make tests correct (excluding gcc fp contract mess)

quickwritereader · 2020-09-10T16:17:51Z

I can fix inf vs big number easily actually but not asin acos. for that I need to decrease precision

glaringlee · 2020-09-10T16:18:27Z

I can fix inf vs big number easily actually but not asin acos. for that I need to decrease precision

Ah, got u. Agree!

quickwritereader · 2020-09-14T21:18:48Z

@glaringlee I will try to add ifdef block to fallback when CPU_CAPABILITY_* are not defined.
what do you think something like this could fix internal build tests?

#if defined(CPU_CAPABILITY_DEFAULT) || defined(_MSC_VER)
#define TEST_AGAINST_DEFAULT 1
#elif !defined(CPU_CAPABILITY_AVX) &&  !defined(CPU_CAPABILITY_AVX2) && !defined(CPU_CAPABILITY_VSX)
#define TEST_AGAINST_DEFAULT 1
#else
#undef TEST_AGAINST_DEFAULT
#endif

…ts when CPU_CAPABILITY_* is missing

facebook-github-bot

@glaringlee has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

glaringlee · 2020-09-15T03:46:37Z

@glaringlee I will try to add ifdef block to fallback when CPU_CAPABILITY_* are not defined.
what do you think something like this could fix internal build tests?
#if defined(CPU_CAPABILITY_DEFAULT) || defined(_MSC_VER)
#define TEST_AGAINST_DEFAULT 1
#elif !defined(CPU_CAPABILITY_AVX) &&  !defined(CPU_CAPABILITY_AVX2) && !defined(CPU_CAPABILITY_VSX)
#define TEST_AGAINST_DEFAULT 1
#else
#undef TEST_AGAINST_DEFAULT
#endif

@quickwritereader
sry, off the work earlier today. Did not see ur update till now. I imported this change to fb internal just now. I think it should work. Let's see. Just wondering what is the time difference we have, so I can pay more attention to this in the right time, I am in EST time zone, how about you?

quickwritereader · 2020-09-15T05:42:51Z

GMT+4

glaringlee · 2020-09-15T14:26:52Z

@quickwritereader This worked in fb internal. All test passed. I am running a github CI test now. #44712
Will update here later.

Just have one question here:
https://github.com/pytorch/pytorch/pull/42685/files?file-filters%5B%5D=.cmake&file-filters%5B%5D=.h&file-filters%5B%5D=.txt#diff-c218972661375812ff646eaec7b7ddd7R1118
(The file might be folded, this is in vec256_test_all_types.h line 1118)

Now above piece will be hit only when CPU_CAPABILITY_DEFAULT is defined and AVX/AVX2 macros are not defined under Clang or GNU. And T rr = real * real; can still cause overflow when real is a big number in its type (eg. float, and real = 1e+38, etc). Are we sure this won't be the problem anymore?

quickwritereader · 2020-09-15T15:49:07Z

@glaringlee
hi, I'm glad it seems working.
for the abs operation, it checks relatively. so for cases inf vs big number it should return equal. So I believe there should not be any problem.
As human works are error-prone I will try to keep an eye on it if there happens any issue.

glaringlee · 2020-09-15T16:04:34Z

@glaringlee
hi, I'm glad it seems working.
for the abs operation, it checks relatively. so for cases inf vs big number it should return equal. So I believe there should not be any problem.
As human works are error-prone I will try to keep an eye on it if there happens any issue.

Understood! Thanks a lot. Github CI test is still running, so far so good.

glaringlee

@quickwritereader Github CI finished. All passed. I think we are good now. I'll go ahead to land this.
Thank you so much for working on this huge test!

facebook-github-bot

@glaringlee has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: [Tests for Vec256 classes https://github.com/pytorch/pytorch/issues/15676](https://github.com/pytorch/pytorch/issues/15676) Testing Current list: - [x] Blends - [x] Memory: UnAlignedLoadStore - [x] Arithmetics: Plus,Minu,Multiplication,Division - [x] Bitwise: BitAnd, BitOr, BitXor - [x] Comparison: Equal, NotEqual, Greater, Less, GreaterEqual, LessEqual - [x] MinMax: Minimum, Maximum, ClampMin, ClampMax, Clamp - [x] SignManipulation: Absolute, Negate - [x] Interleave: Interleave, DeInterleave - [x] Rounding: Round, Ceil, Floor, Trunc - [x] Mask: ZeroMask - [x] SqrtAndReciprocal: Sqrt, RSqrt, Reciprocal - [x] Trigonometric: Sin, Cos, Tan - [x] Hyperbolic: Tanh, Sinh, Cosh - [x] InverseTrigonometric: Asin, ACos, ATan, ATan2 - [x] Logarithm: Log, Log2, Log10, Log1p - [x] Exponents: Exp, Expm1 - [x] ErrorFunctions: Erf, Erfc, Erfinv - [x] Pow: Pow - [x] LGamma: LGamma - [x] Quantization: quantize, dequantize, requantize_from_int - [x] Quantization: widening_subtract, relu, relu6 Missing: - [ ] Constructors, initializations - [ ] Conversion , Cast - [ ] Additional: imag, conj, angle (note: imag and conj only checked for float complex) #### Notes on tests and testing framework - some math functions are tested within domain range - mostly testing framework randomly tests against std implementation within the domain or within the implementation domain for some math functions. - some functions are tested against the local version. ~~For example, std::round and vector version of round differs. so it was tested against the local version~~ - round was tested against pytorch at::native::round_impl. ~~for double type on **Vsx vec_round failed for (even)+0 .5 values**~~ . it was solved by using vec_rint - ~~**complex types are not tested**~~ **After enabling complex testing due to precision and domain some of the complex functions failed for vsx and x86 avx as well. I will either test it against local implementation or check within the accepted domain** - ~~quantizations are not tested~~ Added tests for quantizing, dequantize, requantize_from_int, relu, relu6, widening_subtract functions - the testing framework should be improved further - ~~For now `-DBUILD_MOBILE_TEST=ON `will be used for Vec256Test too~~ Vec256 Test cases will be built for each CPU_CAPABILITY Fixes: #15676 Pull Request resolved: #42685 Reviewed By: malfet Differential Revision: D23034406 Pulled By: glaringlee fbshipit-source-id: d1bf03acdfa271c88744c5d0235eeb8b77288ef8

This is to add vec256 test (introduced in #42685) into linux CI system. The whole test will last 50 to 60 seconds. Differential Revision: [D23772923](https://our.internmc.facebook.com/intern/diff/D23772923) [ghstack-poisoned]

Summary: [Tests for Vec256 classes https://github.com/pytorch/pytorch/issues/15676](https://github.com/pytorch/pytorch/issues/15676) Testing Current list: - [x] Blends - [x] Memory: UnAlignedLoadStore - [x] Arithmetics: Plus,Minu,Multiplication,Division - [x] Bitwise: BitAnd, BitOr, BitXor - [x] Comparison: Equal, NotEqual, Greater, Less, GreaterEqual, LessEqual - [x] MinMax: Minimum, Maximum, ClampMin, ClampMax, Clamp - [x] SignManipulation: Absolute, Negate - [x] Interleave: Interleave, DeInterleave - [x] Rounding: Round, Ceil, Floor, Trunc - [x] Mask: ZeroMask - [x] SqrtAndReciprocal: Sqrt, RSqrt, Reciprocal - [x] Trigonometric: Sin, Cos, Tan - [x] Hyperbolic: Tanh, Sinh, Cosh - [x] InverseTrigonometric: Asin, ACos, ATan, ATan2 - [x] Logarithm: Log, Log2, Log10, Log1p - [x] Exponents: Exp, Expm1 - [x] ErrorFunctions: Erf, Erfc, Erfinv - [x] Pow: Pow - [x] LGamma: LGamma - [x] Quantization: quantize, dequantize, requantize_from_int - [x] Quantization: widening_subtract, relu, relu6 Missing: - [ ] Constructors, initializations - [ ] Conversion , Cast - [ ] Additional: imag, conj, angle (note: imag and conj only checked for float complex) #### Notes on tests and testing framework - some math functions are tested within domain range - mostly testing framework randomly tests against std implementation within the domain or within the implementation domain for some math functions. - some functions are tested against the local version. ~~For example, std::round and vector version of round differs. so it was tested against the local version~~ - round was tested against pytorch at::native::round_impl. ~~for double type on **Vsx vec_round failed for (even)+0 .5 values**~~ . it was solved by using vec_rint - ~~**complex types are not tested**~~ **After enabling complex testing due to precision and domain some of the complex functions failed for vsx and x86 avx as well. I will either test it against local implementation or check within the accepted domain** - ~~quantizations are not tested~~ Added tests for quantizing, dequantize, requantize_from_int, relu, relu6, widening_subtract functions - the testing framework should be improved further - ~~For now `-DBUILD_MOBILE_TEST=ON `will be used for Vec256Test too~~ Vec256 Test cases will be built for each CPU_CAPABILITY Fixes: pytorch#15676 Pull Request resolved: pytorch#42685 Reviewed By: malfet Differential Revision: D23034406 Pulled By: glaringlee fbshipit-source-id: d1bf03acdfa271c88744c5d0235eeb8b77288ef8

Vec256 Test cases

82bde5e

quickwritereader mentioned this pull request Aug 6, 2020

Vsx initial support issue27678 #41541

Closed

24 tasks

pytorchbot added the open source label Aug 6, 2020

ezyang requested review from VitalyFedyunin and colesbury August 7, 2020 16:19

ezyang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 7, 2020

VitalyFedyunin reviewed Aug 9, 2020

View reviewed changes

Comment thread caffe2/CMakeLists.txt Outdated

quickwritereader added 2 commits August 9, 2020 18:40

Vec256Test: Complex multiplication tested against local implementations.

d7bb19d

Vec256Test.h are refactored

Vec256Test: Vec256 tests were moved into the BUILD_TEST section

2478977

quickwritereader requested a review from VitalyFedyunin August 9, 2020 16:18

vec256Tests: addressed some compiler build failers

f0328cd

quickwritereader force-pushed the vec256_test_issue15676 branch from 5c1eff7 to f0328cd Compare August 10, 2020 10:21

facebook-github-bot reviewed Aug 10, 2020

View reviewed changes

VitalyFedyunin reviewed Aug 10, 2020

View reviewed changes

Comment thread aten/src/ATen/test/Vec256Test.cpp Outdated

VitalyFedyunin reviewed Aug 10, 2020

View reviewed changes

Comment thread aten/src/ATen/test/Vec256Test.cpp Outdated

Comment thread aten/src/ATen/test/Vec256Test.cpp Outdated

Comment thread aten/src/ATen/test/Vec256Test.cpp Outdated

Comment thread aten/src/ATen/test/Vec256Test.cpp Outdated

VitalyFedyunin requested a review from glaringlee August 11, 2020 17:13

VitalyFedyunin added the module: vectorization Related to SIMD vectorization, e.g., Vec256 label Aug 11, 2020

quickwritereader added 2 commits August 12, 2020 02:06

Vec256Test: test seed was added, outputs compiler error when typed te…

c88f741

…st isn't supported

typo

85c9a45

quickwritereader requested a review from VitalyFedyunin August 11, 2020 22:36

Vec256Test was renamed as vec256_test_all_types

cc512b3

glaringlee reviewed Aug 12, 2020

View reviewed changes

Vec256 Test: minor fixes

23fa1e3

glaringlee requested changes Aug 13, 2020

View reviewed changes

vec256 test: minor fix#2

f70a0aa

quickwritereader added 4 commits September 15, 2020 01:51

merge

2b4dbac

Merge branch 'pytorch-master' into vec256_test_issue15676

9ca4d4b

vec256: decrease default tolerance for float. redirect to default tes…

ccccc38

…ts when CPU_CAPABILITY_* is missing

typo fix

14e2ef9

facebook-github-bot reviewed Sep 15, 2020

View reviewed changes

glaringlee approved these changes Sep 15, 2020

View reviewed changes

facebook-github-bot reviewed Sep 16, 2020

View reviewed changes

facebook-github-bot closed this in 6954ae1 Sep 16, 2020

glaringlee mentioned this pull request Sep 17, 2020

Add vec256 test into linux CI tests #44912

Closed

ezyang added the merged label Sep 28, 2020

Conversation

quickwritereader commented Aug 6, 2020 • edited by VitalyFedyunin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes on tests and testing framework

Uh oh!

quickwritereader commented Aug 6, 2020

Uh oh!

dr-ci Bot commented Aug 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Uh oh!

ezyang commented Aug 7, 2020

Uh oh!

VitalyFedyunin commented Aug 9, 2020

Uh oh!

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

VitalyFedyunin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

glaringlee left a comment

Choose a reason for hiding this comment

Uh oh!

quickwritereader commented Aug 12, 2020

Uh oh!

glaringlee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

glaringlee commented Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quickwritereader commented Sep 10, 2020

Uh oh!

glaringlee commented Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quickwritereader commented Sep 10, 2020

Uh oh!

quickwritereader commented Sep 10, 2020

Uh oh!

glaringlee commented Sep 10, 2020

Uh oh!

quickwritereader commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

glaringlee commented Sep 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quickwritereader commented Sep 15, 2020

Uh oh!

glaringlee commented Sep 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quickwritereader commented Sep 15, 2020

Uh oh!

glaringlee commented Sep 15, 2020

Uh oh!

glaringlee left a comment

Choose a reason for hiding this comment

Uh oh!

quickwritereader commented Aug 6, 2020 •

edited by VitalyFedyunin

Loading

dr-ci Bot commented Aug 7, 2020 •

edited

Loading

glaringlee commented Sep 10, 2020 •

edited

Loading

glaringlee commented Sep 10, 2020 •

edited

Loading

quickwritereader commented Sep 14, 2020 •

edited

Loading

glaringlee commented Sep 15, 2020 •

edited

Loading

glaringlee commented Sep 15, 2020 •

edited

Loading