Add distributed ranges as experimental feature. #1479

BenBrock · 2024-04-04T23:33:11Z

This draft PR adds distributed ranges as an experimental feature, inside the onedpl::experimental::dr namespace.

…o dist-ranges_cleanup

* cleanups * cleanups of include paths * cleanups of include paths and error messages

…synch, + comments

…trait for some oneDPL types

This reverts commit cb7259d.

…zation for some oneDPL types.

… zip_forward_iteratoris not used in the device code

Co-authored-by: Łukasz Ślusarczyk <112692748+lslusarczyk@users.noreply.github.com>

MikeDvorskiy

Left the comments and questions regarding to for_each implementation (sp/algorithms/for_each.hpp)

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/for_each.hpp

MikeDvorskiy · 2024-08-01T13:00:13Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/for_each.hpp

+namespace oneapi::dpl::experimental::dr::sp
+{
+
+template <typename ExecutionPolicy, distributed_range R, typename Fn>


What kinds of execution policy are supported? (by design and by this implementation)
As far the C++20 concepts (distributed_range at least) are already used, it makes sense to add constrains on an execution policy as well.
In oneDPL, there is oneapi::dpl::is_execution_policy_v for example.

Does it make sense to add a constrain on Fn as well?

The first line in the method is static_assert that the only supported execution policy is

static_assert( // currently only one policy supported std::is_same_v<std::remove_cvref_t<ExecutionPolicy>, sycl_device_collection>);

IMO this is enough and simple

I don't think so. There are no constrains for functor in std::for_each

Having "concepts" for some parameter(s) in a signature and having "static_assert" for the other parameter(s) - not consistent approach. Or we use type requirements (concepts and contains) in a algo signature or don't use at all. As far as we use C++20, I would vote for "concepts and constrains" usage. A user of oneDPL usually sees into the documentation where a signature there is.

But there is in std::ranges::for_each

(Originally posted my comment about par_unseq here, but have moved it below.)

We basically only have one type of device policy, which is a sycl_device_collection. The standard library doesn't have any constraints for ExecutionPolicy, which is why we have things as they are now.

I'd be fine with adding a is_execution_policy_v requirement or execution_policy concept.

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/for_each.hpp

MikeDvorskiy · 2024-08-01T13:12:13Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/for_each.hpp

+
+    for (auto&& segment : ranges::segments(r))
+    {
+        auto&& q = __detail::queue(ranges::rank(segment));


As far as I understand just sycl device policy is supported for current implementtaion..
So, in the future, what will be queue in case of a host policy, std::execution::par, for example?

It can be a separate code without queues and the code is selected by constexpr. We will see. For now there is no need to design the solution.

In that case, probably it makes sense to add a constrain on an execution policy, like
is_sycl_device_collection_v?

MikeDvorskiy · 2024-08-01T13:14:11Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/for_each.hpp

+        auto first = stdrng::begin(local_segment);
+
+        auto event = dr::__detail::parallel_for(q, sycl::range<>(stdrng::distance(local_segment)),
+                                                [=](auto idx) { fn(*(first + idx)); });


I don't insist... but first[idx] instead of *(first + idx) more readable and shorter, IMHO.

thank's for spotting this, applied in #1758 here and in a few other places

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/for_each.hpp

MikeDvorskiy · 2024-08-01T13:16:37Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/for_each.hpp

+    __detail::wait(events);
+}
+
+template <typename ExecutionPolicy, distributed_iterator Iter, typename Fn>


The same comment (see above) regarding ExecutionPolicy.

same answer, for now static_assert is enough I think

MikeDvorskiy · 2024-08-01T13:22:00Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/for_each.hpp

+
+template <distributed_range R, typename Fn>
+void
+for_each(R&& r, Fn fn)


That re-call of for_each w/o policy to for_each(par_unseq) is done by design? Is the some explanation and statement in the documentation?

According to the static_assert (line 34) above par_unseq is not currently supported...

Taking into account 1) and 2) does whether it make sense to release non-policy for_each versions at all?

These algorithms are restricted by distributed range/iterator concepts. It's a question if we want to keep it this way in the long run, but let's keep the approach as-is for now.

This par_unseq is from our namespace which is

inline sycl_device_collection par_unseq;

inline sycl_device_collection par_unseq;

It totally confuses any C++ developers which are aware of C+17 execution policies.
(and it is a potential name conflict...)
I would suggest renaming one in something proper name.

