Stable sort for CPU by nikitaved · Pull Request #50052 · pytorch/pytorch

nikitaved · 2021-01-04T18:44:04Z

Fixes #38681 for the CPU.

facebook-github-bot · 2021-01-04T18:44:25Z

💊 CI failures summary and remediations

As of commit 87635b4 (more details on the Dr. CI page):

1/1 failures possibly* introduced in this PR
- 1/1 non-CircleCI failure(s)

ci.pytorch.org: 1 failed

Failed: pr/pytorch-linux-bionic-rocm4.0.1-py3.6

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

Follow this link to opt-out of these comments for your Pull Requests.

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

nikitaved · 2021-01-04T18:46:33Z

argsort does belong to the legacy functionality. Does it actually make sense to extend it as well?

ezyang · 2021-01-05T15:23:51Z

@glaringlee you reviewed the port on this code, can you review this PR too?

glaringlee · 2021-01-06T00:03:34Z

@ailzhang This breaks the xla build due to signature change in sorting functions. Can you advise?

glaringlee · 2021-01-06T00:17:14Z

@nikitaved pytorch_linux_backward_compatibility_check_test is broken since the signature of the sort function changed, please add the changed function into the following list to bypass them, expiration date 01/31/2021 should be fine.
https://github.com/pytorch/pytorch/blob/master/test/backward_compatibility/check_backward_compatibility.py#L53

Here is a quick link to the compatibility check error log.
https://app.circleci.com/pipelines/github/pytorch/pytorch/256847/workflows/dfb6662c-2cf4-45f4-85a9-840b3547bb34/jobs/9959226

JackCaoG · 2021-01-06T00:29:29Z

@glaringlee I will draft a pr to pt/xla to address the upstream signature change

glaringlee · 2021-01-06T16:22:01Z

argsort does belong to the legacy functionality. Does it actually make sense to extend it as well?

This is what we current do for argsort.
https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/LegacyDefinitions.cpp#L28
It would be good to extend it, can we do it in a separate PR?

…taved/stable_sort_CPU

glaringlee · 2021-01-11T18:22:55Z

@nikitaved
test_torch.py need to apply the same thing, adding onlyCPU for stable sort one.
Here is an example:

pytorch/test/test_torch.py

Line 6706 in 20a3882

[torch.int16, torch.int32, torch.int64], True, [onlyOnCPUAndCUDA]),

…taved/stable_sort_CPU

glaringlee · 2021-01-12T20:05:43Z

@JackCaoG @ailzhang This is ready to land. Should I land this code first or you guys land xla code first? thx

glaringlee · 2021-01-12T20:07:05Z

@nikitaved Can you please rebase the code to resolve the conflict in check_backwrard_compatibility.py?

JackCaoG · 2021-01-12T20:19:29Z

@JackCaoG @ailzhang This is ready to land. Should I land this code first or you guys land xla code first? thx

@glaringlee you can land this one first, I will merge the xla one once this is merged

…mmon_methods_invocations.py

…taved/stable_sort_CPU_take_2

… sort

…taved/stable_sort_CPU_take_2

Summary: Fixes #38681. A duplicate of #50052 created to become importable to the fb internal tests. Pull Request resolved: #51790 Reviewed By: agolynski Differential Revision: D26279045 Pulled By: glaringlee fbshipit-source-id: 348e171dee9c370a76002b65d0c82c329f57a421

Summary: Fixes pytorch#38681. A duplicate of pytorch#50052 created to become importable to the fb internal tests. Pull Request resolved: pytorch#51790 Reviewed By: agolynski Differential Revision: D26279045 Pulled By: glaringlee fbshipit-source-id: 348e171dee9c370a76002b65d0c82c329f57a421

rgommers · 2021-03-09T15:04:13Z

Completed in gh-51790, closing.

Summary: Fixes pytorch#38681. A duplicate of pytorch#50052 created to become importable to the fb internal tests. Pull Request resolved: pytorch#51790 Reviewed By: agolynski Differential Revision: D26279045 Pulled By: glaringlee fbshipit-source-id: 348e171dee9c370a76002b65d0c82c329f57a421

