Add demo_graph, hack to make IR scopes work again (fixing JIT passes)#10749
Closed
ml7 wants to merge 2 commits intopytorch:masterfrom
Closed
Add demo_graph, hack to make IR scopes work again (fixing JIT passes)#10749ml7 wants to merge 2 commits intopytorch:masterfrom
ml7 wants to merge 2 commits intopytorch:masterfrom
Conversation
…pytorch#10749) Summary: Pull Request resolved: pytorch#10749 Added a demo_graph so that users can try out writing model graph visualization for TensorboardX with various models (VGG, ResNet, DenseNet, and some other toy examples). Working implementation that generates the graph correctly without extraneous information. It's pretty hacky and I'm trying to clean it up, any comments on the code are appreciated! The problem was that the JIT passes weren't preserving scopes when they were creating new nodes so nodes were ending up without scope names and this caused us to lose valuable information in visualization. Sample graph visualization (ResNet model) generated below: {F136387033} Differential Revision: D9389809 fbshipit-source-id: 5e21ca3fe7fea292ae126982180999bcc2041b45
apaszke
suggested changes
Aug 28, 2018
Contributor
apaszke
left a comment
There was a problem hiding this comment.
insertConstantWithScope seems unnecessary and mostly duplicates the code of insertConstant. We should use Scope* more instead of passing around strings with scope names.
| // Clone the node and add it to the new graph | ||
| auto cloneNode = [&](Node * node) { | ||
| auto n_ = ctx.block->appendNode(ctx.block->owningGraph()->createClone(node, envFn)); | ||
| n_->setScopeFromString(node->scopeName()); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| auto s = *constant_as<at::Scalar>(it->output()); | ||
| WithInsertPoint guard(*it); | ||
| Value* r = block->owningGraph()->insertConstant(s.toTensor()); | ||
| Value* r = block->owningGraph()->insertConstantWithScope(s.toTensor(), it->scopeName()); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| std::string name2 = scope_root_.get()->name().toDisplayString(); | ||
| if (name1.find(name2) < 0 && | ||
| name2.find(name1) < 0 && | ||
| scope->getRoot() != scope_root_.get()) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Remove insertConstantWithScope
Member
|
ping @apaszke |
Contributor
Contributor
|
Also cc @lanpa on this one. |
orionr
added a commit
to orionr/pytorch
that referenced
this pull request
Oct 5, 2018
Summary: In order to support tensorboardX and other visualization tools, we need to make sure a non-empty scope is set on all nodes added by the JIT. This attempts to do this, but is still a WIP. This is a new version of pytorch#10749 Differential Revision: D10224380 fbshipit-source-id: 3368f6d86e9f61dd699a868c41a8cb73a899ea26
Contributor
|
Closing in favor of the new PR. |
orionr
added a commit
to orionr/pytorch
that referenced
this pull request
Oct 22, 2018
Summary: In order to support tensorboardX and other visualization tools, we need to make sure a non-empty scope is set on all nodes added by the JIT. This attempts to do this, but is still a WIP. This is a new version of pytorch#10749 Pull Request resolved: pytorch#12400 Differential Revision: D10224380 fbshipit-source-id: ae99180f98b71510e2a6261c5afe66fb79b1c857
facebook-github-bot
pushed a commit
that referenced
this pull request
Oct 24, 2018
Summary: In order to support tensorboardX and other visualization tools, we need to make sure a non-empty scope is set on all nodes added by the JIT. This attempts to do this, but is still a WIP. This is a new version of #10749 Pull Request resolved: #12400 Reviewed By: ezyang Differential Revision: D10224380 Pulled By: orionr fbshipit-source-id: d1bccd0eee9ef7c4354112c6a39a5987bfac2994
laurentdupin
pushed a commit
to laurentdupin/pytorch
that referenced
this pull request
Apr 24, 2026
Summary: In order to support tensorboardX and other visualization tools, we need to make sure a non-empty scope is set on all nodes added by the JIT. This attempts to do this, but is still a WIP. This is a new version of pytorch#10749 Pull Request resolved: pytorch#12400 Reviewed By: ezyang Differential Revision: D10224380 Pulled By: orionr fbshipit-source-id: d1bccd0eee9ef7c4354112c6a39a5987bfac2994
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Added a demo_graph so that users can try out writing model graph visualization for TensorboardX with various models (VGG, ResNet, DenseNet, and some other toy examples).
Working implementation that generates the graph correctly without extraneous information. It's pretty hacky and I'm trying to clean it up, any comments on the code are appreciated!
The problem was that the JIT passes weren't preserving scopes when they were creating new nodes so nodes were ending up without scope names and this caused us to lose valuable information in visualization.
Sample graph visualization (ResNet model) generated below:
{F136387033}
Differential Revision: D9389809