unify float comparison helpers with isclose by NitishNaineni · Pull Request #8131 · NVIDIA/cccl

NitishNaineni · 2026-03-22T05:50:28Z

Description

replaced 6 different float comparison implementations across cub/thrust/c2h with one isclose function using the PEP-0485 symmetric formula: |a - b| <= max(a_tol, r_tol * max(|a|, |b|))

what changed:

added c2h/include/c2h/isclose.h with the canonical isclose function
rewrote REQUIRE_APPROX_EQ* macros to use isclose
replaced CompareResults float/double in test_util.h
replaced WithinRel calls in check_results.cuh and thread_reduce tests
replaced require_almost_equal in thrust complex tests
replaced hand-rolled inline comparison in segmented scan tests

default tolerance is 1000 * numeric_limits<T>::epsilon() which is derived from the type's precision instead of hardcoded magic numbers. callers can still pass custom tolerances when needed.

left per-algorithm wilkinson tolerance tuning and libcudacxx/cudax test unification for follow-ups.

Checklist

New or existing tests cover these changes.
The documentation is up to date with these changes.

replaced 6 different float comparison implementations (fptest_close, is_about, almost_equal, CompareResults, WithinRel, REQUIRE_APPROX_EQ macros) with one canonical isclose function using the PEP-0485 symmetric formula. closes NVIDIA#7662

copy-pr-bot · 2026-03-22T05:50:33Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

bernhardmgruber · 2026-03-23T07:37:41Z

Thanks for this contribution!

@oleksandr-pavlyk and @fbusato can you take a look at this?

oleksandr-pavlyk · 2026-03-23T13:53:31Z

c2h/include/c2h/isclose.h

@@ -0,0 +1,45 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved.
+// SPDX-License-Identifier: BSD-3


Suggested change

// SPDX-License-Identifier: BSD-3

// SPDX-License-Identifier: BSD-3-Clause

I see that some headers in c2h use BSD-3, but the documented identifier is BSD-3-Clause.

oleksandr-pavlyk · 2026-03-23T14:11:57Z

c2h/include/c2h/isclose.h

