[FX] Added how to write transformations section #51278
Conversation
💊 CI failures summary and remediationsAs of commit 53d4228 (more details on the Dr. CI page): ✅ None of the CI failures appear to be your fault 💚
🚧 1 ongoing upstream failure:These were probably caused by upstream breakages that are not fixed yet.
This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
| - `Shape | ||
| Propagation <https://github.com/pytorch/pytorch/blob/master/torch/fx/experimental/shape_prop.py>`__ | ||
| - `Roofline | ||
| Analyzer <https://github.com/pytorch/pytorch/blob/a9f88511b8155ba9620730fb175dee8c54e346d5/torch/fx/experimental/cost_model.py>`__ |
There was a problem hiding this comment.
We should point to this pytorch/tutorials#1328
There was a problem hiding this comment.
waiting for Brian to make a placeholder.
| fusion! <https://github.com/pytorch/pytorch/blob/ec86cec20a8a2312a2295d7bc8be6e88256a2de4/torch/fx/experimental/fuser.py>`__ | ||
|
|
||
| For simple transformations that only consist of substitutions, you can also | ||
| make use of the `subgraph rewriter. <https://github.com/pytorch/pytorch/blob/master/torch/fx/subgraph_rewriter.py>`__ |
There was a problem hiding this comment.
Should point this to the API reference section on replace_pattern when it lands
| the shape information of tensors in your graph. In this case, instead of | ||
| looping over the FX graph and modifying it, you can write an interpreter | ||
| on top of the FX graph! As the FX IR is quite simple, it’s easy to | ||
| reimplement an interpreter that also captures your desired attributes. |
There was a problem hiding this comment.
Interpreter and Transformer are landed (https://github.com/pytorch/pytorch/blob/master/torch/fx/interpreter.py), so we should mention those here
Also I don't think the section header makes sense, you can do more than just analyses with the interpreter pattern. I'd suggest just making this section about "The Interpreter Pattern"
There was a problem hiding this comment.
One somewhat awkward thing about the Interpreter/Transformer patterns are that the actual pattern itself is now hidden behind an abstraction in the shape propagation pass.
I think it's fine, so I'll just mention that you can implement the pattern using the interpreter module.
| that require a large amount of rewrite rules (such as vmap or grad), | ||
| this can often improve readability and maintainability of the rules. | ||
|
|
||
| TODO: Example transformations (need to be included first) |
There was a problem hiding this comment.
Make sure to update before landing
| symbolic trace). This can be used for capturing shape information for | ||
| downstream passes, but it can also be used to capture other information | ||
| about execution. | ||
| TODO: Add roofline analysis pass once it gets merged. |
There was a problem hiding this comment.
Make sure to update before landing
There was a problem hiding this comment.
@Chillee has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Make sure to fix the docs build errors: |
|
BTW I just tried patching this on top of master and it seems to be deleting the debugging section. You'll probably have to rebase manually |
|
Here's a patchfile of a manual rebase https://gist.github.com/jamesr66a/64c9301ccbde8cec7905818c6453689e |
|
@jamesr66a Do you know why the docs builds are failing? Is it because of the warning? |
There was a problem hiding this comment.
@Chillee has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@Chillee yes, allcaps |
There was a problem hiding this comment.
@Chillee has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
I still need to add links to vmap/grad/decomposition, but those haven't been added to the examples folder yet.