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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JIT] Unannotated NamedTuple Type should use TensorType::getInferred() #46326

Closed
eellison opened this issue Oct 14, 2020 · 2 comments
Closed
Labels
days module: bootcamp We plan to do a full writeup on the issue, and then get someone to do it for onboarding oncall: jit Add this issue/PR to JIT oncall triage queue
Projects

Comments

@eellison
Copy link
Contributor

eellison commented Oct 14, 2020

馃悰 Bug / 馃殌 Feature

import torch
from collections import namedtuple
_MyNamedTuple = namedtuple('_MyNamedTuple', ['value'])

@torch.jit.script
def foo():
    return _MyNamedTuple(1)
> Expected a value of type 'Tensor' for argument 'value' but instead found type 'int'.

It would be nice if we showed that the Tensor type was inferred because no type annotations were given for the named tuple. There was a user internally who was confused by this.

cc @gmagogsfm

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Oct 14, 2020
@github-actions github-actions bot added this to Need triage in JIT Triage Oct 14, 2020
@wconstab wconstab added days module: bootcamp We plan to do a full writeup on the issue, and then get someone to do it for onboarding labels Oct 20, 2020
@wconstab wconstab moved this from Need triage to In discussion in JIT Triage Oct 20, 2020
@Lilyjjo Lilyjjo assigned Lilyjjo and unassigned gmagogsfm Oct 20, 2020
@Lilyjjo
Copy link
Contributor

Lilyjjo commented Oct 21, 2020

Implementation pointers:

The error message is generated in torch/csrc/jit/frontend/schema_matching.cpp鈥檚 tryMatchArgument(...). There are multiple ways of solving this issue, so feel free to take a different approach. One way would be to get is_inferred_type() on the value鈥檚 type in this function to return true. The return value of is_inferred_type() is determined by /aten/src/ATen/core/type.cpp鈥檚 TensorType::create(...) that sets the is_inferred_ value. You would need to track down where in the code NamedTuple calls this TensorType::create(...) and set the is_inferred bool to be true. If this isn鈥檛 feasible, then generating the error message when the type refinement of the unannotated variable for the NamedTuple could also work.

The code that deals with the NamedTuple can be found by searching for 'NamedTuple' in:
torch/csrc/jit/python/python_sugared_value.cpp
torch/csrc/jit/frontend/sugared_value.cpp
torch/csrc/jit/frontend/ir_emitter.cpp

@Lilyjjo
Copy link
Contributor

Lilyjjo commented Oct 21, 2020

For reference, this is a way to give the needed type annotations to resolve the error:

import torch
from typing import NamedTuple
_MyNamedTuple = NamedTuple('_MyNamedTuple', [('value', int)])

@torch.jit.script
def foo():
    return _MyNamedTuple(1)

@Lilyjjo Lilyjjo removed their assignment Oct 21, 2020
tugsbayasgalan pushed a commit to tugsbayasgalan/pytorch that referenced this issue Oct 29, 2020
Summary: If there is no annotation given, we want to show users that the type is inferred

Test Plan: Added a new test case that throws an error with the expected error message

Reviewers: Yanan Cao

Subscribers:

Tasks: pytorch#46326

Tags:
JIT Triage automation moved this from In discussion to Done Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
days module: bootcamp We plan to do a full writeup on the issue, and then get someone to do it for onboarding oncall: jit Add this issue/PR to JIT oncall triage queue
Projects
JIT Triage
  
Done
Development

Successfully merging a pull request may close this issue.

5 participants