Can you explain what's problematic about using the name par_unseq as a catch-all execution policy to execute across multiple devices?

From my understanding of the spec, par_unseq requires:

Execution in "unordered fashion" in "unspecified threads of execution," and

These threads must provide "weakly parallel forward progress guarantees."

From what I understand, threads in SYCL have weakly parallel forward progress guarantees. Is it the "threads of execution" part that would be violated?

Other libraries like Nvidia's stdpar use par_unseq to mean "execute in parallel, including on GPUs if that's where the data is." That's what we're doing here, and it seems like a reasonable choice to me. Is there a convention in oneDPL that par_unseq is CPU only, or is there another good reason to not use par_unseq as the "sane default" that includes potentially running on a GPU if that's where the data is resident?

adamfidel · 2024-08-01T14:38:04Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/inclusive_scan.hpp

+    std::size_t idx = 0;
+    for (auto&& segs : zipped_segments)
+    {
+        auto&& [in_segment, out_segment] = segs;


It seems like this line can be moved within the if scope. Or perhaps this loop can be rewritten to have a if (idx == 0) continue; at the top and remove the following if entirely.

adamfidel · 2024-08-01T14:39:14Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/inclusive_scan.hpp

+            h.depends_on(event);
+            h.single_task([=]() {
+                stdrng::range_value_t<O> value = *src_iter;
+                *dst_iter = value;


Can we remove the explicit temporary here by rewriting as follows:

Suggested change

*dst_iter = value;

*dst_iter = *src_iter;

I think these changes should be fine (#1766), but need to be tested on PVC to double-check before merging.

@lslusarczyk could you check if things look okay on your end?

adamfidel · 2024-08-01T14:47:34Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/init.hpp

+init(R&& devices) requires(std::is_same_v<sycl::device, std::remove_cvref_t<stdrng::range_value_t<R>>>)
+{
+    __detail::devices_.assign(stdrng::begin(devices), stdrng::end(devices));
+    __detail::global_context_ = new sycl::context(__detail::devices_);


From what I can tell, there is nothing preventing a user from calling this init function multiple times, which would result in a memory leak here. Would it help if the context was stored in a shared_ptr or unique_ptr? Or is there something else that I am missing?

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/exclusive_scan.hpp

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/reduce.hpp

julianmi · 2024-08-01T15:31:57Z

test/distributed_ranges/common/reduce.cpp

I think these tests are a good start but could be expanded to increase the test coverage. E.g.:

Testing with 10 elements only seems limited for what DR is designed for. The other oneDPL tests use loops with increasing counts.

It looks like we test with int only. Different types and potentially a custom type might identify some gaps.

It looks like we test with std::plus and the custom max operator only.

julianmi · 2024-08-01T15:32:27Z

test/distributed_ranges/common/reduce.cpp

+
+TYPED_TEST_SUITE(Reduce, AllTypes);
+
+TYPED_TEST(Reduce, Range) {


Should this be called RangeMax instead of the test below?

julianmi · 2024-08-01T15:33:06Z

test/distributed_ranges/common/reduce.cpp

+TYPED_TEST(Reduce, Range) {
+  Ops1<TypeParam> ops(10);
+
+  auto max = [](double x, double y) { return std::max(x, y); };


Should we use TypeParam instead of double?

julianmi · 2024-08-01T15:36:49Z

test/distributed_ranges/common/reduce.cpp

+  Ops1<TypeParam> ops(10);
+
+  auto max = [](double x, double y) { return std::max(x, y); };
+  EXPECT_EQ(std::reduce(ops.vec.begin(), ops.vec.end(), 3, max),


The init is set to the integer 3 independent of TypeParam. This could change the result depending on TypeParam. Is this by design? And do we need tests with init of type TypeParam?

julianmi · 2024-08-01T15:38:25Z

test/distributed_ranges/common/reduce.cpp

+            xp::reduce(ops.dist_vec, 3, max));
+}
+
+TYPED_TEST(Reduce, Max) {


Should this test be called RangePlus?

julianmi · 2024-08-01T15:41:26Z

test/distributed_ranges/common/reduce.cpp

+            xp::reduce(ops.dist_vec, 3, std::plus{}));
+}
+
+TYPED_TEST(Reduce, Iterators) {


Should this test be called IteratorsPlus? And why don't we test max with iterators?

* concepts.hpp moved and included in dpl/distributed_ranges * fixes after merge --------- Co-authored-by: Łukasz Ślusarczyk <112692748+lslusarczyk@users.noreply.github.com>

* sp::views::enumerate and __detail::enumarate merged * update * update * update * C++23 enumerate_view used instead of our implementation * cleanup * update * detail/enumerate.hpp should be removed * self applied comments * fix compilation in g++13 --------- Co-authored-by: Łukasz Ślusarczyk <lukasz.slusarczyk@intel.com>

akukanov · 2024-08-02T07:37:09Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/view_detectors.hpp

+namespace oneapi::dpl::experimental::dr
+{
+
+template <typename T>
+struct is_ref_view : std::false_type
+{
+};


Are the "view detectors" considered a part of public API? If not, better move those to __detail.

akukanov · 2024-08-02T07:37:20Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/view_detectors.hpp

+template <typename T>
+struct is_iota_view : std::false_type
+{
+};
+template <std::weakly_incrementable W>
+struct is_iota_view<stdrng::iota_view<W>> : std::true_type
+{
+};
+
+template <typename T>
+inline constexpr bool is_iota_view_v = is_iota_view<T>{};


is_iota_view is not used anywhere, including the original DR repository.

akukanov · 2024-08-02T07:57:22Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/view_detectors.hpp

+#if (defined __cpp_lib_ranges_slide)
+
+template <typename T>
+struct is_sliding_view<stdrng::slide_view<T>> : std::true_type
+{
+};
+template <typename T>
+inline constexpr bool is_sliding_view_v = is_sliding_view<std::remove_cvref_t<T>>::value;
+
+#endif


Should not is_sliding_view_v be moved out of #if...#endif?

akukanov · 2024-08-02T08:02:18Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/view_detectors.hpp

+template <typename T>
+struct is_zip_view : std::false_type
+{
+};
+
+#if (defined _cpp_lib_ranges_zip)
+template <typename... Views>
+struct is_zip_view<stdrng::zip_view<Views...>> : std::true_type
+{
+};
+
+#endif
+template <typename T>
+inline constexpr bool is_zip_view_v = is_zip_view<T>::value;


is_zip_view is not used anywhere, including the original DR repo.

akukanov · 2024-08-02T14:04:49Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/ranges.hpp

While detail/ranges.hpp suggests that there are some internals of the DR implementation, in fact, this file contains the implementation of public customization points for DR: ranges::rank and ranges::segments. So I would suggest to call it appropriately - maybe cpos.hpp, or split into rank.hpp and segments.hpp. And, since it contains public APIs, I would also place this file into the root directory instead of detail.

akukanov · 2024-08-02T15:39:34Z

include/oneapi/dpl/distributed_ranges

+// This file incorporates work covered by the following copyright and permission
+// notice:
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+//


I believe this part of the copyright header should not be used within DR, since it does not inherit nor use any LLVM work. It should only apply to the standard parallel algorithms in oneDPL.

MikeDvorskiy · 2024-08-02T17:31:14Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/for_each.hpp

+
+        auto local_segment = __detail::local(segment);
+
+        auto first = stdrng::begin(local_segment);


Is local_segment a range? if yes, why do we need to get an iterator and use the iterator in parallel_for instead of the range (local_segment)?
Probably, the code below is better?

auto event = dr::__detail::parallel_for(q, sycl::range<>(stdrng::distance(local_segment)), [=](auto idx) { fn(local_segment[idx]); });

This would probably work, but I'm not 100% sure it would work all the time. We'd need to make sure that local_segment is a non-owning, trivially copyable view in all cases.

One other consideration is that in some cases the view might be bigger than the iterator, which could add some small overhead.

…ments (#1756)

* Relative paths in includes * format fix * fixes in __detail namespace * removed unnamed namespaces * fixes in namespaces * ranges::local_or_identity in place of __detail::local * minor fix * ranges.hpp update * ranges.hpp update * local_or_identity made __detail * removed is_localizable --------- Co-authored-by: Łukasz Ślusarczyk <112692748+lslusarczyk@users.noreply.github.com> Co-authored-by: Łukasz Ślusarczyk <lukasz.slusarczyk@intel.com>

Co-authored-by: Łukasz Ślusarczyk <lukasz.slusarczyk@intel.com>

MikeDvorskiy · 2024-08-05T12:04:07Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/fill.hpp

+
+template <std::contiguous_iterator Iter>
+requires(!std::is_const_v<std::iter_value_t<Iter>> && std::is_trivially_copyable_v<std::iter_value_t<Iter>>) sycl::event
+    fill_async(Iter first, Iter last, const std::iter_value_t<Iter>& value)


Firstly, I would ask why the implementation of "fill" algo is not consistent with others, for_each for example?

Why there is no oneapi::dpl::experimental::dr::sp::for_each_async, but there is fill_async?

Why sync version fill doesn't have ExecutionPolicy parameter, but for_each has one?
What exactly versions (sync/async) and signatures do we want to release?

We originally implemented fill using SYCL fill. The asynchronous version of fill is a single-GPU algorithm that originally called SYCL's q.fill(). However, SYCL's fill fails on buffers larger than 2^31, so we implemented a work-around.

In the future we'd perhaps like to go back to SYCL's q.fill(), as in theory you might be able to implement a fill faster than with a for_each. However, right now, it's pretty similar to for_each, and we could probably just implement it directly with for_each.

Lack of execution policy is just an oversight.

MikeDvorskiy · 2024-08-05T12:10:52Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/fill.hpp

+auto
+fill(DR&& r, const T& value)
+{
+    fill_async(r, value).wait();


I would suggest having very simple implementation based on already implemented oneapi::dpl::experimental::dr::spfor_each and remove the all other implementation stuff from the file. We avoid huge code duplication.

oneapi::dpl::experimental::dr::for_each(std::forward<DR>(r), [value](auto& x) { x = value;});

See my comment above. I think this is a good idea (for now), although the current code structure would make it very easy to return to using SYCL's fill, which in theory could be more efficient.

MikeDvorskiy · 2024-08-05T12:12:08Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/fill.hpp

+auto
+fill(Iter first, Iter last, const T& value)
+{
+    fill_async(stdrng::subrange(first, last), value).wait();


oneapi::dpl::experimental::dr::for_each(first, last, [value](auto& x) { x = value;}); ? (see my comment above)

MikeDvorskiy · 2024-08-05T12:24:25Z

test/distributed_ranges/common/iota_view.cpp

+}
+
+// https://github.com/oneapi-src/distributed-ranges/issues/787
+//TYPED_TEST(IotaView, Copy) {


Since a github issue already created , probably it makes sense to remove that commented code below?

MikeDvorskiy · 2024-08-05T12:33:31Z

include/oneapi/dpl/internal/distributed_ranges_impl/sp/algorithms/iota.hpp

+{
+    auto iota_view = stdrng::views::iota(value, T(value + stdrng::distance(r)));
+
+    for_each(par_unseq, views::zip(iota_view, r), [](auto&& elem) {


It seems that for_each call with par_unseq leads to a static_assert within oneapi::dpl::experimental::dr::sp::for_each implementation....

* An experiment to use std ranges unconditionally * Change to a macro that defines the shim header name * Change #ifndef to #ifdef * Remove extra underscore in the namespace macro name * Rework to use a single macro, according to the review suggestion

rarutyun

Was not able to look a lot from the first attempt but left some comments. My suggestion is to reach me in Teams in case you have questions (just to speed things up)

rarutyun · 2024-08-07T04:30:34Z

include/oneapi/dpl/distributed_ranges

+#include "oneapi/dpl/internal/common_config.h"
+#include "oneapi/dpl/pstl/onedpl_config.h"
+
+#if __cplusplus > 202002L


This macro doesn't work on Microsoft. Let's use _ONEDPL___cplusplus or whatever it's called

rarutyun · 2024-08-07T04:44:22Z

include/oneapi/dpl/internal/distributed_ranges_impl/concepts.hpp

+
+#include "detail/ranges.hpp"
+
+namespace oneapi::dpl::experimental::dr


The biggest question is why those concepts are public?

I think it is good to make these concepts public. At the very least, it makes the documentation clearer, but also it allows someone to implement their own variations of distributed containers, etc. And it certainly facilitates possible discussions.

rarutyun · 2024-08-07T05:04:38Z

CMakeLists.txt

+        # if C++23 or newer, include Distributed Ranges (experimental)
+        if (CMAKE_CXX_STANDARD GREATER_EQUAL 23)
+            set(ONEDPL_USE_DR TRUE)
+            message(STATUS "Adding Distributed Ranges to the project")
+        else()
+            message(STATUS "C++23 required to use Distributed Ranges in oneDPL")
+        endif()


The biggest problem that I have with requiring c++23 for distributed ranges is that this API is an experimental one. That means ideally we want to get feedback from the users. From the past we know that our experimental API is not very popular. Requiring c++23 has (almost) killed our chances to get any feedback.

With regard to zip_view complexity. I think @kboyarinov has the implementation. You can ask him (when he is back from vacation) if it's still the case. I believe he will be more than happy to donate one.

rarutyun · 2024-08-07T05:47:40Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/iterator_adaptor.hpp

+    template <typename... Args>
+    requires(sizeof...(Args) >= 1 &&
+             !((sizeof...(Args) == 1 && (std::is_same_v<nonconst_iterator, std::decay_t<Args>> || ...)) ||
+               (std::is_same_v<const_iterator, std::decay_t<Args>> || ...) ||
+               (std::is_same_v<nonconst_accessor_type, std::decay_t<Args>> || ...) ||
+               (std::is_same_v<const_accessor_type, std::decay_t<Args>> || ...)) &&
+             std::is_constructible_v<accessor_type, Args...>) iterator_adaptor(Args&&... args)
+        : accessor_(std::forward<Args>(args)...)


Sorry if my comment is going to be wrong but that's because this part is absolutely unreadable.

First please confirm the design intent here. As far as I understand sizeof...(Args) should apply to all || conditions. So that !(sizeof...(Args) == 1 && (is_same<const_iterator> || is_same<non_const_iterator, etc> (of course this is pseudo code). What I wrote also means that we need to check only the first argument in parameter pack, not all of them.

If that's the design intent and my ability to parse parentheses is not that bad:) this is not what is written for this constructor.

I will give my suggestion that should work for my understanding of the design intent and we can discuss it further on, if necessary. Probably I don't understand the design intent but then I need an explanation.

Suggested change

template <typename... Args>

requires(sizeof...(Args) >= 1 &&

!((sizeof...(Args) == 1 && (std::is_same_v<nonconst_iterator, std::decay_t<Args>> || ...)) ||

(std::is_same_v<const_iterator, std::decay_t<Args>> || ...) ||

(std::is_same_v<nonconst_accessor_type, std::decay_t<Args>> || ...) ||

(std::is_same_v<const_accessor_type, std::decay_t<Args>> || ...)) &&

std::is_constructible_v<accessor_type, Args...>) iterator_adaptor(Args&&... args)

: accessor_(std::forward<Args>(args)...)

template <typename Arg, typename... Args>

requires(sizeof...(Args) > 0

&& !std::is_same_v<nonconst_iterator, std::decay_t<Arg>

&& !std::is_same_v<const_iterator, std::decay_t<Arg>>

&& !std::is_same_v<nonconst_accessor_type, std::decay_t<Arg>>

&& !std::is_same_v<const_accessor_type, std::decay_t<Arg>>

&& std::is_constructible_v<accessor_type, Arg, Args...>)

iterator_adaptor(Arg&& arg, Args&&... args)

: accessor_(std::forward<Arg>(arg), std::forward<Args>(args)...)

{

}

The places of && this is just an example. While we have to use extra parameter (both template and function) look how simpler the requires condition is in the suggestion.

rarutyun · 2024-08-07T05:48:36Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/iterator_adaptor.hpp

+    bool
+    operator!=(const_iterator other) const
+    {
+        return !(*this == other);
+    }


If our minimal required standard is c++20 we don't need operator!=

rarutyun · 2024-08-07T05:56:36Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/iterator_adaptor.hpp

+    auto
+    segments() const noexcept requires(ranges::__detail::has_segments_method<accessor_type>)
+    {
+        return accessor_.segments();


Does accessor_.segments() never throw? If not it's better to have noexcept(noexcept(accessor_.segments())

rarutyun · 2024-08-07T06:05:20Z

include/oneapi/dpl/internal/distributed_ranges_impl/concepts.hpp

+template <typename I>
+concept remote_iterator = std::forward_iterator<I> && requires(I& iter)
+{
+    ranges::rank(iter);


can we potentially end up with the name conflict for ranges namespace name? My guess is we should not based on my knowledge of lookup rules but do we want to change this line and the lines below to dr::ranges just to be safe?

To be honest I don't have a strong preference

rarutyun · 2024-08-07T06:06:51Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/iterator_adaptor.hpp

+    }
+
+    auto
+    segments() const noexcept requires(ranges::__detail::has_segments_method<accessor_type>)


What is the point of having requires-clause here. C++ doesn't case an error when users don't touch a function. The argument that concepts provide better diagnostics is very agruable.

rarutyun · 2024-08-07T06:08:18Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/ranges.hpp

+{
+
+template <typename>
+inline constexpr bool disable_rank = false;


Is this a public customization point? In other words, do we allow the users to specialize this variable?

rarutyun · 2024-08-07T06:13:43Z

include/oneapi/dpl/internal/distributed_ranges_impl/detail/ranges.hpp

+    // OR, if not available,
+    // 2) r.begin().rank(), if iterator is `remote_iterator`
+    template <stdrng::forward_range R>
+    requires((has_rank_method<R> && !disable_rank<std::remove_cv_t<R>>) ||


You don't need this requires clause in my opinion because you have all if constexpr checks for within the function It just pollutes the code. If the intent is to provide better diagnostics what I would do is writing a __dependent_false.

So it's

template <typename... Args> inline constexpr __dependent_false = false;

And then, between line 84 and 85 I would put

static_assert(__dependent_false<R>) // or another trick without __dependent_false. static_assert(sizeof(R) == 0); // which is always false

To me it's much simpler than listing all the conditions twice. You have several such places in this PR

* Updates from Adam's comments * Update for clang-format * Slight fix * Fix for `inclusive_scan` / `exclusive_scan` of size 0 * enabled back empty test in xclusive scans --------- Co-authored-by: Łukasz Ślusarczyk <lukasz.slusarczyk@intel.com>

lslusarczyk and others added 14 commits March 13, 2024 12:21

copied shp and common

318f723

added tests

1ac29d0

some compilation fixes, cmake now seems to be complete

8557b72

cleanups

440e673

cleanups of include paths

2f4897f

cleanups of include paths and error messages

9189849

Merge branch 'dist-ranges_cleanup' of github.com:mateuszpn/oneDPL int…

11fdbe7

…o dist-ranges_cleanup

Distributed Ranges cleanup (#1448)

e371685

* cleanups * cleanups of include paths * cleanups of include paths and error messages

shp namespace moved to experimental

cd56589

namespace update

9e97305

update

d492476

update

477b204

CI updated for distributed-ranges

0152437

Merge branch 'oneapi-src:main' into dr_namespace

5e43e65

BenBrock mentioned this pull request Apr 4, 2024

Distributed-ranges namespace #1475

Closed

MikeDvorskiy and others added 15 commits April 5, 2024 14:17

[oneDPL][hetero] + missed synch between patterns,removed unnecessary …

2f84319

…synch, + comments

Merge branch 'oneapi-src:main' into dr_namespace

bc74b46

[oneDPL][sycl] + sycl::is_device_copyable specialization of the SYCL …

658e465

…trait for some oneDPL types

[oneDPL][sycl] + sycl::is_device_copyable specialization fixes

8e943bb

[oneDPL][sycl] sycl::is_device_copyable specialization: + clang format

f31af79

[oneDPL][sycl] sycl::is_device_copyable specialization: + a comment

35bb6bf

[oneDPL][sycl] + #include "sycl_traits.h"

80cf60b

[oneDPL][sycl] + forward declaration for __early_exit_find_or

89c92c3

[oneDPL][sycl] + necessary includes

60884a2

Revert "[oneDPL][sycl] + necessary includes"

ddaf6fe

This reverts commit cb7259d.

[oneDPL][sycl] + necessary forward declarations

797498d

[oneDPL][sycl] include place changed

03c69f1

[oneDPL][sycl][dpcpp] #include "sycl_traits.h" //SYCL traits speciali…

1047d4a

…zation for some oneDPL types.

[oneDPL] removed _ONEDPL_DEVICE_COPYABLE(zip_forward_iterator) due to…

db83006

… zip_forward_iteratoris not used in the device code

remove matrices and logs

5e98434

akukanov mentioned this pull request Aug 1, 2024

Fixes in __detail namespace #1729

Merged

distributed_device_policy renamed to sycl_device_collection (#1746)

81fb32e

Co-authored-by: Łukasz Ślusarczyk <112692748+lslusarczyk@users.noreply.github.com>

MikeDvorskiy reviewed Aug 1, 2024

View reviewed changes

lslusarczyk mentioned this pull request Aug 1, 2024

removed fixranges #1750

Merged

akukanov mentioned this pull request Aug 1, 2024

distributed_device_policy renamed to sycl_device_collection #1746

Merged

adamfidel reviewed Aug 1, 2024

View reviewed changes

julianmi reviewed Aug 1, 2024

View reviewed changes

lslusarczyk and others added 2 commits August 1, 2024 21:44

removed fixranges (#1750)

3ca7233

concepts.hpp moved and included in dpl/distributed_ranges (#1748)

e760ce0

* concepts.hpp moved and included in dpl/distributed_ranges * fixes after merge --------- Co-authored-by: Łukasz Ślusarczyk <112692748+lslusarczyk@users.noreply.github.com>

lslusarczyk mentioned this pull request Aug 2, 2024

removed is_owning_view #1753

Merged

lslusarczyk and others added 2 commits August 2, 2024 12:06

removed unused is_owning_view (#1753)

cc69962

akukanov reviewed Aug 2, 2024

View reviewed changes

MikeDvorskiy reviewed Aug 2, 2024

View reviewed changes

lslusarczyk and others added 3 commits August 2, 2024 20:39

disabled all tests except distributed-ranges to speed up applying com…

d6f21d3

…ments (#1756)

sp/zip_view.hpp -> sp/views/zip.hpp (#1754)

530380b

Co-authored-by: Łukasz Ślusarczyk <lukasz.slusarczyk@intel.com>

MikeDvorskiy reviewed Aug 5, 2024

View reviewed changes

lslusarczyk and others added 2 commits August 5, 2024 15:15

applied Mike comments about foraech (#1758)

f3a1df5

rarutyun reviewed Aug 7, 2024

View reviewed changes

Updates from Adam's comments (#1766)

68f59fe

* Updates from Adam's comments * Update for clang-format * Slight fix * Fix for `inclusive_scan` / `exclusive_scan` of size 0 * enabled back empty test in xclusive scans --------- Co-authored-by: Łukasz Ślusarczyk <lukasz.slusarczyk@intel.com>

akukanov removed this from the 2022.7.0 milestone Aug 9, 2024

BenBrock mentioned this pull request Oct 28, 2024

Implement direct_iterator and make_direct_iterator #861

Open


		TYPED_TEST_SUITE(Reduce, AllTypes);

		TYPED_TEST(Reduce, Range) {


		auto local_segment = __detail::local(segment);

		auto first = stdrng::begin(local_segment);


		#include "detail/ranges.hpp"

		namespace oneapi::dpl::experimental::dr

-    template <typename... Args>
-    requires(sizeof...(Args) >= 1 &&
-             !((sizeof...(Args) == 1 && (std::is_same_v<nonconst_iterator, std::decay_t<Args>> || ...)) ||
-               (std::is_same_v<const_iterator, std::decay_t<Args>> || ...) ||
-               (std::is_same_v<nonconst_accessor_type, std::decay_t<Args>> || ...) ||
-               (std::is_same_v<const_accessor_type, std::decay_t<Args>> || ...)) &&
-             std::is_constructible_v<accessor_type, Args...>) iterator_adaptor(Args&&... args)
-        : accessor_(std::forward<Args>(args)...)
+    template <typename Arg, typename... Args>
+        requires(sizeof...(Args) > 0
+                 && !std::is_same_v<nonconst_iterator, std::decay_t<Arg>
+                 && !std::is_same_v<const_iterator, std::decay_t<Arg>>
+                 && !std::is_same_v<nonconst_accessor_type, std::decay_t<Arg>>
+                 && !std::is_same_v<const_accessor_type, std::decay_t<Arg>>
+                 && std::is_constructible_v<accessor_type, Arg, Args...>)
+     iterator_adaptor(Arg&& arg, Args&&... args)
+        : accessor_(std::forward<Arg>(arg), std::forward<Args>(args)...)
+        {
+        }

Add distributed ranges as experimental feature. #1479

Are you sure you want to change the base?

Add distributed ranges as experimental feature. #1479

Conversation

BenBrock commented Apr 4, 2024

MikeDvorskiy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenBrock Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lslusarczyk Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akukanov Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeDvorskiy Aug 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rarutyun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenBrock Aug 5, 2024 •

edited

Loading

lslusarczyk Aug 5, 2024 •

edited

Loading

akukanov Aug 2, 2024 •

edited

Loading

MikeDvorskiy Aug 2, 2024 •

edited

Loading