From c549d52196039e9b0a683a87b8b54fbfd4596dd9 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Fri, 23 Aug 2024 09:32:34 -0400 Subject: [PATCH 01/11] add spec compliant API while supporting legacy extension API Signed-off-by: Dan Hoeflinger --- .../dpl/pstl/histogram_extension_defs.h | 12 +++++++-- include/oneapi/dpl/pstl/histogram_impl.h | 25 ++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/include/oneapi/dpl/pstl/histogram_extension_defs.h b/include/oneapi/dpl/pstl/histogram_extension_defs.h index c45c819a62f..94f6cf53f98 100644 --- a/include/oneapi/dpl/pstl/histogram_extension_defs.h +++ b/include/oneapi/dpl/pstl/histogram_extension_defs.h @@ -22,10 +22,18 @@ namespace oneapi { namespace dpl { +template +oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator2> +histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, + typename std::iterator_traits<_RandomAccessIterator1>::value_type first_bin_min_val, + typename std::iterator_traits<_RandomAccessIterator1>::value_type last_bin_max_val, + _RandomAccessIterator2 histogram_first); template ::value_type> -oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator2> + typename _ValueType> +std::enable_if_t> && + !std::is_same_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type>, + _RandomAccessIterator2> histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, _ValueType first_bin_min_val, _ValueType last_bin_max_val, _RandomAccessIterator2 histogram_first); diff --git a/include/oneapi/dpl/pstl/histogram_impl.h b/include/oneapi/dpl/pstl/histogram_impl.h index 362685bd19c..6cf50703da8 100644 --- a/include/oneapi/dpl/pstl/histogram_impl.h +++ b/include/oneapi/dpl/pstl/histogram_impl.h @@ -46,9 +46,32 @@ __pattern_histogram(_Tag, _ExecutionPolicy&& exec, _RandomAccessIterator1 __firs } // namespace __internal +template +oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator2> +histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, + typename std::iterator_traits<_RandomAccessIterator1>::value_type first_bin_min_val, + typename std::iterator_traits<_RandomAccessIterator1>::value_type last_bin_max_val, + _RandomAccessIterator2 histogram_first) +{ + using _BoundaryType = typename std::iterator_traits<_RandomAccessIterator1>::value_type; + const auto __dispatch_tag = oneapi::dpl::__internal::__select_backend(exec, first, histogram_first); + + oneapi::dpl::__internal::__pattern_histogram( + __dispatch_tag, ::std::forward<_ExecutionPolicy>(exec), first, last, num_bins, + oneapi::dpl::__internal::__evenly_divided_binhash<_BoundaryType>(first_bin_min_val, last_bin_max_val, num_bins), + histogram_first); + return histogram_first + num_bins; +} + +// This overload is provided to support an extension to the oneDPL specification to support the original implementation +// of the histogram API, where the boundary type _ValueType could differ from the value type of the input iterator, +// and required `operator<` and `operator<=` to be defined between _ValueType and +// std::iterator_traits<_RandomAccessIterator1>::value_type rather than enforcing they were the same type template -oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator2> +std::enable_if_t> && + !std::is_same_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type>, + _RandomAccessIterator2> histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, _ValueType first_bin_min_val, _ValueType last_bin_max_val, _RandomAccessIterator2 histogram_first) { From c40b3fe5b2ec7b630a812b6f95c6f7108195c602 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Fri, 23 Aug 2024 12:11:27 -0400 Subject: [PATCH 02/11] ::std -> std Signed-off-by: Dan Hoeflinger --- include/oneapi/dpl/pstl/histogram_extension_defs.h | 2 +- include/oneapi/dpl/pstl/histogram_impl.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/oneapi/dpl/pstl/histogram_extension_defs.h b/include/oneapi/dpl/pstl/histogram_extension_defs.h index 94f6cf53f98..5b6304912c0 100644 --- a/include/oneapi/dpl/pstl/histogram_extension_defs.h +++ b/include/oneapi/dpl/pstl/histogram_extension_defs.h @@ -31,7 +31,7 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt template -std::enable_if_t> && +std::enable_if_t> && !std::is_same_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type>, _RandomAccessIterator2> histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, diff --git a/include/oneapi/dpl/pstl/histogram_impl.h b/include/oneapi/dpl/pstl/histogram_impl.h index 6cf50703da8..b7a8b898a61 100644 --- a/include/oneapi/dpl/pstl/histogram_impl.h +++ b/include/oneapi/dpl/pstl/histogram_impl.h @@ -57,7 +57,7 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt const auto __dispatch_tag = oneapi::dpl::__internal::__select_backend(exec, first, histogram_first); oneapi::dpl::__internal::__pattern_histogram( - __dispatch_tag, ::std::forward<_ExecutionPolicy>(exec), first, last, num_bins, + __dispatch_tag, std::forward<_ExecutionPolicy>(exec), first, last, num_bins, oneapi::dpl::__internal::__evenly_divided_binhash<_BoundaryType>(first_bin_min_val, last_bin_max_val, num_bins), histogram_first); return histogram_first + num_bins; @@ -69,7 +69,7 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt // std::iterator_traits<_RandomAccessIterator1>::value_type rather than enforcing they were the same type template -std::enable_if_t> && +std::enable_if_t> && !std::is_same_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type>, _RandomAccessIterator2> histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, @@ -78,7 +78,7 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt const auto __dispatch_tag = oneapi::dpl::__internal::__select_backend(exec, first, histogram_first); oneapi::dpl::__internal::__pattern_histogram( - __dispatch_tag, ::std::forward<_ExecutionPolicy>(exec), first, last, num_bins, + __dispatch_tag, std::forward<_ExecutionPolicy>(exec), first, last, num_bins, oneapi::dpl::__internal::__evenly_divided_binhash<_ValueType>(first_bin_min_val, last_bin_max_val, num_bins), histogram_first); return histogram_first + num_bins; From ba9ece8a697470d251bc419bce3f9056f6ac945a Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 26 Aug 2024 08:57:23 -0400 Subject: [PATCH 03/11] adding required extra unused defaulted template param Signed-off-by: Dan Hoeflinger --- include/oneapi/dpl/pstl/histogram_extension_defs.h | 3 ++- include/oneapi/dpl/pstl/histogram_impl.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/oneapi/dpl/pstl/histogram_extension_defs.h b/include/oneapi/dpl/pstl/histogram_extension_defs.h index 5b6304912c0..31235c00bff 100644 --- a/include/oneapi/dpl/pstl/histogram_extension_defs.h +++ b/include/oneapi/dpl/pstl/histogram_extension_defs.h @@ -22,7 +22,8 @@ namespace oneapi { namespace dpl { -template +template oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator2> histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, typename std::iterator_traits<_RandomAccessIterator1>::value_type first_bin_min_val, diff --git a/include/oneapi/dpl/pstl/histogram_impl.h b/include/oneapi/dpl/pstl/histogram_impl.h index b7a8b898a61..87c22a8894d 100644 --- a/include/oneapi/dpl/pstl/histogram_impl.h +++ b/include/oneapi/dpl/pstl/histogram_impl.h @@ -46,7 +46,8 @@ __pattern_histogram(_Tag, _ExecutionPolicy&& exec, _RandomAccessIterator1 __firs } // namespace __internal -template +template //final unused template param is to avoid breaking change from original API (below) oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator2> histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, typename std::iterator_traits<_RandomAccessIterator1>::value_type first_bin_min_val, From a7dab72563d9e0036ce7795f138f973070e45faa Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 26 Aug 2024 09:00:00 -0400 Subject: [PATCH 04/11] restoring formatting only change Signed-off-by: Dan Hoeflinger --- include/oneapi/dpl/pstl/histogram_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/oneapi/dpl/pstl/histogram_impl.h b/include/oneapi/dpl/pstl/histogram_impl.h index 87c22a8894d..1592436ca9b 100644 --- a/include/oneapi/dpl/pstl/histogram_impl.h +++ b/include/oneapi/dpl/pstl/histogram_impl.h @@ -79,7 +79,7 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt const auto __dispatch_tag = oneapi::dpl::__internal::__select_backend(exec, first, histogram_first); oneapi::dpl::__internal::__pattern_histogram( - __dispatch_tag, std::forward<_ExecutionPolicy>(exec), first, last, num_bins, + __dispatch_tag, ::std::forward<_ExecutionPolicy>(exec), first, last, num_bins, oneapi::dpl::__internal::__evenly_divided_binhash<_ValueType>(first_bin_min_val, last_bin_max_val, num_bins), histogram_first); return histogram_first + num_bins; From d70778886799bb7a66b2fa3dab91f297134f3a72 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Mon, 26 Aug 2024 14:52:17 -0400 Subject: [PATCH 05/11] improving comment language Signed-off-by: Dan Hoeflinger --- include/oneapi/dpl/pstl/histogram_impl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/oneapi/dpl/pstl/histogram_impl.h b/include/oneapi/dpl/pstl/histogram_impl.h index 1592436ca9b..0d22fb191a3 100644 --- a/include/oneapi/dpl/pstl/histogram_impl.h +++ b/include/oneapi/dpl/pstl/histogram_impl.h @@ -47,7 +47,7 @@ __pattern_histogram(_Tag, _ExecutionPolicy&& exec, _RandomAccessIterator1 __firs } // namespace __internal template //final unused template param is to avoid breaking change from original API (below) + typename> //final unused template param support extension API in combination with overload below oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator2> histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, typename std::iterator_traits<_RandomAccessIterator1>::value_type first_bin_min_val, @@ -66,8 +66,8 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt // This overload is provided to support an extension to the oneDPL specification to support the original implementation // of the histogram API, where the boundary type _ValueType could differ from the value type of the input iterator, -// and required `operator<` and `operator<=` to be defined between _ValueType and -// std::iterator_traits<_RandomAccessIterator1>::value_type rather than enforcing they were the same type +// and required `<`, `<=`, `+`, `-`, and `/` to be defined between _ValueType and +// std::iterator_traits<_RandomAccessIterator1>::value_type rather than enforcing they were the same type or convertible template std::enable_if_t> && From fcfaf5998c350354e0138fcb0d3eac2f0488aca8 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Wed, 28 Aug 2024 08:38:36 -0400 Subject: [PATCH 06/11] improved enable_if logic Signed-off-by: Dan Hoeflinger --- .../dpl/pstl/histogram_extension_defs.h | 40 ++++++++++---- include/oneapi/dpl/pstl/histogram_impl.h | 54 ++++++++++++++----- 2 files changed, 70 insertions(+), 24 deletions(-) diff --git a/include/oneapi/dpl/pstl/histogram_extension_defs.h b/include/oneapi/dpl/pstl/histogram_extension_defs.h index 31235c00bff..42b005db796 100644 --- a/include/oneapi/dpl/pstl/histogram_extension_defs.h +++ b/include/oneapi/dpl/pstl/histogram_extension_defs.h @@ -22,22 +22,13 @@ namespace oneapi { namespace dpl { -template +template oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator2> histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, typename std::iterator_traits<_RandomAccessIterator1>::value_type first_bin_min_val, typename std::iterator_traits<_RandomAccessIterator1>::value_type last_bin_max_val, _RandomAccessIterator2 histogram_first); -template -std::enable_if_t> && - !std::is_same_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type>, - _RandomAccessIterator2> -histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, - _ValueType first_bin_min_val, _ValueType last_bin_max_val, _RandomAccessIterator2 histogram_first); - template oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator3> @@ -45,6 +36,35 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt _RandomAccessIterator2 boundary_first, _RandomAccessIterator2 boundary_last, _RandomAccessIterator3 histogram_first); +// Support extention API to cover existing API (previous to specification) with the following two overloads + +// This overload is provided to support an extension to the oneDPL specification to support the original implementation +// of the histogram API, where the boundary type _ValueType could differ from the value type of the input iterator, +// and required `<`, `<=`, `+`, `-`, and `/` to be defined between _ValueType and +// std::iterator_traits<_RandomAccessIterator1>::value_type rather than enforcing they were the same type or convertible +template +std::enable_if_t< + oneapi::dpl::execution::is_execution_policy_v> && + !std::is_convertible_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type>, + _RandomAccessIterator2> +histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, + _ValueType first_bin_min_val, _ValueType last_bin_max_val, _RandomAccessIterator2 histogram_first); + +// This overload is provided to support an extension to the oneDPL specification to support the original implementation +// of the histogram API, where if users explicitly-specify all template arguments, their arguments for bin boundary min +// and max are convertible to the specified _ValueType, and the _ValueType is convertible to the value type of the +// input iterator. +template +std::enable_if_t< + oneapi::dpl::execution::is_execution_policy_v> && + std::is_convertible_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type> && + std::is_convertible_v<_RealValueType1, _ValueType> && std::is_convertible_v<_RealValueType2, _ValueType>, + _RandomAccessIterator2> +histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, + _RealValueType1 first_bin_min_val, _RealValueType2 last_bin_max_val, _RandomAccessIterator2 histogram_first); + } // end namespace dpl } // end namespace oneapi diff --git a/include/oneapi/dpl/pstl/histogram_impl.h b/include/oneapi/dpl/pstl/histogram_impl.h index 0d22fb191a3..fb3e2722208 100644 --- a/include/oneapi/dpl/pstl/histogram_impl.h +++ b/include/oneapi/dpl/pstl/histogram_impl.h @@ -46,8 +46,7 @@ __pattern_histogram(_Tag, _ExecutionPolicy&& exec, _RandomAccessIterator1 __firs } // namespace __internal -template //final unused template param support extension API in combination with overload below +template oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator2> histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, typename std::iterator_traits<_RandomAccessIterator1>::value_type first_bin_min_val, @@ -64,15 +63,34 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt return histogram_first + num_bins; } +template +oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator3> +histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, + _RandomAccessIterator2 boundary_first, _RandomAccessIterator2 boundary_last, + _RandomAccessIterator3 histogram_first) +{ + const auto __dispatch_tag = oneapi::dpl::__internal::__select_backend(exec, first, boundary_first, histogram_first); + + ::std::ptrdiff_t num_bins = boundary_last - boundary_first - 1; + oneapi::dpl::__internal::__pattern_histogram( + __dispatch_tag, ::std::forward<_ExecutionPolicy>(exec), first, last, num_bins, + oneapi::dpl::__internal::__custom_boundary_binhash{boundary_first, boundary_last}, histogram_first); + return histogram_first + num_bins; +} + +// Support extention API to cover existing API (previous to specification) with the following two overloads + // This overload is provided to support an extension to the oneDPL specification to support the original implementation // of the histogram API, where the boundary type _ValueType could differ from the value type of the input iterator, // and required `<`, `<=`, `+`, `-`, and `/` to be defined between _ValueType and // std::iterator_traits<_RandomAccessIterator1>::value_type rather than enforcing they were the same type or convertible template -std::enable_if_t> && - !std::is_same_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type>, - _RandomAccessIterator2> +std::enable_if_t< + oneapi::dpl::execution::is_execution_policy_v> && + !std::is_convertible_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type>, + _RandomAccessIterator2> histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, _ValueType first_bin_min_val, _ValueType last_bin_max_val, _RandomAccessIterator2 histogram_first) { @@ -85,19 +103,27 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt return histogram_first + num_bins; } -template -oneapi::dpl::__internal::__enable_if_execution_policy<_ExecutionPolicy, _RandomAccessIterator3> -histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, - _RandomAccessIterator2 boundary_first, _RandomAccessIterator2 boundary_last, - _RandomAccessIterator3 histogram_first) +// This overload is provided to support an extension to the oneDPL specification to support the original implementation +// of the histogram API, where if users explicitly-specify all template arguments, their arguments for bin boundary min +// and max are convertible to the specified _ValueType, and the _ValueType is convertible to the value type of the +// input iterator. +template +std::enable_if_t< + oneapi::dpl::execution::is_execution_policy_v> && + std::is_convertible_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type> && + std::is_convertible_v<_RealValueType1, _ValueType> && std::is_convertible_v<_RealValueType2, _ValueType>, + _RandomAccessIterator2> +histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIterator1 last, _Size num_bins, + _RealValueType1 first_bin_min_val, _RealValueType2 last_bin_max_val, _RandomAccessIterator2 histogram_first) { - const auto __dispatch_tag = oneapi::dpl::__internal::__select_backend(exec, first, boundary_first, histogram_first); + const auto __dispatch_tag = oneapi::dpl::__internal::__select_backend(exec, first, histogram_first); - ::std::ptrdiff_t num_bins = boundary_last - boundary_first - 1; oneapi::dpl::__internal::__pattern_histogram( __dispatch_tag, ::std::forward<_ExecutionPolicy>(exec), first, last, num_bins, - oneapi::dpl::__internal::__custom_boundary_binhash{boundary_first, boundary_last}, histogram_first); + oneapi::dpl::__internal::__evenly_divided_binhash<_ValueType>(_ValueType{first_bin_min_val}, + _ValueType{last_bin_max_val}, num_bins), + histogram_first); return histogram_first + num_bins; } From 1cdfc3fc9d7cc4720a5f2fe9a96ff9439562480e Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Wed, 28 Aug 2024 09:05:37 -0400 Subject: [PATCH 07/11] correct spelling Signed-off-by: Dan Hoeflinger --- include/oneapi/dpl/pstl/histogram_extension_defs.h | 2 +- include/oneapi/dpl/pstl/histogram_impl.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/oneapi/dpl/pstl/histogram_extension_defs.h b/include/oneapi/dpl/pstl/histogram_extension_defs.h index 42b005db796..a74282daaef 100644 --- a/include/oneapi/dpl/pstl/histogram_extension_defs.h +++ b/include/oneapi/dpl/pstl/histogram_extension_defs.h @@ -36,7 +36,7 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt _RandomAccessIterator2 boundary_first, _RandomAccessIterator2 boundary_last, _RandomAccessIterator3 histogram_first); -// Support extention API to cover existing API (previous to specification) with the following two overloads +// Support extension API to cover existing API (previous to specification) with the following two overloads // This overload is provided to support an extension to the oneDPL specification to support the original implementation // of the histogram API, where the boundary type _ValueType could differ from the value type of the input iterator, diff --git a/include/oneapi/dpl/pstl/histogram_impl.h b/include/oneapi/dpl/pstl/histogram_impl.h index fb3e2722208..06f477adf72 100644 --- a/include/oneapi/dpl/pstl/histogram_impl.h +++ b/include/oneapi/dpl/pstl/histogram_impl.h @@ -79,7 +79,7 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt return histogram_first + num_bins; } -// Support extention API to cover existing API (previous to specification) with the following two overloads +// Support extension API to cover existing API (previous to specification) with the following two overloads // This overload is provided to support an extension to the oneDPL specification to support the original implementation // of the histogram API, where the boundary type _ValueType could differ from the value type of the input iterator, From c3f87d38193cd9c883e4c77fabadfbcff1a05cbb Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Wed, 28 Aug 2024 10:05:23 -0400 Subject: [PATCH 08/11] adding testing for extension APIs; minor bugfixes in sequential test implementation Signed-off-by: Dan Hoeflinger --- .../numeric/numeric.ops/histogram.pass.cpp | 174 ++++++++++++++++-- test/support/histogram_serial_impl.h | 6 +- 2 files changed, 166 insertions(+), 14 deletions(-) diff --git a/test/parallel_api/numeric/numeric.ops/histogram.pass.cpp b/test/parallel_api/numeric/numeric.ops/histogram.pass.cpp index b16885cf8aa..de65083f81c 100644 --- a/test/parallel_api/numeric/numeric.ops/histogram.pass.cpp +++ b/test/parallel_api/numeric/numeric.ops/histogram.pass.cpp @@ -26,6 +26,96 @@ using namespace TestUtils; #if TEST_DPCPP_BACKEND_PRESENT +// Create a custom wrapped arithmetic type which does not implement conversion +// to the base type, but implements all the necessary arithmetic operations +// to implement the histogram kernel +template +struct wrapped_boundary +{ + wrapped_boundary(T bound) : boundary(bound) + {} + bool operator<(const T& other) + { + return boundary < other; + } + bool operator>(const T& other) + { + return other < *this; + } + bool operator>=(const T& other) + { + return !(*this < other); + } + bool operator<=(const T& other) + { + return !(other < *this); + } + + + bool operator<(const wrapped_boundary& other) + { + return boundary < other.boundary; + } + bool operator>(const wrapped_boundary& other) + { + return other < *this; + } + bool operator>=(const wrapped_boundary& other) + { + return !(*this < other); + } + bool operator<=(const wrapped_boundary& other) + { + return !(other < *this); + } + T boundary; + + friend + wrapped_boundary + operator-(const wrapped_boundary& wrap, const wrapped_boundary& other) + { + return wrapped_boundary{wrap.boundary - other.boundary}; + } + + friend + wrapped_boundary + operator+(const wrapped_boundary& wrap, const wrapped_boundary& other) + { + return wrapped_boundary{wrap.boundary + other.boundary}; + } + + friend + bool operator<(const T& other, const wrapped_boundary& wrap) + { + return other < wrap.boundary; + } + friend + bool operator>=(const T& other, const wrapped_boundary& wrap) + { + return !(other < wrap); + } + friend + bool operator<=(const T& other, const wrapped_boundary& wrap) + { + return !(wrap < other); + } + friend + bool operator>(const T& other, const wrapped_boundary& wrap) + { + return wrap < other; + } + + friend T operator-(const T& other, const wrapped_boundary& wrap) + { + return other - wrap.boundary; + } + + friend std::uint64_t operator/(const std::uint64_t& other, const wrapped_boundary& wrap) + { + return other / wrap.boundary; + } +}; + struct test_histogram_even_bins { template @@ -38,7 +128,46 @@ struct test_histogram_even_bins histogram_sequential(in_first, in_last, bin_size, bin_min, bin_max, expected_bin_first); auto orr = ::oneapi::dpl::histogram(exec, in_first, in_last, bin_size, bin_min, bin_max, bin_first); EXPECT_TRUE(bin_last == orr, "histogram returned wrong iterator"); - EXPECT_EQ_N(expected_bin_first, bin_first, bin_size, "wrong result from histogram"); + EXPECT_EQ_N(expected_bin_first, bin_first, bin_size, "wrong result from even bins histogram"); + ::std::fill_n(bin_first, bin_size, trash); + } +}; + +template +struct test_histogram_even_bins_extension_explicit_template +{ + template + void + operator()(Policy&& exec, Iterator1 in_first, Iterator1 in_last, Iterator2 expected_bin_first, + Iterator2 expected_bin_last, Iterator3 bin_first, Iterator3 bin_last, Size n, T bin_min, T bin_max, + Size trash) + { + const Size bin_size = bin_last - bin_first; + histogram_sequential(in_first, in_last, bin_size, bin_min, bin_max, expected_bin_first); + auto orr = ::oneapi::dpl::histogram(exec, in_first, in_last, bin_size, bin_min, + bin_max, bin_first); + EXPECT_TRUE(bin_last == orr, "histogram returned wrong iterator"); + EXPECT_EQ_N(expected_bin_first, bin_first, bin_size, "wrong result from explicit template histogram extension"); + ::std::fill_n(bin_first, bin_size, trash); + } +}; + +struct test_histogram_even_bins_extension_non_convertible +{ + template + void + operator()(Policy&& exec, Iterator1 in_first, Iterator1 in_last, Iterator2 expected_bin_first, + Iterator2 expected_bin_last, Iterator3 bin_first, Iterator3 bin_last, Size n, T bin_min, T bin_max, + Size trash) + { + const Size bin_size = bin_last - bin_first; + wrapped_boundary wrapped_bin_min{bin_min}; + wrapped_boundary wrapped_bin_max{bin_max}; + histogram_sequential(in_first, in_last, bin_size, wrapped_bin_min, wrapped_bin_max, expected_bin_first); + auto orr = ::oneapi::dpl::histogram(exec, in_first, in_last, bin_size, wrapped_bin_min, wrapped_bin_max, + bin_first); + EXPECT_TRUE(bin_last == orr, "histogram returned wrong iterator"); + EXPECT_EQ_N(expected_bin_first, bin_first, bin_size, "wrong result from non convertible histogram extension"); ::std::fill_n(bin_first, bin_size, trash); } }; @@ -56,12 +185,12 @@ struct test_histogram_range_bins histogram_sequential(in_first, in_last, boundary_first, boundary_last, expected_bin_first); auto orr = ::oneapi::dpl::histogram(exec, in_first, in_last, boundary_first, boundary_last, bin_first); EXPECT_TRUE(bin_last == orr, "histogram returned wrong iterator"); - EXPECT_EQ_N(expected_bin_first, bin_first, bin_size, "wrong result from histogram"); + EXPECT_EQ_N(expected_bin_first, bin_first, bin_size, "wrong result from custom range histogram"); ::std::fill_n(bin_first, bin_size, trash); } }; -template <::std::size_t CallNumber, typename Size, typename T> +template <::std::size_t CallNumber, typename ExplicitValueType, typename Size, typename T> void test_range_and_even_histogram(Size n, T min_boundary, T max_boundary, T overflow, Size jitter, Size num_bins, Size trash) @@ -74,11 +203,11 @@ test_range_and_even_histogram(Size n, T min_boundary, T max_boundary, T overflow Sequence expected(num_bins, [](size_t k) { return 0; }); Sequence out(num_bins, [&](size_t k) { return trash; }); - invoke_on_all_hetero_policies()(test_histogram_even_bins(), in.begin(), in.end(), expected.begin(), + invoke_on_all_hetero_policies()(test_histogram_even_bins(), in.begin(), in.end(), expected.begin(), expected.end(), out.begin(), out.end(), Size(in.size()), min_boundary, max_boundary, trash); # if !ONEDPL_FPGA_DEVICE - invoke_on_all_hetero_policies()(test_histogram_even_bins(), in.cbegin(), in.cend(), + invoke_on_all_hetero_policies()(test_histogram_even_bins(), in.cbegin(), in.cend(), expected.begin(), expected.end(), out.begin(), out.end(), Size(in.size()), min_boundary, max_boundary, trash); # endif // !ONEDPL_FPGA_DEVICE @@ -86,17 +215,40 @@ test_range_and_even_histogram(Size n, T min_boundary, T max_boundary, T overflow T offset = (max_boundary - min_boundary) / T(num_bins); Sequence boundaries(num_bins + 1, [&](size_t k) { return k * offset + (std::rand() % jitter) + min_boundary; }); - invoke_on_all_hetero_policies()(test_histogram_range_bins(), in.begin(), in.end(), + invoke_on_all_hetero_policies()(test_histogram_range_bins(), in.begin(), in.end(), boundaries.begin(), boundaries.end(), expected.begin(), expected.end(), out.begin(), out.end(), trash); # if !ONEDPL_FPGA_DEVICE - invoke_on_all_hetero_policies()(test_histogram_range_bins(), in.cbegin(), in.cend(), + invoke_on_all_hetero_policies()(test_histogram_range_bins(), in.cbegin(), in.cend(), boundaries.cbegin(), boundaries.cend(), expected.begin(), expected.end(), out.begin(), out.end(), trash); # endif // !ONEDPL_FPGA_DEVICE + + //test extension + + invoke_on_all_hetero_policies()(test_histogram_even_bins_extension_explicit_template{}, in.begin(), in.end(), expected.begin(), + expected.end(), out.begin(), out.end(), Size(in.size()), + min_boundary, max_boundary, trash); +# if !ONEDPL_FPGA_DEVICE + invoke_on_all_hetero_policies()(test_histogram_even_bins_extension_explicit_template{}, in.cbegin(), in.cend(), + expected.begin(), expected.end(), out.begin(), out.end(), + Size(in.size()), min_boundary, max_boundary, trash); +# endif // !ONEDPL_FPGA_DEVICE + + + invoke_on_all_hetero_policies()(test_histogram_even_bins_extension_non_convertible(), in.begin(), in.end(), expected.begin(), + expected.end(), out.begin(), out.end(), Size(in.size()), + min_boundary, max_boundary, trash); +# if !ONEDPL_FPGA_DEVICE + invoke_on_all_hetero_policies()(test_histogram_even_bins_extension_non_convertible(), in.cbegin(), in.cend(), + expected.begin(), expected.end(), out.begin(), out.end(), + Size(in.size()), min_boundary, max_boundary, trash); +# endif // !ONEDPL_FPGA_DEVICE + + } -template <::std::size_t CallNumber, typename T, typename Size> +template <::std::size_t CallNumber, typename T, typename Size, typename ExplicitValueType> void test_histogram(T min_boundary, T max_boundary, T overflow, Size jitter, Size trash) { @@ -104,7 +256,7 @@ test_histogram(T min_boundary, T max_boundary, T overflow, Size jitter, Size tra { for (Size n = 0; n <= 100000; n = n <= 16 ? n + 1 : Size(3.1415 * n)) { - test_range_and_even_histogram(n, min_boundary, max_boundary, overflow, jitter, bin_size, trash); + test_range_and_even_histogram(n, min_boundary, max_boundary, overflow, jitter, bin_size, trash); } } } @@ -114,10 +266,10 @@ int main() { #if TEST_DPCPP_BACKEND_PRESENT - test_histogram<0, float, uint32_t>(10000.0f, 110000.0f, 300.0f, uint32_t(50), uint32_t(99999)); + test_histogram<0, float, uint32_t, double>(10000.0f, 110000.0f, 300.0f, uint32_t(50), uint32_t(99999)); #if !ONEDPL_FPGA_DEVICE - test_histogram<1, std::int32_t, uint64_t>(-50000, 50000, 10000, uint64_t(5), uint64_t(99999)); + test_histogram<1, std::int32_t, uint64_t, std::int64_t>(-50000, 50000, 10000, uint64_t(5), uint64_t(99999)); #endif //!ONEDPL_FPGA_DEVICE #endif // TEST_DPCPP_BACKEND_PRESENT diff --git a/test/support/histogram_serial_impl.h b/test/support/histogram_serial_impl.h index 0427125682d..0374f31c536 100644 --- a/test/support/histogram_serial_impl.h +++ b/test/support/histogram_serial_impl.h @@ -17,17 +17,17 @@ #define _HISTOGRAM_SERIAL_IMPL_H template -::std::enable_if_t, ::std::uint32_t> +::std::enable_if_t, ::std::uint32_t> get_bin(T1 value, T2 min, T2 max, Size num_bins) { return ::std::uint32_t((::std::uint64_t(value - min) * ::std::uint64_t(num_bins)) / (max - min)); } template -::std::enable_if_t, ::std::uint32_t> +::std::enable_if_t, ::std::uint32_t> get_bin(T1 value, T2 min, T2 max, Size num_bins) { - return ::std::uint32_t((value - min) * (T1(num_bins) / (max - min))); + return ::std::uint32_t((value - min) * (T2(num_bins) / (max - min))); } template From 127f2cbd96a2627408a955a7b381c7c489dd3c17 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Wed, 28 Aug 2024 13:41:07 -0400 Subject: [PATCH 09/11] Improve comment about explicit specification of template args Signed-off-by: Dan Hoeflinger --- include/oneapi/dpl/pstl/histogram_extension_defs.h | 3 ++- include/oneapi/dpl/pstl/histogram_impl.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/oneapi/dpl/pstl/histogram_extension_defs.h b/include/oneapi/dpl/pstl/histogram_extension_defs.h index a74282daaef..6379621c056 100644 --- a/include/oneapi/dpl/pstl/histogram_extension_defs.h +++ b/include/oneapi/dpl/pstl/histogram_extension_defs.h @@ -54,7 +54,8 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt // This overload is provided to support an extension to the oneDPL specification to support the original implementation // of the histogram API, where if users explicitly-specify all template arguments, their arguments for bin boundary min // and max are convertible to the specified _ValueType, and the _ValueType is convertible to the value type of the -// input iterator. +// input iterator. Note that _ValueType is not deducable from the function arugments, so this overload is only used +// when users explicitly specify the _ValueType template argument. template std::enable_if_t< diff --git a/include/oneapi/dpl/pstl/histogram_impl.h b/include/oneapi/dpl/pstl/histogram_impl.h index 06f477adf72..cfcd13352a0 100644 --- a/include/oneapi/dpl/pstl/histogram_impl.h +++ b/include/oneapi/dpl/pstl/histogram_impl.h @@ -106,7 +106,8 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt // This overload is provided to support an extension to the oneDPL specification to support the original implementation // of the histogram API, where if users explicitly-specify all template arguments, their arguments for bin boundary min // and max are convertible to the specified _ValueType, and the _ValueType is convertible to the value type of the -// input iterator. +// input iterator. Note that _ValueType is not deducable from the function arugments, so this overload is only used +// when users explicitly specify the _ValueType template argument. template std::enable_if_t< From ce768f50235e446dfe6e3076fab18951a1480049 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Wed, 28 Aug 2024 16:27:28 -0400 Subject: [PATCH 10/11] spelling Signed-off-by: Dan Hoeflinger --- include/oneapi/dpl/pstl/histogram_extension_defs.h | 2 +- include/oneapi/dpl/pstl/histogram_impl.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/oneapi/dpl/pstl/histogram_extension_defs.h b/include/oneapi/dpl/pstl/histogram_extension_defs.h index 6379621c056..1d418fa9a63 100644 --- a/include/oneapi/dpl/pstl/histogram_extension_defs.h +++ b/include/oneapi/dpl/pstl/histogram_extension_defs.h @@ -54,7 +54,7 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt // This overload is provided to support an extension to the oneDPL specification to support the original implementation // of the histogram API, where if users explicitly-specify all template arguments, their arguments for bin boundary min // and max are convertible to the specified _ValueType, and the _ValueType is convertible to the value type of the -// input iterator. Note that _ValueType is not deducable from the function arugments, so this overload is only used +// input iterator. Note that _ValueType is not deducable from the function arguments, so this overload is only used // when users explicitly specify the _ValueType template argument. template diff --git a/include/oneapi/dpl/pstl/histogram_impl.h b/include/oneapi/dpl/pstl/histogram_impl.h index cfcd13352a0..5f6d65e9ed4 100644 --- a/include/oneapi/dpl/pstl/histogram_impl.h +++ b/include/oneapi/dpl/pstl/histogram_impl.h @@ -106,7 +106,7 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt // This overload is provided to support an extension to the oneDPL specification to support the original implementation // of the histogram API, where if users explicitly-specify all template arguments, their arguments for bin boundary min // and max are convertible to the specified _ValueType, and the _ValueType is convertible to the value type of the -// input iterator. Note that _ValueType is not deducable from the function arugments, so this overload is only used +// input iterator. Note that _ValueType is not deducable from the function arguments, so this overload is only used // when users explicitly specify the _ValueType template argument. template From 8e329b916f0786b8479c0bcb92d958b8f5b05069 Mon Sep 17 00:00:00 2001 From: Dan Hoeflinger Date: Wed, 28 Aug 2024 17:25:24 -0400 Subject: [PATCH 11/11] deprecating APIs, and adding test macro to avoid testing deprecated APIs Signed-off-by: Dan Hoeflinger --- include/oneapi/dpl/pstl/histogram_impl.h | 8 ++++++++ .../numeric/numeric.ops/histogram.pass.cpp | 16 +++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/oneapi/dpl/pstl/histogram_impl.h b/include/oneapi/dpl/pstl/histogram_impl.h index 5f6d65e9ed4..dd786257013 100644 --- a/include/oneapi/dpl/pstl/histogram_impl.h +++ b/include/oneapi/dpl/pstl/histogram_impl.h @@ -87,6 +87,10 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt // std::iterator_traits<_RandomAccessIterator1>::value_type rather than enforcing they were the same type or convertible template +[[deprecated("Use of oneapi::dpl::histogram API with bin boundary value arguments which are not convertible to the " + " value type of the input iterator is deprecated and will be removed in a future release. Please use " + " the oneapi::dpl histogram API with the same value type for both the input iterator and bin boundary " + " values.")]] std::enable_if_t< oneapi::dpl::execution::is_execution_policy_v> && !std::is_convertible_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type>, @@ -110,6 +114,10 @@ histogram(_ExecutionPolicy&& exec, _RandomAccessIterator1 first, _RandomAccessIt // when users explicitly specify the _ValueType template argument. template +[[deprecated("Use of oneapi::dpl::histogram API with explicit specification of the template type for the " + " bin boundary value parameters is deprecated and will be removed in a future release. Please use " + " the oneapi::dpl histogram API with the same value type for both the input iterator and bin boundary " + " values.")]] std::enable_if_t< oneapi::dpl::execution::is_execution_policy_v> && std::is_convertible_v<_ValueType, typename std::iterator_traits<_RandomAccessIterator1>::value_type> && diff --git a/test/parallel_api/numeric/numeric.ops/histogram.pass.cpp b/test/parallel_api/numeric/numeric.ops/histogram.pass.cpp index de65083f81c..fdc597f6fc8 100644 --- a/test/parallel_api/numeric/numeric.ops/histogram.pass.cpp +++ b/test/parallel_api/numeric/numeric.ops/histogram.pass.cpp @@ -26,6 +26,8 @@ using namespace TestUtils; #if TEST_DPCPP_BACKEND_PRESENT +#define TEST_DEPRECATED_APIS 1 + // Create a custom wrapped arithmetic type which does not implement conversion // to the base type, but implements all the necessary arithmetic operations // to implement the histogram kernel @@ -224,27 +226,27 @@ test_range_and_even_histogram(Size n, T min_boundary, T max_boundary, T overflow expected.end(), out.begin(), out.end(), trash); # endif // !ONEDPL_FPGA_DEVICE - //test extension - + //test deprecated extension +# if TEST_DEPRECATED_APIS invoke_on_all_hetero_policies()(test_histogram_even_bins_extension_explicit_template{}, in.begin(), in.end(), expected.begin(), expected.end(), out.begin(), out.end(), Size(in.size()), min_boundary, max_boundary, trash); -# if !ONEDPL_FPGA_DEVICE +# if !ONEDPL_FPGA_DEVICE invoke_on_all_hetero_policies()(test_histogram_even_bins_extension_explicit_template{}, in.cbegin(), in.cend(), expected.begin(), expected.end(), out.begin(), out.end(), Size(in.size()), min_boundary, max_boundary, trash); -# endif // !ONEDPL_FPGA_DEVICE +# endif // !ONEDPL_FPGA_DEVICE invoke_on_all_hetero_policies()(test_histogram_even_bins_extension_non_convertible(), in.begin(), in.end(), expected.begin(), expected.end(), out.begin(), out.end(), Size(in.size()), min_boundary, max_boundary, trash); -# if !ONEDPL_FPGA_DEVICE +# if !ONEDPL_FPGA_DEVICE invoke_on_all_hetero_policies()(test_histogram_even_bins_extension_non_convertible(), in.cbegin(), in.cend(), expected.begin(), expected.end(), out.begin(), out.end(), Size(in.size()), min_boundary, max_boundary, trash); -# endif // !ONEDPL_FPGA_DEVICE - +# endif // !ONEDPL_FPGA_DEVICE +# endif // TEST_DEPRECATED_APIS }