cuspatial icon indicating copy to clipboard operation
cuspatial copied to clipboard

[FEA]: Create `CUPROJ_EXPECT_VECTORS_EQUIVALENT` for cuproj testing

Open wblangdon opened this issue 11 months ago • 8 comments

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

wblangdon avatar Dec 30 '24 16:12 wblangdon

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.

GPUtester avatar Dec 30 '24 16:12 GPUtester

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.

isVoid avatar Dec 31 '24 08:12 isVoid

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 avatar Dec 31 '24 13:12 wblangdon

@wblangdon can this be closed now?

harrism avatar Jan 08 '25 11:01 harrism

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

wblangdon avatar Jan 08 '25 12:01 wblangdon

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.

isVoid avatar Jan 09 '25 03:01 isVoid

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 .

harrism avatar Jan 09 '25 03:01 harrism

Agreed - updated title.

isVoid avatar Jan 09 '25 04:01 isVoid