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

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

Closed
wants to merge 24 commits into from

Conversation

Copy link

pytorch-bot bot commented Apr 24, 2024

🔗 Helpful Links

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

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 (image):

NEW FAILURES - The following jobs have failed:

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

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

…cache collisions"

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

[ghstack-poisoned]
…cache collisions"

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

[ghstack-poisoned]
…cache collisions"

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

[ghstack-poisoned]
…cache collisions"

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

[ghstack-poisoned]
…cache collisions"

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

[ghstack-poisoned]
…cache collisions"

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

[ghstack-poisoned]
…cache collisions"

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

[ghstack-poisoned]
…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 Apr 26, 2024
…sions

ghstack-source-id: 39b1a7a23677e38814e8c2ab579f0678139b5cb7
Pull Request resolved: #124881
…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 Apr 27, 2024
…sions

ghstack-source-id: 1d70485e82db529f0c6cf482fcba2bd97ef30a5f
Pull Request resolved: #124881
…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 Apr 27, 2024
…sions

ghstack-source-id: 5320a1497cb9fce5a7b01755fbb7c00914de42a7
Pull Request resolved: #124881
…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 Apr 27, 2024
…sions

ghstack-source-id: 5320a1497cb9fce5a7b01755fbb7c00914de42a7
Pull Request resolved: #124881
…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 Apr 27, 2024
…sions

ghstack-source-id: 35d41e77b75aa36504bdcc96e286666f312d625c
Pull Request resolved: #124881
…cache collisions"

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

[ghstack-poisoned]
@anijain2305 anijain2305 mentioned this pull request Apr 27, 2024
anijain2305 added a commit that referenced this pull request Apr 27, 2024
…sions

ghstack-source-id: bd9fe3a96748bba33cc066909b414b433087bd80
Pull Request resolved: #124881
…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 Apr 29, 2024
…sions

ghstack-source-id: 45ba66e3f2f573c5c0707175e39767be0c5e59cd
Pull Request resolved: #124881
@anijain2305 anijain2305 added the keep-going Don't stop on first failure, keep running tests until the end label Apr 29, 2024
…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 Apr 29, 2024
…sions

ghstack-source-id: 499a94dd9c0a2c3fbdbab47f19277ac661d11c8e
Pull Request resolved: #124881
…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 Apr 29, 2024
…sions

ghstack-source-id: 829e6811c9d9787763de6b18607d349a800ff863
Pull Request resolved: #124881
…cache collisions"

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

[ghstack-poisoned]
…cache collisions"

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

[ghstack-poisoned]
…cache collisions"

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

[ghstack-poisoned]
…cache collisions"

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

[ghstack-poisoned]
@@ -7,6 +7,7 @@
import copy
import copyreg
import dataclasses
import dis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dynamo kicks in while creating the new function.

…cache collisions"

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

[ghstack-poisoned]
…cache collisions"

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

[ghstack-poisoned]
…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 May 1, 2024
…sions

ghstack-source-id: 78e4904615c0f83239fc3ca6506b5ed146391a8f
Pull Request resolved: #124881
@anijain2305
Copy link
Contributor Author

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 anijain2305 closed this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/inductor keep-going Don't stop on first failure, keep running tests until the end module: dynamo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant