Skip to content
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

Generate special clear error messages for known common misuses in TorchScript #58122

Open
gmagogsfm opened this issue May 12, 2021 · 14 comments
Open

Comments

@gmagogsfm
Copy link
Contributor

@gmagogsfm gmagogsfm commented May 12, 2021

There are some common misuse patterns in TorchScript that we should issue clear error messages for instead of generating generic error that doesn't capture root cause of error.

Here are a few examples:

  • Attempting to construct a nn.Module inside TorchScript. This currently errors out because TorchScript would attempt to compile __init__() method of module, which usually contains a call to super(), which isn't supported. Instead, TS should really recognize that a call to constructor of nn.Module is the real problem.
  • Calling into known torch.* components that are not scriptable. For example, torch.distributions currently is not scriptable. If TS sees a call into torch.distributions methods, it should warn users about it and prompt them to use jit.trace instead.
  • Registering new buffers. This isn't supported because it is effectively modifying type of nn.Module. We should also give a better error message here.

cc @gmagogsfm

@ansley
Copy link

@ansley ansley commented May 14, 2021

If any OSS contributors are interested in taking this on as a way to learn about our codebase, I can be your point of contact. Please email me at ansley@fb.com with any questions

I'll leave this up for 30 days, after which point I'll fix the errors myself
cc @gmagogsfm

@ansley ansley moved this from Need triage to Pending in JIT Triage May 14, 2021
@venkateshtata
Copy link
Contributor

@venkateshtata venkateshtata commented May 15, 2021

I am new to Pytorch and really want to understand the codebase, can I take up this issue and tell me where to start ? @ansley @gmagogsfm

@ansley
Copy link

@ansley ansley commented May 15, 2021

@venkateshtata Yes, absolutely! Go ahead and pick one of the three issues listed above to start with. (I think it will be easier to start with one than to try to get all three done at once.) I can help you figure out where to start and answer any questions you might have

@venkateshtata
Copy link
Contributor

@venkateshtata venkateshtata commented May 15, 2021

Okay @ansley, Thank you.

@Gilf641
Copy link

@Gilf641 Gilf641 commented May 16, 2021

Hey @ansley, I would like to work on one of them. I have a basic knowledge of PyTorch and want to contribute to OSS.

@ansley
Copy link

@ansley ansley commented May 16, 2021

@Gilf641 Same deal, pick one and I'll help answer any questions you might have! Thanks in advance for contributing

@ansley
Copy link

@ansley ansley commented May 16, 2021

I'm really excited that we've had so much interest in these issues (thank you, OSS community!). We have three separate problems, so I think we can have three separate people contribute. The issues will be first come, first serve. If you scroll down in the comments on this post and don't see anyone who's volunteered to take the issue, it's yours! Of course, please make sure to leave your own comment so that we all know who's working on what. Don't hesitate to reach out to me at ansley@fb.com with any questions.

@lezwon
Copy link

@lezwon lezwon commented May 16, 2021

hey @ansley, I'd like to take up the second issue concerning torch.distributions. I'd like to know where I can add this check.

@Gilf641
Copy link

@Gilf641 Gilf641 commented May 16, 2021

Hey @ansley, I'll take up the third issue, related to registering new buffers.

@venkateshtata
Copy link
Contributor

@venkateshtata venkateshtata commented May 17, 2021

@ansley
Right now when we do something like below :

import torch

@torch.jit.script
class MyCell(torch.nn.Module):
    def __init__(self):
        super(MyCell, self).__init__()
        self.linear = torch.nn.Linear(4, 4)

    def forward(self, x, h):
        new_h = torch.tanh(self.linear(x) + h)
        return new_h, new_h

my_cell = MyCell()
x, h = torch.rand(3, 4), torch.rand(3, 4)
traced_cell = torch.jit.script(my_cell, (x, h))
print(traced_cell)
traced_cell(x, h)

We get this Error RuntimeError: Type '<class '__main__.Model'>' cannot be compiled since it inherits from nn.Module, pass an instance instead

Should I change the error message as cannot be compiled as a call to constructor of nn.Module is being made ?

@ansley ansley moved this from Pending to In discussion in JIT Triage May 17, 2021
@ansley
Copy link

@ansley ansley commented May 17, 2021

@gmagogsfm will be posting some repros to make this easier for you to start with

@mzainuddin51
Copy link

@mzainuddin51 mzainuddin51 commented May 19, 2021

@ansley I would like to work on the second issue if it is already not fixed

@lezwon
Copy link

@lezwon lezwon commented May 23, 2021

Hey @mzainuddin51 , I'm actually working on the second issue. Just need some help from @ansley and @gmagogsfm.

@Gilf641
Copy link

@Gilf641 Gilf641 commented May 25, 2021

Hey @ansley, I have gone through the Torchscript part, understood how it's being used. Now I need some directions to solve the third issue. Can you guide me on this?

Thank You!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
JIT Triage
  
In discussion
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants