Skip to content

Port existing zero_dim_dispatch optimizations from codegen and remove codegen capability.#37615

Closed
gchanan wants to merge 9 commits intogh/gchanan/265/basefrom
gh/gchanan/265/head
Closed

Port existing zero_dim_dispatch optimizations from codegen and remove codegen capability.#37615
gchanan wants to merge 9 commits intogh/gchanan/265/basefrom
gh/gchanan/265/head

Conversation

@gchanan
Copy link
Copy Markdown
Contributor

@gchanan gchanan commented Apr 30, 2020

Stack from ghstack:

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal. There is only one left with fmod.

Differential Revision: D21338030

… codegen capability.

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

[ghstack-poisoned]
… and remove codegen capability."

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

[ghstack-poisoned]
@gchanan gchanan requested a review from bhosmer April 30, 2020 22:28
… and remove codegen capability."

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

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

[ghstack-poisoned]
@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented Apr 30, 2020

💊 Build failures summary and remediations

As of commit 1337a13 (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


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 24 times.

… and remove codegen capability."

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

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

[ghstack-poisoned]
gchanan added a commit that referenced this pull request Apr 30, 2020
… codegen capability.

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

ghstack-source-id: bc6df53
Pull Request resolved: #37615
Copy link
Copy Markdown

@bhosmer bhosmer left a comment

Choose a reason for hiding this comment

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

LG, couple notes:

  • haven't double checked that the new routing logic added in LegacyDefinitions.cpp replaces 100% of the pairs discovered by discover_zero_dim_tensor_operations
  • happened to notice #37580, linked from #33094, which does the same codegen removal but without replacing it in the implementations. Looks like it's being worked on now, so might want to give them a heads up?

… and remove codegen capability."

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

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

[ghstack-poisoned]
gchanan added a commit that referenced this pull request May 4, 2020
… codegen capability.

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

ghstack-source-id: 188ed70
Pull Request resolved: #37615
… and remove codegen capability."

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

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

[ghstack-poisoned]
gchanan added a commit that referenced this pull request May 4, 2020
… codegen capability.

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

ghstack-source-id: f7fcb62
Pull Request resolved: #37615
@gchanan
Copy link
Copy Markdown
Contributor Author

gchanan commented May 4, 2020

haven't double checked that the new routing logic added in LegacyDefinitions.cpp replaces 100% of the pairs discovered by discover_zero_dim_tensor_operations

I checked the codegen using tool/git_add_generated_dirs.sh. That's not particularly interesting though, we missed a lot of these optimizations when porting, these are just the ones we are seeing now.

@gchanan
Copy link
Copy Markdown
Contributor Author

gchanan commented May 4, 2020

happened to notice #37580, linked from #33094, which does the same codegen removal but without replacing it in the implementations. Looks like it's being worked on now, so might want to give them a heads up?

I didn't look into that much detail but I think the analysis there is just wrong -- like, it's talking about lines in the codegen script and not looking at what is actually output in the codegen. But again, I don't think it's a particularly big deal, I'll just make this PR apply the optimization.

… and remove codegen capability."

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

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

[ghstack-poisoned]
gchanan added a commit that referenced this pull request May 5, 2020
… codegen capability.

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

ghstack-source-id: 4c2e310
Pull Request resolved: #37615
… and remove codegen capability."

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

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

[ghstack-poisoned]
… and remove codegen capability."

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

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

[ghstack-poisoned]
gchanan added a commit that referenced this pull request May 5, 2020
… codegen capability.

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

ghstack-source-id: f66ec96
Pull Request resolved: #37615
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@gchanan merged this pull request in f29f96d.

@facebook-github-bot facebook-github-bot deleted the gh/gchanan/265/head branch May 10, 2020 14:16
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
… codegen capability. (pytorch#37615)

Summary:
Pull Request resolved: pytorch#37615

We probably missed a lot of these when we ported things from TH, but it's also probably not a huge deal.  There is only one left with fmod.

Test Plan: Imported from OSS

Reviewed By: bhosmer

Differential Revision: D21338030

Pulled By: gchanan

fbshipit-source-id: c133b4e37df87a53797939e9f757cea9446834e8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants