Move post compile steps into post_compile1/post_compile2 method #141656
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/141656
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (2 Unrelated Failures)As of commit dc597d0 with merge base dbbebee ( 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. |
github-actions
bot
requested review from
SherlockNoMad,
albanD,
antoniojkim,
bdhirsh and
miladm
| static_input_idxs: Sequence[int], | ||
| fx_kwargs: _CompileFxKwargs, | ||
| inputs_to_check: Sequence[int], | ||
| boxed_forward_device_index: Optional[BoxedDeviceIndex], |
There was a problem hiding this comment.
I just bashed arguments in here until all the free variables were lifted as arguments. It probably needs a little reordering / coalescing, will figure this out on the next PR.
| compiled_graph = FxGraphCache.post_compile( | ||
| compiled_graph, example_inputs, cudagraphs, gm | ||
| ) | ||
| compiled_graph.post_compile2(example_inputs, cudagraphs, gm) |
There was a problem hiding this comment.
Why is there post_compile1/post_compile2? It has to do with the FxGraphCache.load path above. I plan to refactor that too, but this is an intermediate step.
| # We only return a string in aot mode, in which case we don't | ||
| # need to do any post-compilation steps: we just return the string, | ||
| # which is the filename of the compiled code. | ||
| return compiled_graph |
There was a problem hiding this comment.
You can see above that post compile does nothing in AOT mode. I want to eventually have AOTInductor do its own custom post compile logic at this point. However, I think there really should only be one post compile method.
|
@pytorchbot merge |
Merge startedYour 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 |
Merge failedReason: 1 jobs have failed, first few of them are: trunk / linux-focal-rocm6.2-py3.10 / test (default, 2, 2, linux.rocm.gpu) Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge -f "unrelated failures only" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
…rch#141656) The intention for turning these into methods is so that the AOTInductor compile path can implement them differently. I haven't worked out the implications yet though, but this seemed like a good stopping point for now. Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#141656 Approved by: https://github.com/aorenste, https://github.com/jamesjwu, https://github.com/jansel
Signed-off-by: Edward Z. Yang <ezyang@meta.com> ghstack-source-id: 8e34983 Pull Request resolved: pytorch/pytorch#141656
5 participants
Stack from ghstack (oldest at bottom):
The intention for turning these into methods is so that the AOTInductor compile path can implement them differently. I haven't worked out the implications yet though, but this seemed like a good stopping point for now.
Signed-off-by: Edward Z. Yang ezyang@meta.com
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang @aakhundov