[Static Runtime] Make canEnableStaticRuntime examine sub-blocks#87396

Closed

mikeiovine wants to merge 1 commit intopytorch:masterfrom

mikeiovine:export-D40564452

mikeiovine commented Oct 20, 2022

Summary:
Someone was running into problems where

Static Runtime enablement would fail
We would try to fall back to the JIT interpreter after trying to create StaticModule
The fallback fails because Static Runtime mangled the graph.

We don't want to prevent Static Runtime from mutating its input due to memory concerns. The intent of canEnableStaticRuntime is to catch issues in the module before Static Runtime messes with it.

With this diff, StaticModule instantiation can be avoided by querying canEnableStaticRuntime and the issue is fixed.

Test Plan: New unit test

Differential Revision: D40564452

pytorch-bot bot commented Oct 20, 2022 •

edited

Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/87396

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 12ca662:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

pytorch-bot bot added the release notes: jit label

facebook-github-bot added the fb-exported label

Contributor

facebook-github-bot commented Oct 20, 2022

This pull request was exported from Phabricator. Differential Revision: D40564452

mikeiovine force-pushed the export-D40564452 branch from 0849a5d to 31ac4f4 Compare

October 20, 2022 22:52

mikeiovine pushed a commit to mikeiovine/pytorch that referenced this pull request


          [Static Runtime] Make canEnableStaticRuntime examine sub-blocks (pyto…

31ac4f4

…rch#87396)

Summary:
Pull Request resolved: pytorch#87396

Someone was running into problems where

1) Static Runtime enablement would fail
2) We would try to fall back to the JIT interpreter *after trying to create `StaticModule`*
3) The fallback fails because Static Runtime mangled the graph.

We don't want to prevent Static Runtime from mutating its input due to memory concerns. The intent of `canEnableStaticRuntime` is to catch issues in the module before Static Runtime messes with it.

With this diff, `StaticModule` instantiation can be avoided by querying `canEnableStaticRuntime` and the issue is fixed.

Test Plan: New unit test

Reviewed By: tenpercent

Differential Revision: D40564452

fbshipit-source-id: c40f0fb03c974db476e1d3525b45320cd1aac91e

Contributor

facebook-github-bot commented Oct 20, 2022

This pull request was exported from Phabricator. Differential Revision: D40564452

tenpercent approved these changes

View reviewed changes

pytorch-bot bot added the ciflow/trunk label

mikeiovine force-pushed the export-D40564452 branch from 31ac4f4 to 841ff6d Compare

October 21, 2022 14:06

Contributor

facebook-github-bot commented Oct 21, 2022

This pull request was exported from Phabricator. Differential Revision: D40564452

1 similar comment

Contributor

facebook-github-bot commented Oct 21, 2022

This pull request was exported from Phabricator. Differential Revision: D40564452

mikeiovine pushed a commit to mikeiovine/pytorch that referenced this pull request


          [Static Runtime] Make canEnableStaticRuntime examine sub-blocks (pyto…

99c1cd7

…rch#87396)

Summary:
Pull Request resolved: pytorch#87396

Someone was running into problems where

1) Static Runtime enablement would fail
2) We would try to fall back to the JIT interpreter *after trying to create `StaticModule`*
3) The fallback fails because Static Runtime mangled the graph.

We don't want to prevent Static Runtime from mutating its input due to memory concerns. The intent of `canEnableStaticRuntime` is to catch issues in the module before Static Runtime messes with it.

With this diff, `StaticModule` instantiation can be avoided by querying `canEnableStaticRuntime` and the issue is fixed.

Test Plan: New unit test

Reviewed By: tenpercent

Differential Revision: D40564452

fbshipit-source-id: f304746c52198d38b2a9a1b48a0f8a0059e654da

mikeiovine force-pushed the export-D40564452 branch from 841ff6d to 99c1cd7 Compare

October 21, 2022 15:38

Contributor

facebook-github-bot commented Oct 24, 2022

This pull request was exported from Phabricator. Differential Revision: D40564452

mikeiovine force-pushed the export-D40564452 branch from 99c1cd7 to c77e4d1 Compare

October 24, 2022 13:06

mikeiovine pushed a commit to mikeiovine/pytorch that referenced this pull request


          [Static Runtime] Make canEnableStaticRuntime examine sub-blocks (pyto…

812da0f

…rch#87396)

Summary:
Pull Request resolved: pytorch#87396

Someone was running into problems where

1) Static Runtime enablement would fail
2) We would try to fall back to the JIT interpreter *after trying to create `StaticModule`*
3) The fallback fails because Static Runtime mangled the graph.

We don't want to prevent Static Runtime from mutating its input due to memory concerns. The intent of `canEnableStaticRuntime` is to catch issues in the module before Static Runtime messes with it.

With this diff, `StaticModule` instantiation can be avoided by querying `canEnableStaticRuntime` and the issue is fixed.

Test Plan: New unit test

Reviewed By: tenpercent

Differential Revision: D40564452

fbshipit-source-id: f1111ae4628dc778be063b098d11931e64eb4352

mikeiovine force-pushed the export-D40564452 branch from c77e4d1 to 812da0f Compare

October 24, 2022 18:32

Contributor

facebook-github-bot commented Oct 24, 2022

This pull request was exported from Phabricator. Differential Revision: D40564452

tenpercent approved these changes

View reviewed changes

Contributor

facebook-github-bot commented Oct 25, 2022

This pull request was exported from Phabricator. Differential Revision: D40564452

