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

Ensure remainder of advanced indexing backwards is contiguous. #36957

Closed
wants to merge 3 commits into from

Conversation

gchanan
Copy link
Contributor

@gchanan gchanan commented Apr 20, 2020

Stack from ghstack:

This is attempt to make the current master behavior the same as on 1.4. I don't know if this is more performant than just making the linearIndex contiguous.

Fixes: #36956

Differential Revision: D21144107

This is attempt to make the current master behavior the same as on 1.4.  I don't know if this is more performant than just making the linearIndex contiguous.

Fixes: #36956

[ghstack-poisoned]
gchanan added a commit that referenced this pull request Apr 20, 2020
This is a more straightforward solution to the problem than #36957; I don't know about the relative performance.

Fixes: #36956

[ghstack-poisoned]
gchanan added a commit that referenced this pull request Apr 20, 2020
…guous."

This is a more straightforward solution to the problem than #36957; I don't know about the relative performance.

Fixes: #36956

Differential Revision: [D21144146](https://our.internmc.facebook.com/intern/diff/D21144146)

[ghstack-poisoned]
…ous."

This is attempt to make the current master behavior the same as on 1.4.  I don't know if this is more performant than just making the linearIndex contiguous.

Fixes: #36956

Differential Revision: [D21144107](https://our.internmc.facebook.com/intern/diff/D21144107)

[ghstack-poisoned]
@gchanan gchanan requested review from albanD and ngimel April 20, 2020 22:47
gchanan added a commit to gchanan/pytorch that referenced this pull request Apr 20, 2020
This is attempt to make the current master behavior the same as on 1.4.  I don't know if this is more performant than just making the linearIndex contiguous.

Fixes: pytorch#36956

ghstack-source-id: f5ac46a6e77dce2e82396079d2ec426953ac5966
Pull Request resolved: pytorch#36957
Copy link
Collaborator

@ngimel ngimel left a comment

Choose a reason for hiding this comment

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

This should be more performant than making linearIndex contiguous, so it's good.

Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Looks good !

gchanan added a commit that referenced this pull request Apr 20, 2020
…guous."

This is a more straightforward solution to the problem than #36957; I don't know about the relative performance.

Fixes: #36956

Differential Revision: [D21144146](https://our.internmc.facebook.com/intern/diff/D21144146)

[ghstack-poisoned]
gchanan added a commit to gchanan/pytorch that referenced this pull request Apr 20, 2020
…uous.

This is a more straightforward solution to the problem than pytorch#36957; I don't know about the relative performance.

Fixes: pytorch#36956

ghstack-source-id: 43c48eaee7232cd3ed2b108edbbee24c11e8321a
Pull Request resolved: pytorch#36959
@dr-ci
Copy link

dr-ci bot commented Apr 20, 2020

💊 Build failures summary and remediations

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


  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following build failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_macos_10_13_py3_test (1/1)

Step: "Test" (full log | pattern match details | 🔁 rerun)

Apr 20 16:13:58 RuntimeError: test_autograd failed!
Apr 20 16:13:57 Generating XML reports... 
Apr 20 16:13:58 Generated XML report: test-reports/python-unittest/TEST-TestAutograd-20200420161018.xml 
Apr 20 16:13:58 Generated XML report: test-reports/python-unittest/TEST-TestAutogradDeviceTypeCPU-20200420161018.xml 
Apr 20 16:13:58 Generated XML report: test-reports/python-unittest/TEST-TestAutogradFunctional-20200420161018.xml 
Apr 20 16:13:58 Generated XML report: test-reports/python-unittest/TEST-TestMultithreadAutograd-20200420161018.xml 
Apr 20 16:13:58 Traceback (most recent call last): 
Apr 20 16:13:58   File "test/run_test.py", line 699, in <module> 
Apr 20 16:13:58     main() 
Apr 20 16:13:58   File "test/run_test.py", line 692, in main 
Apr 20 16:13:58     raise RuntimeError(message) 
Apr 20 16:13:58 RuntimeError: test_autograd failed! 
Apr 20 16:13:58 + cleanup 
Apr 20 16:13:58 + retcode=1 
Apr 20 16:13:58 + set +x 

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 on the GitHub issue tracker.

See how this bot performed.

This comment has been revised 2 times.

gchanan added a commit that referenced this pull request Apr 20, 2020
…guous."

This is a more straightforward solution to the problem than #36957; I don't know about the relative performance.

Fixes: #36956

Differential Revision: [D21144146](https://our.internmc.facebook.com/intern/diff/D21144146)

[ghstack-poisoned]
gchanan added a commit that referenced this pull request Apr 20, 2020
This is a more straightforward solution to the problem than #36957; I don't know about the relative performance.

Fixes: #36956

ghstack-source-id: 6bfefbc5ceb8d0000b4a686fdc410f5fcf45cc1d
Pull Request resolved: #36959
…ous."

This is attempt to make the current master behavior the same as on 1.4.  I don't know if this is more performant than just making the linearIndex contiguous.

Fixes: #36956

Differential Revision: [D21144107](https://our.internmc.facebook.com/intern/diff/D21144107)

[ghstack-poisoned]
gchanan added a commit that referenced this pull request Apr 20, 2020
This is attempt to make the current master behavior the same as on 1.4.  I don't know if this is more performant than just making the linearIndex contiguous.

Fixes: #36956

ghstack-source-id: d9512fe1d23921c49ed422f769c6b3a18040e3a0
Pull Request resolved: #36957
gchanan added a commit that referenced this pull request Apr 20, 2020
#36962)

* [v.1.5.0] Ensure linearIndex of advanced indexing backwards is contiguous.

This is a more straightforward solution to the problem than #36957; I don't know about the relative performance.

Fixes: #36956

ghstack-source-id: 43c48eaee7232cd3ed2b108edbbee24c11e8321a
Pull Request resolved: #36959

* Fix test.
facebook-github-bot pushed a commit that referenced this pull request Apr 21, 2020
)

Summary:
Pull Request resolved: #36959

This is a more straightforward solution to the problem than #36957; I don't know about the relative performance.

Fixes: #36956

Test Plan: Imported from OSS

Differential Revision: D21144146

Pulled By: gchanan

fbshipit-source-id: a10ab905219a73157d5d7183492b52d7c8dd6072
@gchanan gchanan closed this May 5, 2020
@facebook-github-bot facebook-github-bot deleted the gh/gchanan/253/head branch June 5, 2020 14:16
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.

None yet

3 participants