[dynamo][eval_frame] Create a dynamic wrapper fn to avoid cache collisions #124881

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.

Sign up for GitHub

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

Jump to bottom

Closed

anijain2305 wants to merge 24 commits into gh/anijain2305/298/base from gh/anijain2305/298/head

Contributor

anijain2305 commented Apr 24, 2024 •

edited

Loading

Stack from ghstack (oldest at bottom):

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang


          [dynamo][eval_frame] Create a dynamic wrapper fn to avoid cache colli…

45db5ee

…sions

[ghstack-poisoned]

anijain2305 mentioned this pull request

[dynamo] Allow inlining of hooks for the top module #124501

Closed

pytorch-bot bot commented Apr 24, 2024 •

edited

Loading

🔗 Helpful Links

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

📄 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 the bot commands wiki or our office hours

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

❌ 9 New Failures, 1 Unrelated Failure

As of commit e1bf938 with merge base 90258e8 ():

NEW FAILURES - The following jobs have failed:

inductor / rocm6.0-py3.8-inductor / test (inductor, 1, 1, linux.rocm.gpu.2) (gh)
##[error]The operation was canceled.
pull / linux-focal-cuda12.1-py3.10-gcc9 / test (default, 2, 5, linux.4xlarge.nvidia.gpu) (gh)
dynamo/test_hooks.py::HooksTests::test_register_hook_partial_guarding
pull / linux-focal-cuda12.1-py3.10-gcc9-sm86 / test (default, 5, 5, linux.g5.4xlarge.nvidia.gpu) (gh)
dynamo/test_hooks.py::HooksTests::test_register_hook_partial_guarding
pull / linux-focal-py3_8-clang9-xla / test (xla, 1, 1, linux.12xlarge) (gh)
test_resnet18
pull / linux-focal-py3.11-clang10 / test (default, 2, 3, linux.2xlarge) (gh)
dynamo/test_hooks.py::HooksTests::test_register_hook_partial_guarding
pull / linux-focal-py3.12-clang10 / test (default, 2, 3, linux.2xlarge) (gh)
dynamo/test_hooks.py::HooksTests::test_register_hook_partial_guarding
pull / linux-focal-py3.8-clang10 / test (default, 2, 3, linux.2xlarge) (gh)
dynamo/test_hooks.py::HooksTests::test_register_hook_partial_guarding
pull / linux-jammy-py3.10-clang15-asan / test (default, 4, 6, linux.4xlarge) (gh)
dynamo/test_hooks.py::HooksTests::test_register_hook_partial_guarding
pull / linux-jammy-py3.8-gcc11 / test (default, 1, 3, linux.2xlarge) (gh)
dynamo/test_hooks.py::HooksTests::test_register_hook_partial_guarding

FLAKY - The following job failed but was likely due to flakiness present on trunk:

inductor / cuda12.1-py3.10-gcc9-sm86 / test (dynamic_inductor_timm, 2, 2, linux.g5.4xlarge.nvidia.gpu) (gh) (similar failure)
sebotnet33ts_256

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

pytorch-bot bot added ciflow/inductor module: dynamo labels

anijain2305 added 2 commits

April 24, 2024 14:34


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

0dc1561

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

0e21327

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]

anijain2305 commented

View reviewed changes

torch/_dynamo/external_utils.py Outdated Show resolved Hide resolved

anijain2305 added 2 commits

April 24, 2024 17:02


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

9540fae

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

25716dc

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]

anijain2305 mentioned this pull request

[dynamo] Bug fix for LOAD_GLOBAL and STORE_GLOBAL #125002

Closed

anijain2305 added 4 commits

April 26, 2024 10:06


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

0528ec6

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

c3d34c5

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

f5c7cd9

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

9e22bb2

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

96b3b01

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

b8539ca

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]

anijain2305 mentioned this pull request

[dynamo] Collect cell_and_freevars correctly #125097

Closed


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

b37ae41

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

abad34e

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

2aafe02

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]

anijain2305 mentioned this pull request

[dynamo] Fix test #125107

Closed

anijain2305 added a commit that referenced this pull request


          [dynamo][eval_frame] Create a dynamic wrapper fn to avoid cache colli…

46fd450

…sions

ghstack-source-id: bd9fe3a
Pull Request resolved: #124881


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

4530ba2

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]

anijain2305 added the keep-going label


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

1e46e45

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]

anijain2305 mentioned this pull request

[dynamo] use lazy disable dynamo for manual seed #125196

Closed


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

03294b7

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]

anijain2305 added a commit that referenced this pull request


          [dynamo][eval_frame] Create a dynamic wrapper fn to avoid cache colli…

64060d2

…sions

ghstack-source-id: 829e681
Pull Request resolved: #124881

anijain2305 added 4 commits

April 30, 2024 11:53


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

d1c6d16

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

6df8ad7

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

33282a3

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

11b1e9e

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]

anijain2305 commented

View reviewed changes

torch/_dynamo/trace_rules.py

    
              import copy

              import copyreg

              import dataclasses

              import dis

Contributor Author

anijain2305 Apr 30, 2024

Dynamo kicks in while creating the new function.

anijain2305 added 3 commits

April 30, 2024 17:00


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

8869e81

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

765f473

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]


          Update on "[dynamo][eval_frame] Create a dynamic wrapper fn to avoid …

e1bf938

…cache collisions"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 kadeng chauhang

[ghstack-poisoned]

anijain2305 added a commit that referenced this pull request


          [dynamo][eval_frame] Create a dynamic wrapper fn to avoid cache colli…

9304b74

…sions

ghstack-source-id: 78e4904
Pull Request resolved: #124881

Contributor Author

anijain2305 commented May 1, 2024

Closing this PR after iterating it over for a week.

Why this PR?
When we wrap_inline, we want to create a new code object to avoid Dynamo collisions. This makes sense.

Whats the problem?
If we have two different instances of nn.Module. And we put torch.compile on both of them. Today, we will see one compilation (with inlined inbuilt nn modules) because the cache sits on forward.__code__ (or on the inner.code) object. But with this change, there will be a full recompilation. Now, this can be solved by using cache of wrappers. But then it complicates the impl because we need to develop a key for this new cache. Simply caching on code object won't work because __closure__ can change between different functions. We can make more and more comprehensive cache, but given that we call this wrapper infrequently, its fine to just leave it as is.

anijain2305 closed this

github-actions bot deleted the gh/anijain2305/298/head branch

June 4, 2024 02:00

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

Labels

ciflow/inductor keep-going module: dynamo