mikeiovine force-pushed the export-D40564452 branch from 812da0f to bbd8e70 Compare

October 25, 2022 14:26

mikeiovine pushed a commit to mikeiovine/pytorch that referenced this pull request


          [Static Runtime] Make canEnableStaticRuntime examine sub-blocks (pyto…

bbd8e70

…rch#87396)

Summary:
Pull Request resolved: pytorch#87396

Someone was running into problems where

1) Static Runtime enablement would fail
2) We would try to fall back to the JIT interpreter *after trying to create `StaticModule`*
3) The fallback fails because Static Runtime mangled the graph.

We don't want to prevent Static Runtime from mutating its input due to memory concerns. The intent of `canEnableStaticRuntime` is to catch issues in the module before Static Runtime messes with it.

With this diff, `StaticModule` instantiation can be avoided by querying `canEnableStaticRuntime` and the issue is fixed.

Test Plan: New unit test

Reviewed By: tenpercent

Differential Revision: D40564452

fbshipit-source-id: 4415dd3a67bcc1d3594a97a3c94a3149d5c93401

Contributor

facebook-github-bot commented Oct 25, 2022

This pull request was exported from Phabricator. Differential Revision: D40564452

mikeiovine force-pushed the export-D40564452 branch from bbd8e70 to 7c09e53 Compare

October 25, 2022 18:24

Contributor

facebook-github-bot commented Oct 25, 2022

This pull request was exported from Phabricator. Differential Revision: D40564452

mikeiovine force-pushed the export-D40564452 branch from 7c09e53 to d31c2da Compare

October 25, 2022 21:36

mikeiovine pushed a commit to mikeiovine/pytorch that referenced this pull request


          [Static Runtime] Make canEnableStaticRuntime examine sub-blocks (pyto…

d31c2da

…rch#87396)

Summary:
Pull Request resolved: pytorch#87396

Someone was running into problems where

1) Static Runtime enablement would fail
2) We would try to fall back to the JIT interpreter *after trying to create `StaticModule`*
3) The fallback fails because Static Runtime mangled the graph.

We don't want to prevent Static Runtime from mutating its input due to memory concerns. The intent of `canEnableStaticRuntime` is to catch issues in the module before Static Runtime messes with it.

With this diff, `StaticModule` instantiation can be avoided by querying `canEnableStaticRuntime` and the issue is fixed.

Test Plan: New unit test

Reviewed By: tenpercent

Differential Revision: D40564452

fbshipit-source-id: 04bf5c2962ef2e97540373e1fb5edfe621df1a89


          [Static Runtime] Make canEnableStaticRuntime examine sub-blocks (pyto…

12ca662

…rch#87396)

Summary:
Pull Request resolved: pytorch#87396

Someone was running into problems where

1) Static Runtime enablement would fail
2) We would try to fall back to the JIT interpreter *after trying to create `StaticModule`*
3) The fallback fails because Static Runtime mangled the graph.

We don't want to prevent Static Runtime from mutating its input due to memory concerns. The intent of `canEnableStaticRuntime` is to catch issues in the module before Static Runtime messes with it.

With this diff, `StaticModule` instantiation can be avoided by querying `canEnableStaticRuntime` and the issue is fixed.

Test Plan: New unit test

Reviewed By: tenpercent

Differential Revision: D40564452

fbshipit-source-id: 440cb09c0120b8dd8c2b0734c18e741a5963966d

mikeiovine force-pushed the export-D40564452 branch from d31c2da to 12ca662 Compare

October 26, 2022 01:47

Contributor

facebook-github-bot commented Oct 26, 2022

This pull request was exported from Phabricator. Differential Revision: D40564452

Contributor

facebook-github-bot commented Oct 26, 2022

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

Collaborator

pytorchmergebot commented Oct 26, 2022

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot added the Merged label

pytorchmergebot closed this in

ed7a8ab

Contributor

github-actions bot commented Oct 26, 2022

Hey @mikeiovine.
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.

kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request


          [Static Runtime] Make canEnableStaticRuntime examine sub-blocks (pyto…

aa0a533

…rch#87396)

Summary:
Someone was running into problems where

1) Static Runtime enablement would fail
2) We would try to fall back to the JIT interpreter *after trying to create `StaticModule`*
3) The fallback fails because Static Runtime mangled the graph.

We don't want to prevent Static Runtime from mutating its input due to memory concerns. The intent of `canEnableStaticRuntime` is to catch issues in the module before Static Runtime messes with it.

With this diff, `StaticModule` instantiation can be avoided by querying `canEnableStaticRuntime` and the issue is fixed.

Test Plan: New unit test

Differential Revision: D40564452

Pull Request resolved: pytorch#87396
Approved by: https://github.com/tenpercent

kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request


          [Static Runtime] Make canEnableStaticRuntime examine sub-blocks (pyto…

ffd6d88

…rch#87396)

Summary:
Someone was running into problems where

1) Static Runtime enablement would fail
2) We would try to fall back to the JIT interpreter *after trying to create `StaticModule`*
3) The fallback fails because Static Runtime mangled the graph.

We don't want to prevent Static Runtime from mutating its input due to memory concerns. The intent of `canEnableStaticRuntime` is to catch issues in the module before Static Runtime messes with it.

With this diff, `StaticModule` instantiation can be avoided by querying `canEnableStaticRuntime` and the issue is fixed.

Test Plan: New unit test

Differential Revision: D40564452

Pull Request resolved: pytorch#87396
Approved by: https://github.com/tenpercent

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

Labels

ciflow/trunk fb-exported Merged release notes: jit