Implement argsort and make sort numpy API compliant#2386
Conversation
…analytics/heat into 777-Implement_argsort
- Fixed argsort test. - Fixed sort calls for old the sort API
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Thanks! I think it's a good idea to be closer to Numpy and to support argsort. However, I don't think we should silently ignore additional arguments that Numpy users are used to.
Note that the errors in the collection step in pytest happen all over the place, including in other repositories, and have nothing to do with this PR.
| Returns the indices that would sort an array. This is the distributed equivalent of `np.argsort`. | ||
| The sorting is not stable which means that equal elements in the result may have a different ordering than in the | ||
| original array. | ||
| Sorting where `axis==a.split` needs a lot of communication between the processes of MPI. |
There was a problem hiding this comment.
| Sorting where `axis==a.split` needs a lot of communication between the processes of MPI. | |
| Sorting with `axis==a.split` needs a lot of communication between the processes. |
| kind : str, optional | ||
| Sorting algorithm. Gets ignored. | ||
| order : str, list[str], optional | ||
| Used in numpy for array with fields, which are not possible in HeAT. Gets ignored. |
There was a problem hiding this comment.
| Used in numpy for array with fields, which are not possible in HeAT. Gets ignored. | |
| Used in numpy for array with fields, which are not possible in Heat. Gets ignored. |
| kind : str, optional | ||
| Sorting algorithm. Gets ignored. | ||
| order : str, list[str], optional | ||
| Used in numpy for array with fields, which are not possible in HeAT. Gets ignored. | ||
| stable : bool, optional | ||
| Sort stability, currenty not supported. |
There was a problem hiding this comment.
Do we really want to allow arguments that are ignored? What's the policy on this, @ClaudiaComito?
At least, I would assert not stable and raise a warning that the parameters are ignored rather than silently ignoring them.
|
Excellent! If you apply something similar to |
4 participants
Due Diligence
Description
argsortandsorthave been changed to reflect the Numpy API, while also allowing old usage.Issue/s resolved: #777
Type of change
Does this change modify the behaviour of other functions? If so, which?
yes,
sortnow does not give back the indices when sorting like pytorch. Instead a flag must be given to achieve same behaviour.