+  }
+  else
+  {
+    return a == b;


If isclose is used on a complex type, it would use a == b branch, perhaps unexpectedly to the user. Perhaps it is worth adding static_assert(std::is_integral_v<T>, "Non-integral type using exact comparison");, or perhaps a static assertion that type T does not expose real and imag methods.

i have added an assert

updated documented identifier to BSD-3-Clause Co-authored-by: Oleksandr Pavlyk <21087696+oleksandr-pavlyk@users.noreply.github.com>

oleksandr-pavlyk · 2026-03-23T14:35:58Z

c2h/include/c2h/isclose.h

+{
+  if constexpr (std::is_floating_point_v<T>)
+  {
+    return isclose(a, b, T(1000) * std::numeric_limits<T>::epsilon(), T(0));


I would use powers of 2 here. T( 1 << 10 ) would mean discrepancy of 10 binary ULPs is tolerated.

10 binary units is about half of 23 total explicit mantissa units for single precision floating point numbers, but is all most of 11 total explicit half-precision explicit mantissa units.

Given that std::is_floating_point_v<std::float16_t> is true, perhaps we should use T(1 << 8) as default multiplier instead?

good point, switched to T(1 << 8)

oleksandr-pavlyk · 2026-03-23T14:40:43Z

c2h/include/c2h/check_results.cuh

      auto test_imag     = test_results[i].imag();
-      REQUIRE_THAT(expected_real, Catch::Matchers::WithinRel(test_real));
-      REQUIRE_THAT(expected_imag, Catch::Matchers::WithinRel(test_imag));
+      INFO("index " << i);


There is a lot of INFO added which would litter the output on failure in comparisons of large arrays.

I think it would be useful to save values of isclose checks, and place INFO("index" << i); in the branch executed only if isclose check is not met. REQUIRE would reuse the result of testing.

made INFO conditional on failed check

oleksandr-pavlyk · 2026-03-23T14:44:53Z

cub/test/thread_reduce/catch2_test_thread_reduce.cu

      std::accumulate(h_in_float.begin(), h_in_float.begin() + num_items, operator_identity, std_reduce_op);
    run_thread_reduce_kernel(num_items, d_in, d_out, reduce_op);
-    verify_results(reference_result, float{c2h::host_vector<value_t>(d_out)[0]});
+    float test_result = float{c2h::host_vector<value_t>(d_out)[0]};


Nit:

Suggested change

float test_result = float{c2h::host_vector<value_t>(d_out)[0]};

float test_result{c2h::host_vector<value_t>(d_out)[0]};

oleksandr-pavlyk · 2026-03-23T14:52:07Z

thrust/testing/catch2_test_complex.cu

+  CHECK(isclose(static_cast<double>(a.real()), static_cast<double>(b.real())));
+  CHECK(isclose(static_cast<double>(a.imag()), static_cast<double>(b.imag())));


Why cast to double? The cast results in relative tolerance specific to double precision to be used when comparing complex values with less precise real/imaginary types.

Is anything wrong with checking isclose(a.rea(), b.real()) and isclose(a.imag(), b.imag())?

If the issue is that a common type is needed, perhaps use std::common_type<T1, T2>: https://en.cppreference.com/w/cpp/types/common_type.html

oleksandr-pavlyk · 2026-03-23T14:53:51Z

thrust/testing/catch2_test_complex.cu

 ::cuda::std::enable_if_t<!is_complex<T1> && !is_complex<T2>> require_almost_equal(const T1& a, const T2& b)
 {
-  CHECK(a == Catch::Approx(b).margin(DEFAULT_ABSOLUTE_TOL).epsilon(DEFAULT_RELATIVE_TOL));
+  CHECK(isclose(static_cast<double>(a), static_cast<double>(b)));


Same question here.

oleksandr-pavlyk · 2026-03-23T14:54:50Z

/ok to test 97f7b85

bernhardmgruber · 2026-03-23T15:10:22Z

c2h/include/c2h/isclose.h

+// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved.
+// SPDX-License-Identifier: BSD-3-Clause


Important: Any net-new source file should be under Apache-2.0 WITH LLVM-exception

Suggested change

// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved.

// SPDX-License-Identifier: BSD-3-Clause

// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.

// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

updated documented identifier to Apache-2.0 Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com>

github-actions · 2026-03-23T16:24:40Z

😬 CI Workflow Results

🟥 Finished in 1h 28m: Pass: 79%/287 | Total: 7d 14h | Max: 1h 27m | Hits: 80%/170175

See results here.

…t build

NitishNaineni · 2026-03-23T20:54:11Z

reverted the thrust complex test changes for now. isclose lives in c2h which is only accessible from cub and c/parallel tests. thrust, libcudacxx, and cudax tests can't reach it. is there a shared location where a test utility like this could live so all test frameworks can use it?

NitishNaineni requested review from a team as code owners March 22, 2026 05:50

NitishNaineni requested a review from jrhemstad March 22, 2026 05:50

github-project-automation bot added this to CCCL Mar 22, 2026

github-project-automation bot moved this to Todo in CCCL Mar 22, 2026

cccl-authenticator-app bot moved this from Todo to In Review in CCCL Mar 22, 2026

oleksandr-pavlyk reviewed Mar 23, 2026

View reviewed changes

Update c2h/include/c2h/isclose.h

97f7b85

updated documented identifier to BSD-3-Clause Co-authored-by: Oleksandr Pavlyk <21087696+oleksandr-pavlyk@users.noreply.github.com>

oleksandr-pavlyk reviewed Mar 23, 2026

View reviewed changes

bernhardmgruber reviewed Mar 23, 2026

View reviewed changes

NitishNaineni and others added 5 commits March 23, 2026 10:33

Update c2h/include/c2h/isclose.h

9a658be

updated documented identifier to Apache-2.0 Co-authored-by: Bernhard Manfred Gruber <bernhardmgruber@gmail.com>

add static_assert for unsupported types in isclose

b5fc9a5

use T(1 << 8) as default tolerance multiplier

32b5698

defer INFO formatting to only run on failure

526c801

use brace initialization for float test_result

1130c28

revert thrust complex test changes, isclose not accessible from thrus…

6eb6f00

…t build

fbusato self-requested a review March 23, 2026 16:38

		@@ -0,0 +1,45 @@
		// SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved.
		// SPDX-License-Identifier: BSD-3

	// SPDX-License-Identifier: BSD-3
	// SPDX-License-Identifier: BSD-3-Clause

	float test_result = float{c2h::host_vector<value_t>(d_out)[0]};
	float test_result{c2h::host_vector<value_t>(d_out)[0]};

		CHECK(isclose(static_cast<double>(a.real()), static_cast<double>(b.real())));
		CHECK(isclose(static_cast<double>(a.imag()), static_cast<double>(b.imag())));

Conversation

NitishNaineni commented Mar 22, 2026

Description

Checklist

Uh oh!

copy-pr-bot bot commented Mar 22, 2026

Uh oh!

bernhardmgruber commented Mar 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oleksandr-pavlyk commented Mar 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 23, 2026

😬 CI Workflow Results

🟥 Finished in 1h 28m: Pass: 79%/287 | Total: 7d 14h | Max: 1h 27m | Hits: 80%/170175

Uh oh!

NitishNaineni commented Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants