Skip to content

Avoid copies in sorting#1967

Merged
oleksandr-pavlyk merged 3 commits intomasterfrom
avoid-copies-in-sorting
Jan 15, 2025
Merged

Avoid copies in sorting#1967
oleksandr-pavlyk merged 3 commits intomasterfrom
avoid-copies-in-sorting

Conversation

@oleksandr-pavlyk
Copy link
Contributor

@oleksandr-pavlyk oleksandr-pavlyk commented Jan 14, 2025 •
edited
Loading

Use forwarding references to avoid copies of comparator cpp-function, and of iterator.

Also avoid invoking map_back kernel in argsort when arg-sorting 1d array, i.e. when iter_nelems == 1.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • Have you added documentation for your changes, if necessary?
  • Have you added your changes to the changelog?
  • If this PR is a work in progress, are you opening the PR as a draft?

@oleksandr-pavlyk
Use forwarding reference in leaf_sort_impl to avoid copies
082fca1 
Also remove explicit specialization in std::min, quotient_ceil
calls where both arguments have the same type so that type deduction
works.
@oleksandr-pavlyk
Do not invoke map_back kernel if iter_nelems is 1
9a7868d 
The map_back operation is a no-op then.
@github-actions
Copy link

github-actions bot commented Jan 14, 2025 •
edited
Loading

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

@github-actions
Copy link

github-actions bot commented Jan 14, 2025

Array API standard conformance tests for dpctl=0.19.0dev0=py310h93fe807_442 ran successfully.
Passed: 895
Failed: 1
Skipped: 118

@coveralls
Copy link
Collaborator

coveralls commented Jan 14, 2025 •
edited
Loading

Coverage Status

coverage: 87.683%. remained the same
when pulling 4847101 on avoid-copies-in-sorting
into 25a53f5 on master.

ndgrigorian
ndgrigorian reviewed Jan 15, 2025
View reviewed changes
ndgrigorian
ndgrigorian previously approved these changes Jan 15, 2025
View reviewed changes
Copy link
Collaborator

@ndgrigorian ndgrigorian left a comment

Choose a reason for hiding this comment

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

Aside from that small nit LGTM!

ndgrigorian
ndgrigorian previously approved these changes Jan 15, 2025
View reviewed changes
@ndgrigorian
Copy link
Collaborator

ndgrigorian commented Jan 15, 2025

@oleksandr-pavlyk
Pre-commit seems to be complaining about that change, I'll let you push the fix so I can approve while keeping GitHub rule checker from complaining

@oleksandr-pavlyk @ndgrigorian
Update dpctl/tensor/libtensor/include/kernels/sorting/merge_sort.hpp
4847101 
Use bracketed if statement

Co-authored-by: ndgrigorian <46709016+ndgrigorian@users.noreply.github.com>
ndgrigorian
ndgrigorian approved these changes Jan 15, 2025
View reviewed changes
Copy link
Collaborator

@ndgrigorian ndgrigorian left a comment

Choose a reason for hiding this comment

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

Pre-commit fixed again, so I will approve, and hopefully it won't be too hasty this time :)

@github-actions
Copy link

github-actions bot commented Jan 15, 2025

Array API standard conformance tests for dpctl=0.19.0dev0=py310h93fe807_450 ran successfully.
Passed: 893
Failed: 3
Skipped: 118

@github-actions
Copy link

github-actions bot commented Jan 15, 2025

Array API standard conformance tests for dpctl=0.19.0dev0=py310h93fe807_452 ran successfully.
Passed: 894
Failed: 2
Skipped: 118

@oleksandr-pavlyk oleksandr-pavlyk merged commit 2e8c9c0 into master Jan 15, 2025
60 of 62 checks passed
@oleksandr-pavlyk oleksandr-pavlyk deleted the avoid-copies-in-sorting branch January 15, 2025 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ndgrigorian ndgrigorian ndgrigorian approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@oleksandr-pavlyk @coveralls @ndgrigorian