oneDPL
oneDPL copied to clipboard
Add distributed ranges as experimental feature.
This draft PR adds distributed ranges as an experimental feature, inside the onedpl::experimental::dr
namespace.
Distributed Ranges branch is ready to be reviewed by oneDPL developers.
It passed all existing oneDPL CI tasks: https://github.com/oneapi-src/oneDPL/actions/runs/9128713306
It passed our tests executed by CI running in our repo that checkouts and run distributed ranges tests existing on the distributed-ranges branch of the oneDPL repo: https://github.com/oneapi-src/distributed-ranges/actions/workflows/onedpl.yml
I've just reviewed it again myself after finishing all code movement tasks and I've finished all cleanups I found myself in the code.
@akukanov It seems we addressed all comments that you had so far. Are there more comments? If not, then could we please work toward merging this code to main branch?
Tests are passing. We have also added DR UTs to your CI and it is passing. I think it is better when this code will be sooner than later on main branch and will be tested during every PR. In case of new issues found later we will resolve them on main branch.
What do you think?
This is a first pass with higher-level comments.
@julianmi , thank you for review. I've applied and answered all your comments. Please resolve discussions which you think I addressed correctly or answer discussions which you think need future work/discussion. I'm looking forward for the next pass of comments.
@julianmi , I've applied unresolved discussions from yesterday. Please check if all is ok now and resolve what is ok.
@akukanov , all, Here is documentation of Distributed-Ranges API which you may both use as a help during review and you may review it itself checking if it is clear enough.
A couple of comments regarding the CI.
@julianmi , thank you for your valuable comments. In https://github.com/oneapi-src/oneDPL/pull/1663 PR I've refactored Distributed Ranges CI to be tested in the same way other oneDPL tests are run. Please take a look and resolve discussions if it looks good to you now.
@danhoeflinger , thank you for the review and valuable comments. I've applied or answered all of them except https://github.com/oneapi-src/oneDPL/pull/1479#discussion_r1670894363 , which needs some more work and experiments from my side - I will apply this one tomorrow. In the meantime please check if other comments are well applied and resolve the ones which are OK and comment the ones which aren't OK.
In this PR we have a lot of destructors, declared as = default
.
I understand why we need that in case when destructor also declared as virtual
.
But for that in other cases?
Exaxples:
~iterator_adaptor() = default;
~direct_iterator() noexcept = default;
~device_ptr() noexcept = default;
```;
and so far and so on...
In this PR we have a lot of destructors, declared as
= default
. I understand why we need that in case when destructor also declared asvirtual
. But for that in other cases?Exaxples:
~iterator_adaptor() = default; ~direct_iterator() noexcept = default; ~device_ptr() noexcept = default; ```; and so far and so on...
applied in https://github.com/oneapi-src/oneDPL/pull/1745