[Doc] Update the dynamo deepdive doc #108147

youkaichao · 2023-08-29T14:20:22Z

With a new tool depyf to decompile bytecode into human readable source code, understanding dynamo becomes much more easier.

cc @svekars @carljparker @voznesenskym @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @chenyang78 @aakhundov

pytorch-bot · 2023-08-29T14:20:25Z

🔗 Helpful Links

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

📄 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.

✅ No Failures

As of commit 02b2036 with merge base 1b3dc05 ():
💚 Looks good so far! There are no failures yet. 💚

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

youkaichao · 2023-08-29T14:28:04Z

@jansel Maybe we can also refactor the structure of the doc. We can first show readers how it works, and then introduce various types of guards, as guards will occur in the decompiled source code. However, this is a moderate change, and I would like to hear your opinions before moving forward.

Anyway, finally, with the tools I created (torch._dynamo.eval_frame._debug_get_cache_entry_list and depyf), it seems dynamo can be much easier to understand!

BTW:

@msaroufim is considering to integrate the depyf package into dynamo debugging output. Can't wait to support that!

docs/source/torch.compiler_deepdive.rst

jansel · 2023-08-29T20:34:35Z

docs/source/torch.compiler_deepdive.rst

+   source code of __compiled_fn_0:
+   def ignore_this_function_name(self, L_a_ : torch.Tensor, L_b_ : torch.Tensor):
+       l_a_ = L_a_
+       l_b_ = L_b_
+       abs_1 = torch.abs(l_a_)
+       add = abs_1 + 1;  abs_1 = None
+       truediv = l_a_ / add;  l_a_ = add = None
+       sum_1 = l_b_.sum();  l_b_ = None
+       lt = sum_1 < 0;  sum_1 = None
+       return (truediv, lt)


This is actually an FX graph (since the compiler above returned the original graph), so if you just call print() on it it might be cleaner than getsource().

This is not FX graph actually. It is the forward function of that graph. print on it gives <function forward at 0x17c7212d0>

Ah, you could print(fn.__self__) then. Or change the code above to not return .forward.

Okay, somewhat complicated, but print(__compiled_fn_0._torchdynamo_orig_callable.__self__) does work.

jansel · 2023-08-29T20:40:21Z

docs/source/torch.compiler_deepdive.rst

+   source code of __resume_at_30_1:
+   def ignore_this_function_name(a, b):
+       x = a / (torch.abs(a) + 1)
+       if b.sum() < 0:
+           b = b * -1
       return x * b

-   def fn(a, b):
+   source code of __resume_at_38_2:
+   def ignore_this_function_name(a, b):
       x = a / (torch.abs(a) + 1)
-       lt = b.sum() < 0
-       return x, lt
+       if b.sum() < 0:
+           b = b * -1
+       return x * b


This source code is wrong. It is just showing the original code, without any bytecode level changes done by dynamo.

If you print out the bytecode you will see:

different args to the function not in this source code

JUMP_ABSOLUTE as the first instructions, without any corresponding line in the source code. This causes these functions to start in the middle rather than at the top.

In reality, it is more like:

def __resume_at_30_1(b, x): JUMP_ABSOLUTE <target> x = a / (torch.abs(a) + 1) if b.sum() < 0: <target> b = b * -1 return x * b

There is actually no way to represent this precisely with Python source code, since python doesn't have a goto instruction.

No worry, this can be easily fixed by removing unreachable bytecode. I will use my depyf to decompile it.

I fixed it in 4678346. Previously, inspect.getsource does not work for __resume_at_xxx functions. Now I use depyf.decompile to decompile its source code.

Meanwhile, I noticed that the function names of __resume_at_xxx are not valid python function names. Maybe this should be fixed in pytorch master? WDYT?

What makes it invalid?

>>> def __resume_at_30_1(b, x): ... pass ... >>>

__resume_at_30_1.__code__.co_name is actually <resume in toy_example>. The variable name of this function __resume_at_30_1 does not match its "codename" stored in co_name.

ah, that is for stack traces

Possible fix: change https://github.com/pytorch/pytorch/blob/main/torch/_dynamo/resume_execution.py#L368 this line to a valid variable name.

A better fix: change it to be the same as the function variable name, i.e. __resume_at_30_1 stuff.

The best fix I think: combining both names, and change both the variable name and co_name to something like __resume_at_30_1_in_toy_example. This way, maybe we can even remove the unique id, and just use __resume_at_{offset}_in_{funcname}.

Co-authored-by: Jason Ansel <jansel@jansel.net>

youkaichao · 2023-08-30T08:59:15Z

@jansel I made a picture today, which I found pretty illustrative for new users. Do you want to include it into the doc?

jansel · 2023-08-30T18:17:32Z

Yes we can include that.

youkaichao · 2023-08-31T00:07:22Z

Okay, the flowchart is added into the documentation. Regarding this PR, @jansel do you have any further suggestions?

Separate from this PR, we have two remaining issues:

Can we unify the variable name and co_name of resume functions to be __resume_at_{offset}_in_{funcname}_{uid}?
Do we want to integrate the depyf package in the logging of dynamo, so that it prints human readable source code rather than hard-to-understand bytecodes?