Summary: Fixes [https://github.com/pytorch/pytorch/issues/38681](https://github.com/pytorch/pytorch/issues/38681) for the CPU. Pull Request resolved: pytorch#50052 Reviewed By: mrshenli Differential Revision: D25900823 Pulled By: glaringlee fbshipit-source-id: 1a3fa336037d0aa2344d79f46dcacfd478a353d1

Summary: This reverts commit 4d82d19. Pull Request resolved: pytorch#50752 Reviewed By: zou3519 Differential Revision: D25958146 Pulled By: glaringlee fbshipit-source-id: f4068d038f9bd337bac8b673eaeb46a4646f6c77

Summary: Fixes pytorch#38681. A duplicate of pytorch#50052 created to become importable to the fb internal tests. Pull Request resolved: pytorch#51790 Reviewed By: agolynski Differential Revision: D26279045 Pulled By: glaringlee fbshipit-source-id: 348e171dee9c370a76002b65d0c82c329f57a421

facebook-github-bot added the cla signed label Jan 4, 2021

pytorchbot added the open source label Jan 4, 2021

nikitaved mentioned this pull request Jan 4, 2021

Make sort support stable sort, CPU. #45501

Closed

nikitaved force-pushed the nikitaved/stable_sort_CPU branch from e858b7f to ba197ee Compare January 5, 2021 09:29

stable sort for CPU

5d2639f

nikitaved force-pushed the nikitaved/stable_sort_CPU branch from ba197ee to 5d2639f Compare January 5, 2021 11:13

ezyang requested a review from glaringlee January 5, 2021 15:23

ezyang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jan 5, 2021

glaringlee reviewed Jan 6, 2021

View reviewed changes

Comment thread aten/src/ATen/cuda/LegacyTHFunctionsCUDA.cpp Outdated

Comment thread aten/src/ATen/cuda/LegacyTHFunctionsCUDA.cpp Outdated

Comment thread test/test_sort_and_select.py Outdated

Comment thread torch/_torch_docs.py Outdated

glaringlee requested a review from ailzhang January 6, 2021 00:03

JackCaoG mentioned this pull request Jan 6, 2021

Enable stable sort pytorch/xla#2718

Merged

nikitaved added 5 commits January 8, 2021 10:35

remove conflict in native_functions

e9fe227

disable stable on CUDA, modify tests

cf16c9e

update the doc

20e74a6

try whitelist sort for backward compatibility

f7ab98c

Merge branch 'master' of https://github.com/pytorch/pytorch into niki…

20a3882

…taved/stable_sort_CPU

nikitaved added 2 commits January 11, 2021 13:25

disable tests from test_torch for CUDA

d2ddaf6

Merge branch 'master' of https://github.com/pytorch/pytorch into niki…

4da3076

…taved/stable_sort_CPU

merge master

6d3c364

nikitaved added 17 commits February 6, 2021 15:35

implement OpInfo tests. Remove legacy tests from test_torch.py and co…

48111ad

…mmon_methods_invocations.py

better dtype handling in test_stable

771a4a9

update tests to compare against numpy

82962c5

Merge branch 'master' of https://github.com/pytorch/pytorch into niki…

f655233

…taved/stable_sort_CPU_take_2

minor comment improvement

fa8fb10

minor comment improvement 2

444e4f9

Merge branch 'master' of https://github.com/pytorch/pytorch into niki…

daeff52

…taved/stable_sort_CPU_take_2

Merge branch 'master' of https://github.com/pytorch/pytorch into niki…

0b0d61c

…taved/stable_sort_CPU_take_2

Merge branch 'master' of https://github.com/pytorch/pytorch into niki…

9e969eb

…taved/stable_sort_CPU_take_2

add internal asserts checking whether the optional stable has a value

ce33391

add stable sort for quantized tensors

5eaa86f

remove sort from being bc-breaking

11e2ff5

add test that stable=True fails on CUDA

d94b312

minor fixes for OpInfo(sort) as per comments

ad7a0e1

add example for stable=True when the result is different from regular…

4a9b513

… sort

Merge branch 'master' of https://github.com/pytorch/pytorch into niki…

3804181

…taved/stable_sort_CPU_take_2

Merge branch 'master' of https://github.com/pytorch/pytorch into niki…

87635b4

…taved/stable_sort_CPU_take_2

rgommers closed this Mar 9, 2021

nikitaved mentioned this pull request Apr 25, 2022

MAINT: Harmonize argsort params with array_api #75162

Closed

github-actions Bot deleted the nikitaved/stable_sort_CPU branch February 10, 2024 01:57

Conversation

nikitaved commented Jan 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Jan 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

ci.pytorch.org: 1 failed

Uh oh!

nikitaved commented Jan 4, 2021

Uh oh!

ezyang commented Jan 5, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

glaringlee commented Jan 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glaringlee commented Jan 6, 2021

Uh oh!

JackCaoG commented Jan 6, 2021

Uh oh!

glaringlee commented Jan 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glaringlee commented Jan 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glaringlee commented Jan 12, 2021

Uh oh!

glaringlee commented Jan 12, 2021

Uh oh!

JackCaoG commented Jan 12, 2021

Uh oh!

rgommers commented Mar 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

nikitaved commented Jan 4, 2021 •

edited

Loading

facebook-github-bot commented Jan 4, 2021 •

edited

Loading

glaringlee commented Jan 6, 2021 •

edited

Loading

glaringlee commented Jan 6, 2021 •

edited

Loading

glaringlee commented Jan 11, 2021 •

edited

Loading