Dump torch::jit::AliasDb objects as Graphviz files#50452
Dump torch::jit::AliasDb objects as Graphviz files#50452tlemo wants to merge 3 commits intopytorch:masterfrom
Conversation
💊 CI failures summary and remediationsAs of commit 247381d (more details on the Dr. CI page):
Extra GitHub checks: 1 failed
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. |
Codecov Report
@@ Coverage Diff @@
## master #50452 +/- ##
==========================================
+ Coverage 80.66% 80.70% +0.04%
==========================================
Files 1913 1909 -4
Lines 208091 207097 -994
==========================================
- Hits 167849 167134 -715
+ Misses 40242 39963 -279 |
|
The clang-tidy & clang-format failures seem infrastructure related |
| bool AliasDb::dumpToDot(const char* filename) const { | ||
| std::ofstream dot(filename); | ||
| if (!dot.good()) { | ||
| std::cout << "Failed to open the IR graph file: '" << filename << "'\n"; |
There was a problem hiding this comment.
I think we typically use const std::string& and use exceptions for signaling errors rather than return values.
I think an even better (and more consistent with the rest of the JIT API) way would be to just define the op << .
Then the string dump (great idea) you mentioned on slack could use that in the string stream and opening the file yourself when dumping doesn't seem too much of a hassle (and then people can also dump to stdout or over the network or whatever if they want to).
There was a problem hiding this comment.
In general I agree. One can also point out that printing an message to console and returning an error is bad form.
In this case, throwing an exception doesn't play nicely in a debugger, which is one of the intended use-cases. Normally in the debugger a simple message would do, but the return bool is a compromise to allow some basic composition (so the caller can still tell if it faild)
Same thing with the operator<< overloading - doesn't work well in debuggers.
There was a problem hiding this comment.
Yeah, good points. Happily I don't get to decide on it, anyways. But the string would be possible?
There was a problem hiding this comment.
But the string would be possible?
Using std::string for the filename parameter? Same story: from the C++ side, it seems a minor change, but for a debugger, evaluating foo("bar") when foo takes a const std::string& is far from trivial and not all the debugger support it (my workflow involves using gdb, lldb, windbg/cdb & visual studio regularly and I've learned the hard way to keep debugging helpers as simple as possible).
eellison
left a comment
There was a problem hiding this comment.
Sweet, LGTM!! Thanks for doing this.
facebook-github-bot
left a comment
There was a problem hiding this comment.
@eellison has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: This PR adds a simple debugging helper which exports the AliasDb state as a [GraphViz](http://www.graphviz.org/) graph definition. The generated files can be viewed with any Graphviz viewer (including online based, for example http://viz-js.com) Usage: 1. Call `AliasDb::dumpToGraphvizFile()` from a debugger. Using gdb for example: `call aliasDb_->dumpToGraphvizFile("alias.dot")` 2. Add explicit calls to `AliasDb::dumpToGraphvizFile()`, which returns `true` if it succeeds. An example output file is attached: [example.zip](https://github.com/pytorch/pytorch/files/5805840/example.zip) Pull Request resolved: pytorch#50452 Reviewed By: ngimel Differential Revision: D25980222 Pulled By: eellison fbshipit-source-id: 47805a0a81ce73c6ba859340d37b9a806f9000d5
This PR adds a simple debugging helper which exports the AliasDb state as a GraphViz graph definition. The generated files can be viewed with any Graphviz viewer (including online based, for example http://viz-js.com)
Usage:
Call
AliasDb::dumpToGraphvizFile()from a debugger. Using gdb for example:call aliasDb_->dumpToGraphvizFile("alias.dot")Add explicit calls to
AliasDb::dumpToGraphvizFile(), which returnstrueif it succeeds.An example output file is attached: example.zip