jansel · 2023-08-31T02:11:35Z

* Can we unify the variable name and `co_name` of resume functions to be `__resume_at_{offset}_in_{funcname}_{uid}`?

The main way these are used is showing up in stacktraces when errors happen. So I think a more human readable name is ideal. Offsets/uids are not human readable, and people won't know what __resume_at_ is. So while this change will make the output on this page better, it will make the common case worse.

* Do we want to integrate the `depyf` package in the logging of dynamo, so that it prints human readable source code rather than hard-to-understand bytecodes?

I don't think we should install it by default, since it is a debugging tool. I also worry about how reliable it will be for more complex bytecodes (there are a lot of them) and across Python versions.

youkaichao · 2023-08-31T02:21:12Z

I also worry about how reliable it will be for more complex bytecodes (there are a lot of them) and across Python versions.

This is also my concern. How can I test it across many complex bytecodes generated from dynamo? I would be happy to add them as my testcases.

One problem, though, is how to automatically test the correctness of decompiled code. My tests at https://github.com/youkaichao/depyf/blob/master/tests/test.py are simple programs that I know the output. For dynamo bytecode, it is more difficult to test.

jansel · 2023-08-31T02:27:58Z

For the resume_at functions, any bytecode is possible since we copy user bytecode into the output. Bytecodes also change from Python version to Python version.

Not sure how much you want to invest in depfy. If you want to turn it into a full-fledged decompiler then running CPython unit tests in a mode where you compile->decompile->compile would a good starting place. I'd expect that to become a pretty big project.

One other thing to mention is we are considering changing TorchDynamo guards to be implemented in C++ for performance reasons. So I don't want that to come as a surprise.

youkaichao · 2023-08-31T02:39:49Z

Not sure how much you want to invest in depfy.

I want to limit it to the understanding of torchdynamo. The main usecase might be understanding the guarded code, which has not so compilcated bytecode.

Regarding with the resume_at functions, I'm thinking of an alternative approach: we have the source code of the original function, and maybe we can take advantage at that, to prune the ast tree to get the code, rather than decompile all the bytecodes.

Guards in C++ is Okay, and we can understand what it is checking by inspecting the closure variables it captures. Any ongoing discussion on this topic?

jansel · 2023-08-31T02:54:06Z

For guards specifically, we actually generate them by creating Python code and calling exec() to generate the bytecode. So it might be cleaner to just keep the source code we used to generate them around.

You can print out the source code by setting TORCHDYNAMO_PRINT_GUARDS=1.

It gets compiled to bytecode here:

pytorch/torch/_dynamo/guards.py

Line 1162 in 11860d9

exec(pycode, global_builder.scope, out)

We could add something like:

guard_fn.source_code = guard_body

So it would be easier to inspect the generated guards. Maybe we could even wire things up so inspect.getsource() works on them.

youkaichao · 2023-08-31T03:01:31Z

guard_fn.source_code = guard_body

This is great for guards.

And for the resume functions, I think starting with the original function's ast might be a good idea. The original function already provides much information. However, how to link the ast with bytecodes requires a close look at how dynamo generates the bytecodes.

Is it possible to get me into the dev slack https://bit.ly/ptslack ? We can have a dedicated discussion there, which is more convenient than "chatting" at github I think :)

jansel · 2023-08-31T03:20:59Z

I sent you a slack invite.

youkaichao · 2023-08-31T04:06:57Z

I sent you a slack invite.

How can I accept the invitation? I didn't receive any email.

youkaichao · 2023-08-31T12:17:20Z

The one failing check is not caused by this PR, I think :)

youkaichao · 2023-08-31T14:18:41Z

Joined the slack channel. Thank you!

jansel · 2023-08-31T15:47:39Z

@pytorchbot merge

pytorchmergebot · 2023-08-31T15:49:19Z

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

svekars

Thank you, just a couple of editorial suggestions!

docs/source/torch.compiler_deepdive.rst

svekars · 2023-08-31T15:52:07Z

docs/source/torch.compiler_deepdive.rst

+The following diagram demonstrates how ``torch.compile`` transforms and optimizes user-written code:

-Note that we pass a simple `my_compiler` function as the backend compiler, therefore the subgraph code `__resume_at_38_2`, `__resume_at_30_1`, and `__compiled_fn_0._torchdynamo_orig_callable` remain python code. However, if we use other backends like the built-in `inductor`, the subgraph code will be compiled CUDA kernels for GPU or C++ code for CPU.
+.. image:: _static/img/dynamo/flowchart.jpg


Is it possible to add a paragraph that describes what is going on on the diagram?

Sure, fixed in 454676e.

docs/source/torch.compiler_deepdive.rst

Co-authored-by: Svetlana Karslioglu <svekars@meta.com>

pytorchmergebot · 2023-08-31T16:04:34Z

Merge failed

Reason: New commits were pushed while merging. Please rerun the merge command.

Details for Dev Infra team

Raised by workflow job

Co-authored-by: Svetlana Karslioglu <svekars@meta.com>

youkaichao · 2023-09-02T17:13:27Z

The API _debug_get_cache_entry_list has changed since #108335 . I will update the documentation accordingly today.

youkaichao · 2023-09-03T12:40:36Z

@svekars Hi, do you have any additional comments?

youkaichao · 2023-09-03T13:06:00Z

@pytorchmergebot merge

pytorchmergebot · 2023-09-03T13:08:08Z

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

youkaichao · 2023-09-03T13:38:48Z

That's strange. I unexpectedly have the access to call pytorchmerge bot to merge :)

@jansel @svekars are there any rules to follow for calling the mergebot?

jansel · 2023-09-09T00:12:21Z

I think it let you because I approved the PR. Seems fine to me.

youkaichao added 2 commits August 29, 2023 22:09

update doc

9577449

update doc

ba3c46b

pytorchbot added the open source label Aug 29, 2023

fix lint

70f63a7

jansel requested changes Aug 29, 2023

View reviewed changes

youkaichao and others added 4 commits August 30, 2023 09:12

Update docs/source/torch.compiler_deepdive.rst

6975bb3

Co-authored-by: Jason Ansel <jansel@jansel.net>

Update docs/source/torch.compiler_deepdive.rst

ab42e20

Co-authored-by: Jason Ansel <jansel@jansel.net>

use decompile for __resume

4678346

use print for __compiled_fn_0

23f87e7

colesbury added triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module module: dynamo labels Aug 30, 2023

youkaichao added 2 commits August 31, 2023 08:00

add a flowchart

80f4c9d

Merge remote-tracking branch 'pytorch/main' into doc_dynamo_deepdive

efd243d

jansel approved these changes Aug 31, 2023

View reviewed changes

jansel added the module: docs Related to our documentation, both in docs/ and docblocks label Aug 31, 2023

jansel added the topic: not user facing topic category label Aug 31, 2023

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 31, 2023

pytorchmergebot added the merging label Aug 31, 2023

svekars reviewed Aug 31, 2023

View reviewed changes

youkaichao and others added 2 commits September 1, 2023 00:03

Update docs/source/torch.compiler_deepdive.rst

d2c3e3e

Co-authored-by: Svetlana Karslioglu <svekars@meta.com>

Update docs/source/torch.compiler_deepdive.rst

5f40e2b

Co-authored-by: Svetlana Karslioglu <svekars@meta.com>

pytorchmergebot removed the merging label Aug 31, 2023

youkaichao and others added 5 commits September 1, 2023 00:04

Update docs/source/torch.compiler_deepdive.rst

2ececfc

Co-authored-by: Svetlana Karslioglu <svekars@meta.com>

Update docs/source/torch.compiler_deepdive.rst

d405170

Co-authored-by: Svetlana Karslioglu <svekars@meta.com>

Update docs/source/torch.compiler_deepdive.rst

2210cc8

Co-authored-by: Svetlana Karslioglu <svekars@meta.com>

Update docs/source/torch.compiler_deepdive.rst

20f169a

Co-authored-by: Svetlana Karslioglu <svekars@meta.com>

add description of the flowchart

454676e

youkaichao added 2 commits September 3, 2023 16:50

Merge branch 'pytorch_main' into doc_dynamo_deepdive

74f448b

update the usage of _debug_get_cache_entry_list

02b2036

pytorchmergebot added the merging label Sep 3, 2023

pytorchmergebot added Merged and removed merging labels Sep 3, 2023

pytorchmergebot closed this in ba9acbe Sep 3, 2023

youkaichao deleted the doc_dynamo_deepdive branch September 4, 2023 00:59

[Doc] Update the dynamo deepdive doc #108147

[Doc] Update the dynamo deepdive doc #108147

Uh oh!

Conversation

youkaichao commented Aug 29, 2023 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ No Failures

Uh oh!

youkaichao commented Aug 29, 2023

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youkaichao Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youkaichao Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youkaichao Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youkaichao commented Aug 30, 2023

Uh oh!

jansel commented Aug 30, 2023

Uh oh!

youkaichao commented Aug 31, 2023

Uh oh!

jansel commented Aug 31, 2023

Uh oh!

youkaichao commented Aug 31, 2023

Uh oh!

jansel commented Aug 31, 2023

Uh oh!

youkaichao commented Aug 31, 2023

Uh oh!

jansel commented Aug 31, 2023

Uh oh!

youkaichao commented Aug 31, 2023

Uh oh!

jansel commented Aug 31, 2023

Uh oh!

youkaichao commented Aug 31, 2023

Uh oh!

youkaichao commented Aug 31, 2023

Uh oh!

youkaichao commented Aug 31, 2023

Uh oh!

jansel commented Aug 31, 2023

Uh oh!

pytorchmergebot commented Aug 31, 2023

Merge started

Uh oh!

svekars left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

youkaichao commented Aug 29, 2023 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Aug 29, 2023 •

edited

Loading

youkaichao Aug 30, 2023 •

edited

Loading

youkaichao Aug 30, 2023 •

edited

Loading

youkaichao Aug 30, 2023 •

edited

Loading