Skip to content

Improve shape analysis to cover all most commonly used ops#11358

Closed
apaszke wants to merge 4 commits intopytorch:masterfrom
apaszke:improve_shape_analysis
Closed

Improve shape analysis to cover all most commonly used ops#11358
apaszke wants to merge 4 commits intopytorch:masterfrom
apaszke:improve_shape_analysis

Conversation

@apaszke
Copy link
Contributor

@apaszke apaszke commented Sep 7, 2018

Here's a list of ops that are in register_aten_ops.cpp, but aren't supported in shape prop. Everything else should work now.

@pytorchbot pytorchbot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Sep 7, 2018
Copy link
Contributor

@zdevito zdevito left a comment

Choose a reason for hiding this comment

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

The code looks good, but this is a big change and there are no direct tests of this code. Due to how little optimization normally applies, it is very possible this code is wrong and we just don't see the effects in end-to-end runs. At the very least we should have one per operator category in shape_prop that creates a graph with a single op, runs shape prop, and checks that it matches what the op really does if it is run with example inputs.

This comment was marked as off-topic.

@apaszke apaszke force-pushed the improve_shape_analysis branch from 18ea4f1 to d0928ce Compare September 10, 2018 16:05
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

apaszke has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@apaszke apaszke force-pushed the improve_shape_analysis branch from caede43 to 3e5ad2a Compare September 10, 2018 23:49
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

apaszke has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

oncall: jit Add this issue/PR to JIT oncall triage queue open source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants