8000 MAINT: Harmonize argsort params with array_api by HaoZeke · Pull Request #75162 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

MAINT: Harmonize argsort params with array_api #75162

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 16 commits into from
Closed

MAINT: Harmonize argsort params with array_api #75162

wants to merge 16 commits into from

Conversation

HaoZeke
Copy link
Collaborator
@HaoZeke HaoZeke commented Apr 3, 2022

Closes #70922.

  • Does what it says on the tin.
  • No non-standard implementation details.

@HaoZeke HaoZeke requested a review from bdhirsh as a code owner April 3, 2022 20:38
@facebook-github-bot
Copy link
Contributor
facebook-github-bot commented Apr 3, 2022 •
edited
Loading

🔗 Helpful links

✅ No Failures (0 Pending)

As of commit 084c9e2 (more details on the Dr. CI page):

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@HaoZeke HaoZeke force-pushed the argsort_params branch 2 times, most recently from 56234bf to c9e6584 Compare April 3, 2022 22:49
@mruberry mruberry added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Apr 4, 2022
mruberry
mruberry reviewed Apr 4, 2022
View reviewed changes
mruberry
mruberry reviewed Apr 4, 2022
View reviewed changes
mruberry
mruberry reviewed Apr 4, 2022
View reviewed changes
@HaoZeke HaoZeke force-pushed the argsort_params branch 2 times, most recently from 5036353 to 972614d Compare April 10, 2022 19:41
@HaoZeke HaoZeke requested a review from mruberry April 11, 2022 00:24
@rgommers rgommers requested a review from nikitaved April 11, 2022 10:44
@rgommers
Copy link
Collaborator

@nikitaved since IIRC you wrote the stable sorting algorithm that's touched here, can you do the initial review on this PR?

nikitaved
nikitaved reviewed Apr 13, 2022
View reviewed changes
nikitaved
nikitaved reviewed Apr 13, 2022
View reviewed changes
nikitaved
nikitaved reviewed Apr 14, 2022
View reviewed changes
@HaoZeke HaoZeke removed the request for review from bdhirsh April 24, 2022 21:32
@HaoZeke HaoZeke force-pushed the argsort_params branch 5 times, most recently from dda9498 to 48b8c30 Compare April 25, 2022 07:27
rgommers
rgommers reviewed Apr 25, 2022
View reviewed changes
nikitaved
nikitaved reviewed Apr 25, 2022
View reviewed changes
@HaoZeke HaoZeke requested a review from rgommers April 29, 2022 13:51
HaoZeke and others added 16 commits May 29, 2022 23:09
@HaoZeke
ENH: Harmonize argsort CPU with array_api
eae0483
@HaoZeke
DOC: Update docstrings for argsort
3854caa
@HaoZeke
ENH: Harmonize argsort cuda with array_api
e58e707
@HaoZeke
MAINT: argsort defaults from array_api
201f3d3
@HaoZeke
MAINT: Reduce argsort ambiguity
7b9353a
@HaoZeke
MAINT, DOC: Clean up ambiguities, better docs
1766424 
Co-authored-by: Mike Ruberry <mruberry@fb.com>
@HaoZeke @nikitaved
MAINT: Address review comments
ff25a56 
Co-authored-by: nikitaved <nikitavedeneev@gmail.com>
@HaoZeke
MAINT: Cleanup docstring [skip-ci]
f2e54fb
@HaoZeke
TST: Add a test for argsort parameters
62914d8
@HaoZeke
MAINT: Fix linter errors on argsort
07e4752
@HaoZeke
MAINT: Clear compatibility hurdle
3747cf5
@HaoZeke
MAINT: Update argsort after structured kernel port
0601a12
@HaoZeke
TST: Don't try to predict unstable argsort
561a51c
@HaoZeke @nikitaved
TST: Add a test without stable
9c1f854 
Co-authored-by: nikitaved <nikitavedeneev@gmail.com>
@HaoZeke @rgommers
DOC: Update to reflect native_functions
e9a4846 
Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
@HaoZeke
MAINT: Remove fwd_bck since no signatures are modified
084c9e2
@HaoZeke
Copy link
Collaborator Author
HaoZeke commented Jun 6, 2022

@mruberry, is this alright?

mruberry
mruberry approved these changes Jun 9, 2022
View reviewed changes
Copy link
Collaborator
@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @HaoZeke!

@HaoZeke
Copy link
Collaborator Author
HaoZeke commented Jun 9, 2022 6DB6

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@github-actions
Copy link
Contributor
github-actions bot commented Jun 9, 2022

Hey @HaoZeke.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

facebook-github-bot pushed a commit that referenced this pull request Jun 10, 2022
@HaoZeke @facebook-github-bot
MAINT: Harmonize argsort params with array_api (#75162) (#75162)
4581057 
Summary:
Closes [#70922](#70922).
- Does what it says on the tin.
-  No non-standard implementation details.

Pull Request resolved: #75162
Approved by: https://github.com/rgommers, https://github.com/nikitaved, https://github.com/mruberry

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/04407431ffe529d3811802674a1a771c5c6562e4

Reviewed By: osalpekar

Differential Revision: D37058996

Pulled By: osalpekar

fbshipit-source-id: d8dedd8d1892f45448c356558774a8c1b20d96d2
@rgommers rgommers mentioned this pull request Mar 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgommers rgommers rgommers approved these changes

@nikitaved nikitaved nikitaved approved these changes

@mruberry mruberry mruberry approved these changes

Assignees
No one assigned
Labels
cla signed Merged module: python array api Issues related to the Python Array API open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

argsort is missing the stable parameter
7 participants
@HaoZeke @facebook-github-bot @rgommers @pytorchmergebot @nikitaved @mruberry @pytorchbot
0