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.

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

[inductor] Clear cache on ctx manager exit #126146

Closed
wants to merge 6 commits into from
Closed

[inductor] Clear cache on ctx manager exit #126146

wants to merge 6 commits into from

Conversation

xmfan
Copy link
Member

@xmfan xmfan commented May 14, 2024 •
edited

Stack from ghstack (oldest at bottom):

FIXES #126128.

Right now, we only clear the cache on ctx manager enter. So state is bad unless we call fresh_inductor_cache again, usually fine in tests.

Cue compiled autograd tests when going from TestCompiledAutograd -> TestAutogradWithCompiledAutograd.
TestCompiledAutograd uses the ctx manager, but TestAutogradWithCompiledAutograd don't

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang

@xmfan xmfan mentioned this pull request May 14, 2024
Closed
@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented May 14, 2024 •
edited

🔗 Helpful Links

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

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

✅ You can merge normally! (6 Unrelated Failures)

As of commit 9b9aab5 with merge base 91bf952 (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

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

xmfan added a commit that referenced this pull request May 14, 2024
@xmfan
[inductor] Clear cache on ctx manager exit
17abc38 
ghstack-source-id: 27370c47571ff4fd95b800cb9ac95aa913c692e5
Pull Request resolved: #126146
@xmfan
Update on "[inductor] Clear cache on ctx manager exit"
864dcdb 
FIXES #126128

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang

[ghstack-poisoned]
xmfan added a commit that referenced this pull request May 14, 2024
@xmfan
[inductor] Clear cache on ctx manager exit
3bff531 
ghstack-source-id: 41e603942a31156d54824352d73fbc5ef2c460ff
Pull Request resolved: #126146
@xmfan xmfan mentioned this pull request May 14, 2024
Closed
@xmfan xmfan added the topic: not user facing topic category label May 14, 2024
@xmfan
Update on "[inductor] Clear cache on ctx manager exit"
8c807d4 
FIXES #126128.

Right now, we only clear the cache on ctx manager enter. So state is bad unless we call fresh_inductor_cache again,  usually fine in tests.

Cue compiled autograd tests when going from TestCompiledAutograd -> TestAutogradWithCompiledAutograd. 
TestCompiledAutograd uses the ctx manager, but TestAutogradWithCompiledAutograd don't

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang

[ghstack-poisoned]
@xmfan xmfan marked this pull request as ready for review May 14, 2024 15:31
xmfan added 2 commits May 15, 2024 17:42
@xmfan
Update on "[inductor] Clear cache on ctx manager exit"
692290a 
FIXES #126128.

Right now, we only clear the cache on ctx manager enter. So state is bad unless we call fresh_inductor_cache again,  usually fine in tests.

Cue compiled autograd tests when going from TestCompiledAutograd -> TestAutogradWithCompiledAutograd. 
TestCompiledAutograd uses the ctx manager, but TestAutogradWithCompiledAutograd don't

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang

[ghstack-poisoned]
@xmfan
Update on "[inductor] Clear cache on ctx manager exit"
0ce2524 
FIXES #126128.

Right now, we only clear the cache on ctx manager enter. So state is bad unless we call fresh_inductor_cache again,  usually fine in tests.

Cue compiled autograd tests when going from TestCompiledAutograd -> TestAutogradWithCompiledAutograd. 
TestCompiledAutograd uses the ctx manager, but TestAutogradWithCompiledAutograd don't

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang

[ghstack-poisoned]
xmfan added a commit that referenced this pull request May 16, 2024
@xmfan
[inductor] Clear cache on ctx manager exit
643f57a 
ghstack-source-id: 9146aea2680868b25af31a9271a7aa0a396668af
Pull Request resolved: #126146
@xmfan
Update
9b9aab5 
[ghstack-poisoned]
@xmfan xmfan mentioned this pull request May 16, 2024
Closed
bilal2vec pushed a commit to bilal2vec/pytorch that referenced this pull request May 16, 2024
@xmfan @pytorchmergebot
[compiled autograd] clear compiled_autograd_verbose once test is done (…
93524cf 
…pytorch#126148)

verbose flag leaks into tests ran after

Pull Request resolved: pytorch#126148
Approved by: https://github.com/jansel
ghstack dependencies: pytorch#126144, pytorch#126146
This was referenced May 17, 2024
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced May 17, 2024
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
ZelboK pushed a commit to ZelboK/pytorch that referenced this pull request May 19, 2024
@xmfan @ZelboK
[inductor] Clear cache on ctx manager exit (pytorch#126146)
b2efbae 
FIXES pytorch#126128.

Right now, we only clear the cache on ctx manager enter. So state is bad unless we call fresh_inductor_cache again,  usually fine in tests.

Cue compiled autograd tests when going from TestCompiledAutograd -> TestAutogradWithCompiledAutograd.
TestCompiledAutograd uses the ctx manager, but TestAutogradWithCompiledAutograd don't

Pull Request resolved: pytorch#126146
Approved by: https://github.com/jgong5, https://github.com/oulgen
ghstack dependencies: pytorch#126144
ZelboK pushed a commit to ZelboK/pytorch that referenced this pull request May 19, 2024
@xmfan @ZelboK
[compiled autograd] clear compiled_autograd_verbose once test is done (…
b29fd1f 
…pytorch#126148)

verbose flag leaks into tests ran after

Pull Request resolved: pytorch#126148
Approved by: https://github.com/jansel
ghstack dependencies: pytorch#126144, pytorch#126146
pytorchmergebot pushed a commit that referenced this pull request May 19, 2024
@xmfan @pytorchmergebot
[compiled autograd] log in cpp using python logger (#126483)
be67985 
Internal infra may not preserve python and c++ log ordering e.g. MAST logs: https://fburl.com/mlhub/38576cxn, all the `[python_compiled_autograd.cpp] Creating cache entry [...]` logs of the entire run are at the beginning of the file

Pull Request resolved: #126483
Approved by: https://github.com/jansel
ghstack dependencies: #126144, #126146, #126148
pytorchmergebot pushed a commit that referenced this pull request May 19, 2024
@xmfan @pytorchmergebot
[compiled autograd] Better cache miss logging (#126602)
5fb11cd 
- log only first node key cache miss
- log existing node key sizes
- log which node's collected sizes became dynamic
e.g.
```
DEBUG:torch._dynamo.compiled_autograd.__compiled_autograd_verbose:Cache miss due to new autograd node: torch::autograd::GraphRoot (NodeCall 0) with key size 39, previous key sizes=[]
...
DEBUG:torch._dynamo.compiled_autograd.__compiled_autograd_verbose:Cache miss due to new autograd node: torch::autograd::AccumulateGrad (NodeCall 5) with key size 32, previous key sizes=[21]
...
DEBUG:torch._dynamo.compiled_autograd.__compiled_autograd_verbose:Cache miss due to dynamic shapes: collected size idx 0 of torch::autograd::GraphRoot (NodeCall 0)
DEBUG:torch._dynamo.compiled_autograd.__compiled_autograd_verbose:Cache miss due to dynamic shapes: collected size idx 2 of SumBackward0 (NodeCall 1)
DEBUG:torch._dynamo.compiled_autograd.__compiled_autograd_verbose:Cache miss due to dynamic shapes: collected size idx 4 of SumBackward0 (NodeCall 1)
DEBUG:torch._dynamo.compiled_autograd.__compiled_autograd_verbose:Cache miss due to dynamic shapes: collected size idx 2 of ReluBackward0 (NodeCall 2)
DEBUG:torch._dynamo.compiled_autograd.__compiled_autograd_verbose:Cache miss due to dynamic shapes: collected size idx 9 of AddmmBackward0 (NodeCall 3)
DEBUG:torch._dynamo.compiled_autograd.__compiled_autograd_verbose:Cache miss due to dynamic shapes: collected size idx 2 of torch::autograd::AccumulateGrad (NodeCall 5)
DEBUG:torch._dynamo.compiled_autograd.__compiled_autograd_verbose:Cache miss due to dynamic shapes: collected size idx 2 of ReluBackward0 (NodeCall 6)
```

Pull Request resolved: #126602
Approved by: https://github.com/jansel
ghstack dependencies: #126144, #126146, #126148, #126483
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oulgen oulgen oulgen approved these changes

@jgong5 jgong5 jgong5 approved these changes

Assignees
No one assigned
Labels
ciflow/inductor Merged module: dynamo module: inductor topic: not user facing topic category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@xmfan @oulgen @jgong5 @pytorchmergebot