cuda::isclose()#9577
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Changescuda::isclose numeric utility
Comment |
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Jacob Faibussowitsch <jacob.fai@gmail.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Jacob Faibussowitsch <jacob.fai@gmail.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🥳 CI Workflow Results🟩 Finished in 1h 39m: Pass: 100%/120 | Total: 1d 20h | Max: 1h 32m | Hits: 92%/399559See results here. |
| template <class Complex, class AbsTol> | ||
| [[nodiscard]] __host__ __device__ | ||
| bool isclose(const Complex& lhs, | ||
| const Complex& rhs, | ||
| float relative_tol, | ||
| AbsTol absolute_tol) noexcept; |
There was a problem hiding this comment.
Suggestion: In non-complex API we require absolute_tol to be of type T, but here it seems like it can have a different type than Complex::value_type.
I think we should rather do:
| template <class Complex, class AbsTol> | |
| [[nodiscard]] __host__ __device__ | |
| bool isclose(const Complex& lhs, | |
| const Complex& rhs, | |
| float relative_tol, | |
| AbsTol absolute_tol) noexcept; | |
| template <class T> | |
| [[nodiscard]] __host__ __device__ | |
| bool isclose(const /*complex-type*/<T>& lhs, | |
| const /*complex-type*/<T>& rhs, | |
| float relative_tol, | |
| T absolute_tol) noexcept; |
What do you think?
|
|
||
| - Scalar overloads require ``lhs``, ``rhs``, ``absolute_tol`` to have the same arithmetic type (integer or floating point). | ||
| - Complex overloads accept ``cuda::std::complex<T>`` and ``std::complex<T>`` operands. | ||
| - ``AbsTol`` must be the same type as the complex value type. |
There was a problem hiding this comment.
Would be gone if you accept my suggestion
| _CCCL_TEMPLATE(typename _ComplexType, typename _AbsTol) | ||
| _CCCL_REQUIRES(__isclose_complex_comparison_v<_ComplexType, _AbsTol>) | ||
| [[nodiscard]] _CCCL_HOST_DEVICE_API bool | ||
| isclose(const _ComplexType& __lhs, const _ComplexType& __rhs, const float __rel_tol, const _AbsTol __abs_tol) noexcept |
There was a problem hiding this comment.
| _CCCL_TEMPLATE(typename _ComplexType, typename _AbsTol) | |
| _CCCL_REQUIRES(__isclose_complex_comparison_v<_ComplexType, _AbsTol>) | |
| [[nodiscard]] _CCCL_HOST_DEVICE_API bool | |
| isclose(const _ComplexType& __lhs, const _ComplexType& __rhs, const float __rel_tol, const _AbsTol __abs_tol) noexcept | |
| _CCCL_TEMPLATE(template <class> class _LhsComplex, template <class> class _RhsComplex, class _Tp) | |
| _CCCL_REQUIRES(::cuda::std::__cccl_is_integer_v<_Tp> || ::cuda::is_floating_point_v<_Tp>) | |
| [[nodiscard]] _CCCL_HOST_DEVICE_API bool | |
| isclose(const _LhsComplex<_Tp>& __lhs, const _RhsComplex<_Tp>& __rhs, const float __rel_tol, const _Tp __abs_tol) noexcept |
Description
Comparing two floating-point values is a widely used functionality. Every library that use floating-point values need to compare them at some point. CCCL used it too for internal testing.
Python has
math.isclose()https://peps.python.org/pep-0485/, andnumpy.isclose.Several C++ testing frameworks and libraries provide a similar utility,
GoogleTest,Catch2,Boost.Math,Eigen.This PR introduces 3 overloads for
isclose()following implementation decisions ofmath.isclose()we could also think to add interfaces based on C++20 designed initializer.
Supported types: integers, (extended) floating-point, complex (std. cuda, cuda::std).
Requires:
__float128forisfinite()andisinf()#9576