Skip to content

Conversation

@dmitriy-sobolev
Copy link
Contributor

Adding nth_element into oneDPL specification.

@dmitriy-sobolev dmitriy-sobolev marked this pull request as draft July 29, 2025 16:23
@akukanov akukanov added the DPL label Jul 29, 2025
@dmitriy-sobolev dmitriy-sobolev force-pushed the add-ranges-sorting-algorithms branch from e21c85f to f63665d Compare September 19, 2025 13:30
@dmitriy-sobolev dmitriy-sobolev marked this pull request as ready for review September 19, 2025 13:30
@danhoeflinger danhoeflinger self-assigned this Sep 22, 2025
@danhoeflinger danhoeflinger self-requested a review September 22, 2025 12:33
@akukanov akukanov changed the title [oneDPL] Add nth_element parallel range algorithm [oneDPL][ranges] Add nth_element parallel range algorithm Oct 1, 2025
Copy link
Contributor

@akukanov akukanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct.

merge (ExecutionPolicy&& pol, R1&& r1, R2&& r2, OutR&& result, Comp comp = {},
Proj1 proj1 = {}, Proj2 proj2 = {});
// nth_element
Copy link
Contributor

@akukanov akukanov Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #644 we are adding a new section for partitioning algorithms.

nth_element is effectively a partitioning algorithm - it rearranges the elements of a range such that every element "to the right" is "not greater" (with respect to the comparator) than any element "to the left" (though, elements equal to the Nth one can be found at any side of it), On the other hand, the requirements for nth_element are much closer to sort than to partition.

Should we keep this algorithms in the group with sort or move it to the partition group?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it fits better in partitioning than sort.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge #644 first, and then add nth_element into the partition operations group.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formulation looks good. Moving to partitioning section makes sense though.

merge (ExecutionPolicy&& pol, R1&& r1, R2&& r2, OutR&& result, Comp comp = {},
Proj1 proj1 = {}, Proj2 proj2 = {});
// nth_element
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it fits better in partitioning than sort.

Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants