Skip to content

[TorchScript] Fix Intro to TorchScript tutorial #866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

SplitInfinity
Copy link

@SplitInfinity SplitInfinity commented Feb 26, 2020

Summary
There is a section of the Intro to TorchScript tutorial that is supposed
to illustrate that tracing cannot capture control flow but scripting can.
However, the generated IR for both cases is the same because the tutorial
only prints out the IR for the outer module, not the inner module that
contains the control flow. This commit fixes this issue by printing the IR
for both the inner and outer module to clearly show that tracing does not
capture control flow, but scripting does.

Testing
Ran the tutorial file.

Tracing Output:

def forward(self,
    argument_1: Tensor) -> None:
  return None

def forward(self,
    input: Tensor,
    h: Tensor) -> Tuple[Tensor, Tensor]:
  _0 = self.dg
  _1 = (self.linear).forward(input, )
  _2 = (_0).forward(_1, )
  _3 = torch.tanh(torch.add(_1, h, alpha=1))
  return (_3, _3)

Scripting Output:

def forward(self,
    x: Tensor) -> Tensor:
  _0 = bool(torch.gt(torch.sum(x, dtype=None), 0))
  if _0:
    _1 = x
  else:
    _1 = torch.neg(x)
  return _1

def forward(self,
    x: Tensor,
    h: Tensor) -> Tuple[Tensor, Tensor]:
  _0 = (self.dg).forward((self.linear).forward(x, ), )
  new_h = torch.tanh(torch.add(_0, h, alpha=1))
  return (new_h, new_h)

@SplitInfinity
Copy link
Author

SplitInfinity commented Feb 26, 2020

cc: Tutorial authors @jamesr66a @suo

For some reason GitHub won't let me add reviewers

@netlify
Copy link

netlify bot commented Feb 26, 2020

Deploy preview for pytorch-tutorials-preview ready!

Built with commit 7a71c8a

https://deploy-preview-866--pytorch-tutorials-preview.netlify.app

Base automatically changed from master to main February 16, 2021 19:33
Base automatically changed from main to master February 16, 2021 19:37
Copy link

@indigoviolet indigoviolet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, one comment inline. Thanks for fixing this!

@@ -274,6 +274,8 @@ def forward(self, x, h):

my_cell = MyCell(MyDecisionGate())
traced_cell = torch.jit.trace(my_cell, (x, h))

print(traced_cell.dg.code)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add some text here and similarly below, to say which forward is for the decision gate and which is for the cell

Summary:
There is a section of the Intro to TorchScript tutorial that is supposed
to illustrate that tracing cannot capture control flow but scripting can.
However, the generated IR for both cases is the same because the tutorial
only prints out the IR for the outer module, not the inner module that
contains the control flow. This commit fixes this issue by printing the IR
for both the inner and outer module to clearly show that tracing does not
capture control flow, but scripting does.

Testing:
Ran the tutorial file.
@holly1238 holly1238 merged commit 0c83a7b into pytorch:master Apr 9, 2021
rodrigo-techera pushed a commit to Experience-Monks/tutorials that referenced this pull request Nov 29, 2021
Summary:
There is a section of the Intro to TorchScript tutorial that is supposed
to illustrate that tracing cannot capture control flow but scripting can.
However, the generated IR for both cases is the same because the tutorial
only prints out the IR for the outer module, not the inner module that
contains the control flow. This commit fixes this issue by printing the IR
for both the inner and outer module to clearly show that tracing does not
capture control flow, but scripting does.

Testing:
Ran the tutorial file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants