Add test utility functions to compare af::arrays #2249
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mark-poscablo
force-pushed
the
add-af-asserts
branch
from
8095946
to
0eca39e
Compare
|
Now that the basic functionalities of the new test utilities are working, I'm going to try using them in several existing tests we have so far |
- Create the ASSERT_ARRAY_EQ macro to compare two af::array or af_array - Tests types, dims, and values. - Prints the index of the values that are different. - Prints a more readable error message
- Added type comparison in array-vector assert compare. - Changed array and vector names for more clarity. - Changed C API assert to not free C af_array after assert
mark-poscablo
force-pushed
the
add-af-asserts
branch
from
ca4b014
to
a392e8e
Compare
umar456
reviewed
Jul 31, 2018
test/basic.cpp
Outdated
| } | ||
|
|
||
| TEST(Assert, TestEqualsDiffValue) { | ||
| // array A = af::randu(3, 3, 3); |
test/basic.cpp
Outdated
| } | ||
|
|
||
| TEST(Assert, TestEqualsDiffComplexValue) { | ||
| // array A = af::randu(3, 3, 3); |
test/basic.cpp
Outdated
| } | ||
|
|
||
| TEST(Assert, TestVectorDiffVecSize) { | ||
| // array A = af::randu(3, 3, 3); |
test/basic.cpp
Outdated
| } | ||
|
|
||
| TEST(Assert, TestVectorDiffDim4) { | ||
| // array A = af::randu(3, 3, 3); |
test/testHelpers.hpp
Outdated
| return os << name; | ||
| } | ||
|
|
||
| af::dim4 unravelIdx(uint idx, af::dim4 dims, af::dim4 strides) { |
There was a problem hiding this comment.
A comment here explaining the function would be nice.
- Changed ASSERT_EQ(AF_SUCCESS, ...) to ASSERT_SUCCESS(...) - Changed ASSERT_TRUE(allTrue<bool>...) to ASSERT_ARRAY_EQ() in tests
mark-poscablo
force-pushed
the
add-af-asserts
branch
from
ec22fe7
to
3a34907
Compare
umar456
changed the title
umar456
approved these changes
Jul 31, 2018
This was referenced Aug 2, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A lot of tests have common code for gtest asserts. One of those common code is comparing two arrays for equality. The usual method is to get the raw data from the backend to the host and then iterate over the two host arrays for element-wise comparison. A new utility function would simplify that so that we could just pass in the af::arrays directly. It will:
for comparing elements, display an n-dimensional window around the mismatched elementsfor floating-point types, use a threshold of machine error to the equality operator or let the test-writer to define that thresholdfor images, provide an option to save/display the difference between the reference and output images(Strikethroughed items will be done in a separate PR)
Another utility function would be to simplify the assert for error code-returning C arrayfire calls. Instead of displaying the error code in its raw numeric form, the utility function would display the enum name for that error code, thereby making the error message more human-readable. (done)
p.s. I modified
test/basic.cppfor now to try out how the error messages come out.The tests will failThe tests should pass in the CI since we useASSERT_FALSE()on the utility function within the test cases, but when one uses the actual new macro, it should fail and give back an easy-to-read error message.