[FEA]: Create `CUPROJ_EXPECT_VECTORS_EQUIVALENT` for cuproj testing
Version
cuspatial-branch-24.12
On which installation method(s) does this occur?
Source
Describe the issue
CUSPATIAL_EXPECT_VECTORS_EQUIVALENT reports vectors are different even when difference is smaller than tolerance (3rd argument). In the example the differences are 0.1 and the tolerance is 10.0
Minimum reproducible example
see attached zip file.
T tolerance = std::is_same_v<T, double> ? T{1e-6} : T{10};
cout << "tolerance: " << tolerance << endl;
thrust::device_vector<cuproj::vec_2d<T>> expected(4);
thrust::device_vector<cuproj::vec_2d<T>> d_out(4);
for(int i = 0; i < 4; i++){
expected[i] = {float(1.1+i),float(2.1+i)};
d_out[i] = {float(1.2+i),float(2.2+i)};
}
CUSPATIAL_EXPECT_VECTORS_EQUIVALENT(expected, d_out, tolerance);
Relevant log output
see attached zip file
tolerance: 10
../cuspatial/cpp/include/cuspatial_test/vector_equality.hpp:202: Failure
Expected equality of these values:
lhs
Which is: { (1.1,2.1), (2.1,3.1), (3.1,4.1), (4.1,5.1) }
rhs
Which is: { (1.2,2.2), (2.2,3.2), (3.2,4.2), (4.2,5.2) }
Environment details
see attached zip file
Other/Misc.
bug_expect_vector_equivalent.zip
Anticipate problem is in nested if constexpr in inline void expect_vector_equivalent(Vector1 const& lhs, Vector2 const& rhs, U abs_error) in vector_equality.hpp
Hi @wblangdon!
Thanks for submitting this issue - our team has been notified and we'll get back to you as soon as we can! In the mean time, feel free to add any relevant information to this issue.
This is because cuspatial's test suite does not yet work with cuproj::vec_2d<T> yet. If you change the vector element type from cuproj::vec_2d<T> to cuspatial::vec_2d<T>, it should work.
Yip that has worked.-) New code
T tolerance = std::is_same_v<T, double> ? T{1e-6} : T{10}; cout << "tolerance: " << tolerance << endl; thrust::device_vector<cuspatial::vec_2d<T>> expected(4); thrust::device_vector<cuspatial::vec_2d<T>> d_out(4); for(int i = 0; i < 4; i++){ expected[i] = {float(1.1+i),float(2.1+i)}; d_out[i] = {float(1.2+i),float(2.2+i)}; } CUSPATIAL_EXPECT_VECTORS_EQUIVALENT(expected, d_out, tolerance); if I reduce tolerance the difference is reported ok tolerance = tolerance/1000; cout << "tolerance: " << tolerance << endl; CUSPATIAL_EXPECT_VECTORS_EQUIVALENT(expected, d_out, tolerance);
expect_vector_equivalent.bat Cuda compilation tools, release 12.6, V12.6.68
-rw------- 1 ucacbbl crest 916112 Dec 31 11:36 expect_vector_equivalent.o -rwx------ 1 ucacbbl crest 989912 Dec 31 11:36 expect_vector_equivalent expect_vector_equivalent.bat $Revision: 1.00 $ status 0 done Tue Dec 31 11:36:43 AM GMT 2024 37 ucacbbl@whitebait-l% expect_vector_equivalent Start $Revision: 1.00 $ tolerance: 10 tolerance: 0.01 ../cuspatial/cpp/include/cuspatial_test/vector_equality.hpp:186: Failure Value of: to_host<T>(lhs) Expected: contains 4 values, where each value and its corresponding value in { (1.2,2.2), (2.2,3.2), (3.2,4.2), (4.2,5.2) } are approximately equal vec_2d structs Actual: { (1.1,2.1), (2.1,3.1), (3.1,4.1), (4.1,5.1) }, where the value pair ((1.1,2.1), (1.2,2.2)) at index #0 don't match, (1,2) != (1,2) Google Test trace: expect_vector_equivalent.cu:53: <-- line of failure
end main().
BTW despite the use of thrust::device_vector CUSPATIAL_EXPECT_VECTORS_EQUIVALENT is much slower than doing the proj->transform
ps: I have put my code on GitHub https://github.com/wblangdon/cuproj_example
@wblangdon can this be closed now?
CUSPATIAL_EXPECT_VECTORS_EQUIVALENT contains multiple type checks. It would be nice if either it worked with cuproj or it also included a check that its arguments were indeed of type cuspatial::vec_2d<T>
Thanks Bill
I think it's ok for cuspatial-testutils to depend on cuproj. It's just not the other way around, cuproj be an independent header only library. For which I updated the name of the issue as a feature request.
I think there is use for cuProj outside of cuSpatial so we should actually prepare it to be separated (as time permits). So really it should have its own test utilities (e.g. CUPROJ_EXPECT_VECTORS_EQUIVALENT). See #1312 .
Agreed - updated title.