Repository navigation
Port thrust complex.cu tests to catch2_test_complex.cu - #6625
bernhardmgruber merged 8 commits into
Conversation
- Port all 15 test structs from complex.cu to catch2_test_complex2.cu - Fix bug in ComplexMemberOperators: change b_thrust vs b_thrust comparison to b_thrust vs b_std (line 385 in original file) - Initialize variable in ComplexStreamOperators to avoid -Wuninitialized
|
/ok to test 90b6333 |
|
@dunga1k58bh thank you for taking a stab at this! This is super helpful! |
This comment has been minimized.
This comment has been minimized.
|
@bernhardmgruber A pull request failed, but I think it's due to an environment configuration error. What should I do now? |
|
/ok to test 3747cf2 |
We sometimes have CI hickups that require us to manually rerun the CI. Sorry about that! Just wait or ping a maintainer if this happens! |
This comment has been minimized.
This comment has been minimized.
|
@bernhardmgruber Failed due to out of memory |
|
I see an OOM error at compile-time: so I guess the compiler hit a limit on the runner. |
|
|
||
| std::stringstream out; | ||
| out << a; | ||
| thrust::complex<T> b(T(0), T(0)); |
There was a problem hiding this comment.
Important: the previous code default constructed b here:
thrust::complex<T> b;please retain this.
Was there a particular reason, this needed to be changed?
There was a problem hiding this comment.
When I use the default constructor, I get a warning due to -Werror=maybe-uninitialized, because some values may not be initialized. Anyway, I changed it to use default initialization instead.
/home/coder/cccl/thrust/testing/catch2_test_complex.cu: In function ‘void CATCH2_INTERNAL_TEMPLATE_TEST_36() [with TestType = float]’:
/home/coder/cccl/thrust/testing/catch2_test_complex.cu:655:22: note: ‘b.thrust::complex<float>::data.thrust::complex<float>::storage::x’ was declared here
655 | thrust::complex<T> b;
| ^
In constructor ‘Catch::Approx::Approx(const T&) [with T = float; <template-parameter-1-2> = void]’,
inlined from ‘cuda::std::__4::enable_if_t<((bool)(is_complex<T1> && is_complex<T2>))> {anonymous}::require_almost_equal(const T1&, const T2&) [with T1 = thrust::complex<float>; T2 = thrust::complex<float>]’ at /home/coder/cccl/thrust/testing/catch2_test_complex.cu:62:3,
inlined from ‘void CATCH2_INTERNAL_TEMPLATE_TEST_36() [with TestType = float]’ at /home/coder/cccl/thrust/testing/catch2_test_complex.cu:657:23:
/home/coder/cccl/build/cuda13.0-gcc14/thrust-cpp20/_deps/catch2-src/src/catch2/catch_approx.hpp:42:77: error: ‘b.thrust::complex<float>::data.thrust::complex<float>::storage::y’ may be used uninitialized [-Werror=maybe-uninitialized]
42 | explicit Approx( T const& value ): Approx(static_cast<double>(value))
| ^
/home/coder/cccl/thrust/testing/catch2_test_complex.cu: In function ‘void CATCH2_INTERNAL_TEMPLATE_TEST_36() [with TestType = float]’:
/home/coder/cccl/thrust/testing/catch2_test_complex.cu:655:22: note: ‘b.thrust::complex<float>::data.thrust::complex<float>::storage::y’ was declared here
655 | thrust::complex<T> b;
| ^
| template <typename T1, typename T2> | ||
| ::cuda::std::enable_if_t<is_complex<T1>::value && is_complex<T2>::value> REQUIRE_ALMOST_EQUAL(const T1& a, const T2& b) | ||
| { | ||
| CHECK(a.real() == Catch::Approx(b.real()).margin(DEFAULT_ABSOLUTE_TOL).epsilon(DEFAULT_RELATIVE_TOL)); | ||
| CHECK(a.imag() == Catch::Approx(b.imag()).margin(DEFAULT_ABSOLUTE_TOL).epsilon(DEFAULT_RELATIVE_TOL)); | ||
| } | ||
|
|
||
| template <typename T1, typename T2> | ||
| ::cuda::std::enable_if_t<!is_complex<T1>::value && !is_complex<T2>::value> REQUIRE_ALMOST_EQUAL(const T1& a, const T2& b) | ||
| { | ||
| CHECK(a == Catch::Approx(b).margin(DEFAULT_ABSOLUTE_TOL).epsilon(DEFAULT_RELATIVE_TOL)); | ||
| } |
There was a problem hiding this comment.
Important: let's use normal function name spelling, so we don't clash if we introduce a macro REQUIRE_ALMOST_EQUAL (which I may plan to do).
| template <typename T1, typename T2> | |
| ::cuda::std::enable_if_t<is_complex<T1>::value && is_complex<T2>::value> REQUIRE_ALMOST_EQUAL(const T1& a, const T2& b) | |
| { | |
| CHECK(a.real() == Catch::Approx(b.real()).margin(DEFAULT_ABSOLUTE_TOL).epsilon(DEFAULT_RELATIVE_TOL)); | |
| CHECK(a.imag() == Catch::Approx(b.imag()).margin(DEFAULT_ABSOLUTE_TOL).epsilon(DEFAULT_RELATIVE_TOL)); | |
| } | |
| template <typename T1, typename T2> | |
| ::cuda::std::enable_if_t<!is_complex<T1>::value && !is_complex<T2>::value> REQUIRE_ALMOST_EQUAL(const T1& a, const T2& b) | |
| { | |
| CHECK(a == Catch::Approx(b).margin(DEFAULT_ABSOLUTE_TOL).epsilon(DEFAULT_RELATIVE_TOL)); | |
| } | |
| template <typename T1, typename T2> | |
| ::cuda::std::enable_if_t<is_complex<T1>::value && is_complex<T2>::value> require_almost_equal(const T1& a, const T2& b) | |
| { | |
| CHECK(a.real() == Catch::Approx(b.real()).margin(DEFAULT_ABSOLUTE_TOL).epsilon(DEFAULT_RELATIVE_TOL)); | |
| CHECK(a.imag() == Catch::Approx(b.imag()).margin(DEFAULT_ABSOLUTE_TOL).epsilon(DEFAULT_RELATIVE_TOL)); | |
| } | |
| template <typename T1, typename T2> | |
| ::cuda::std::enable_if_t<!is_complex<T1>::value && !is_complex<T2>::value> require_almost_equal(const T1& a, const T2& b) | |
| { | |
| CHECK(a == Catch::Approx(b).margin(DEFAULT_ABSOLUTE_TOL).epsilon(DEFAULT_RELATIVE_TOL)); | |
| } |
There was a problem hiding this comment.
I have updated the code based on your suggestion.
This comment has been minimized.
This comment has been minimized.
- Replace custom `is_complex` struct with variable template for modern C++ - Use normal function name spelling `require_almost_equal` - Initialize variable `b` in stream operator test to avoid uninitialized warning This ensures type traits work in both host and device code and follows modern C++ best practices with variable templates.
…a1k58bh/cccl into port-thrust-complex-tests-to-catch2
|
/ok to test 1bfcc1e |
🥳 CI Workflow Results🟩 Finished in 2h 16m: Pass: 100%/70 | Total: 22h 43m | Max: 2h 13m | Hits: 99%/115481See results here. |
bernhardmgruber
left a comment
There was a problem hiding this comment.
Great work, thx!
… to catch2_test_complex.cu (NVIDIA/cccl#6625) Upstream: NVIDIA/cccl@2f82c4d HIP counterpart (system/hip): none. N/A: upstream moves testing/complex.cu to Catch2 testing/catch2_test_complex.cu; rocThrust has no Catch2 harness, so the catch2 file is not added and legacy testing/complex.cu is kept (same coverage). test/ counterpart: test/test_complex.cpp already covers these cases; unchanged. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
… to catch2_test_complex.cu (NVIDIA/cccl#6625) Upstream: NVIDIA/cccl@2f82c4d HIP counterpart (system/hip): none. N/A: upstream moves testing/complex.cu to Catch2 testing/catch2_test_complex.cu; rocThrust has no Catch2 harness, so the catch2 file is not added and legacy testing/complex.cu is kept (same coverage). test/ counterpart: test/test_complex.cpp already covers these cases; unchanged. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
… to catch2_test_complex.cu (NVIDIA/cccl#6625) Upstream: NVIDIA/cccl@2f82c4d HIP counterpart (system/hip): none. N/A: upstream moves testing/complex.cu to Catch2 testing/catch2_test_complex.cu; rocThrust has no Catch2 harness, so the catch2 file is not added and legacy testing/complex.cu is kept (same coverage). test/ counterpart: test/test_complex.cpp already covers these cases; unchanged. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
… to catch2_test_complex.cu (NVIDIA/cccl#6625) Upstream: NVIDIA/cccl@2f82c4d HIP counterpart (system/hip): none. N/A: upstream moves testing/complex.cu to Catch2 testing/catch2_test_complex.cu; rocThrust has no Catch2 harness, so the catch2 file is not added and legacy testing/complex.cu is kept (same coverage). test/ counterpart: test/test_complex.cpp already covers these cases; unchanged. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
… to catch2_test_complex.cu (NVIDIA/cccl#6625) Upstream: NVIDIA/cccl@2f82c4d HIP counterpart (system/hip): none. N/A: upstream moves testing/complex.cu to Catch2 testing/catch2_test_complex.cu; rocThrust has no Catch2 harness, so the catch2 file is not added and legacy testing/complex.cu is kept (same coverage). test/ counterpart: test/test_complex.cpp already covers these cases; unchanged. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
Description
closes #6589
comparison to b_thrust vs b_std (line 385 in original file)
Checklist