Skip to content

Conversation

@jamesr66a
Copy link
Collaborator

@jamesr66a jamesr66a commented Jun 22, 2019

Stack from ghstack:

Differential Revision: D15956726

Previously the following would fail:

@jit.script
def forward(
    languages: Optional[Dict[int, str]]  
):
    if languages is None:
        print(languages)
RuntimeError: 
variable 'languages' previously has type Optional[Dict[int, str]] but is now being assigned to a value of type Dict[int, str]:
@jit.script
def forward(
    languages: Optional[Dict[int, str]]  
):
    if languages is None:
       ~~~~~~~~~~~~~~~~~ <--- HERE
        print(languages)

Because Dict[int, str] issubtypeof Optional[Dict[int,str]] would return false, as we did not fall back to isSubtypeOf defined in `Type. This fixes that

@pytorchbot pytorchbot added the module: internals Related to internal abstractions in c10 and ATen label Jun 22, 2019
@jamesr66a jamesr66a requested review from bwasti, eellison, suo and zdevito and removed request for suo June 22, 2019 06:32
@apaszke
Copy link
Contributor

apaszke commented Jun 22, 2019

Also, looks like we also don't fall back on Type::isSubtypeOf in OptionalType.

[JIT] Fix isSubtypeOf implementations

gh-metadata: pytorch pytorch 22104 gh/jamesr66a/11/head
@jamesr66a jamesr66a changed the title [JIT] Fix DictType isSubtypeOf [JIT] Fix isSubtypeOf implementations Jun 22, 2019
@zou3519 zou3519 deleted the gh/jamesr66a/11/head branch June 22, 2019 23:39
zdevito pushed a commit to zdevito/ATen that referenced this pull request Jun 23, 2019
Summary:
Pull Request resolved: pytorch/pytorch#22104
ghimport-source-id: 9db14020f424cf2e021d63e9c0fe4017ac7cd6c8

Test Plan: Imported from OSS

Differential Revision: D15956726

Pulled By: jamesr66a

fbshipit-source-id: 85448deab70c5e5b7ab1132652836ed575581868
@facebook-github-bot
Copy link
Contributor

@jamesr66a merged this pull request in 887ecf7.

return create(contained_types[0]);
}

bool operator==(const Type& rhs) const override {
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional[Int] == Option[Number]?

facebook-github-bot pushed a commit that referenced this pull request Jun 26, 2019
Summary:
This is (mostly) the re-application of:
#21088

which was reverted due to an issue conflicting with changes in:
#22104
Pull Request resolved: #22237

Differential Revision: D16012838

Pulled By: nairbv

fbshipit-source-id: 35f4a73c97ab68b4e2648aca96b2176f07b5a883
zdevito pushed a commit to zdevito/ATen that referenced this pull request Jun 26, 2019
Summary:
This is (mostly) the re-application of:
pytorch/pytorch#21088

which was reverted due to an issue conflicting with changes in:
pytorch/pytorch#22104
Pull Request resolved: pytorch/pytorch#22237

Differential Revision: D16012838

Pulled By: nairbv

fbshipit-source-id: 35f4a73c97ab68b4e2648aca96b2176f07b5a883
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged module: internals Related to internal abstractions in c10 and ATen

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants