Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement sort() #312

Merged
merged 40 commits into from
Aug 2, 2019
Merged

Implement sort() #312

merged 40 commits into from
Aug 2, 2019

Conversation

TheSlimvReal
Copy link
Contributor

Targets issue #144.

Sort is implemented using some parallel sorting algorithms with pivots and a balancing afterwards. Problem case is where axis = a.split because there the sorting is affected by the values of the other processes. In the end the returned tensor has the same shape as the local input tensor. The lower processes have the lower (higher) values, if sorting is performed ascending (descending) (MPI_WORLD.rank == 0 has the lowest (highest)).

The sorting is not stable, which means that equal elements might be in a different ordering than in the original array. This leads to the problem that the indices of the elements in the original data can be different and still produce the same sorting result.

@codecov-io
Copy link

codecov-io commented Jun 25, 2019

Codecov Report

Merging #312 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #312      +/-   ##
==========================================
+ Coverage   96.99%   97.06%   +0.06%     
==========================================
  Files          53       53              
  Lines        9051     9265     +214     
==========================================
+ Hits         8779     8993     +214     
  Misses        272      272
Impacted Files Coverage Δ
heat/core/tests/test_manipulations.py 100% <100%> (ø) ⬆️
heat/core/manipulations.py 98.38% <100%> (+0.7%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0dc41cf...0ef42d7. Read the comment docs.

Copy link
Member

@Markus-Goetz Markus-Goetz left a comment

Choose a reason for hiding this comment

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

Really nice work, have minor issues noted down here.

heat/core/manipulations.py Outdated Show resolved Hide resolved
heat/core/manipulations.py Outdated Show resolved Hide resolved
heat/core/manipulations.py Show resolved Hide resolved
heat/core/manipulations.py Outdated Show resolved Hide resolved
heat/core/manipulations.py Outdated Show resolved Hide resolved
heat/core/manipulations.py Outdated Show resolved Hide resolved
Copy link
Member

@Markus-Goetz Markus-Goetz left a comment

Choose a reason for hiding this comment

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

bump

@TheSlimvReal
Copy link
Contributor Author

TheSlimvReal commented Jul 12, 2019

The code is now made more efficient to use less MPI=calls. It still needs one call for each coordinate that is not along the split axis but I don't see a way to improve this because every coordinate needs to be balanced for itself. The speed-up is roughly at about 6 for a vector of size (1000, 1) compared to the version where a MPI-call was made for every swap.

@TheSlimvReal TheSlimvReal merged commit 1722a65 into master Aug 2, 2019
@TheSlimvReal TheSlimvReal deleted the feature/144-sort branch August 2, 2019 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants