[jit] Make NoneType <: Optional[T]#25361
Closed
zdevito wants to merge 8 commits intogh/zdevito/98/basefrom
Closed
Conversation
Previously we had a different None object for each type T so that unwrap optional could still recover the type T from it. After a few months of having this conversion behavior, it has become clear that only the unwrap optional operators cause this problem. Furthermore, it is beneficial to have NoneType <: Optional[T] because this is how IValues work (in particular the None IValue is not tagged). This patch makes the necessary changes to do this. In particular it special cases unwrap optional in export so that it annotates the None to make sure we can recover the type. This also changes how matching and evaluating type values work so that we can consider None matchable to type Optional[T], eventhough we cannot derive T from that match.
[jit] Make NoneType <: Optional[T] Previously we had a different None object for each type T so that unwrap optional could still recover the type T from it. After a few months of having this conversion behavior, it has become clear that only the unwrap optional operators cause this problem. Furthermore, it is beneficial to have NoneType <: Optional[T] because this is how IValues work (in particular the None IValue is not tagged). This patch makes the necessary changes to do this. In particular it special cases unwrap optional in export so that it annotates the None to make sure we can recover the type. This also changes how matching and evaluating type values work so that we can consider None matchable to type Optional[T], eventhough we cannot derive T from that match. gh-metadata: pytorch pytorch 25361 gh/zdevito/98/head
[jit] Make NoneType <: Optional[T] Previously we had a different None object for each type T so that unwrap optional could still recover the type T from it. After a few months of having this conversion behavior, it has become clear that only the unwrap optional operators cause this problem. Furthermore, it is beneficial to have NoneType <: Optional[T] because this is how IValues work (in particular the None IValue is not tagged). This patch makes the necessary changes to do this. In particular it special cases unwrap optional in export so that it annotates the None to make sure we can recover the type. This also changes how matching and evaluating type values work so that we can consider None matchable to type Optional[T], eventhough we cannot derive T from that match. gh-metadata: pytorch pytorch 25361 gh/zdevito/98/head
[jit] Make NoneType <: Optional[T] Previously we had a different None object for each type T so that unwrap optional could still recover the type T from it. After a few months of having this conversion behavior, it has become clear that only the unwrap optional operators cause this problem. Furthermore, it is beneficial to have NoneType <: Optional[T] because this is how IValues work (in particular the None IValue is not tagged). This patch makes the necessary changes to do this. In particular it special cases unwrap optional in export so that it annotates the None to make sure we can recover the type. This also changes how matching and evaluating type values work so that we can consider None matchable to type Optional[T], eventhough we cannot derive T from that match. gh-metadata: pytorch pytorch 25361 gh/zdevito/98/head
Closed
This was referenced Aug 30, 2019
[jit] Make NoneType <: Optional[T] Previously we had a different None object for each type T so that unwrap optional could still recover the type T from it. After a few months of having this conversion behavior, it has become clear that only the unwrap optional operators cause this problem. Furthermore, it is beneficial to have NoneType <: Optional[T] because this is how IValues work (in particular the None IValue is not tagged). This patch makes the necessary changes to do this. In particular it special cases unwrap optional in export so that it annotates the None to make sure we can recover the type. This also changes how matching and evaluating type values work so that we can consider None matchable to type Optional[T], eventhough we cannot derive T from that match. gh-metadata: pytorch pytorch 25361 gh/zdevito/98/head
This was referenced Aug 30, 2019
suo
approved these changes
Aug 30, 2019
aten/src/ATen/core/jit_type.h
Outdated
| return MatchTypeReturn(); | ||
| } | ||
| bool success() const { | ||
| return reason_ == ""; |
Member
There was a problem hiding this comment.
Make reason_ an optional to avoid false success if you report a failure with an empty string for a reason?
torch/csrc/jit/autodiff.cpp
Outdated
| node->replaceInputWith(input, lifted_constant->output()); | ||
| } | ||
| } | ||
| if (inBlock(input->node(), if_not_in_this_block)) |
Member
There was a problem hiding this comment.
Can you add a comment explaining what's going on here? It's hard for me to understand what this code does
| stmt << ss.str(); | ||
| } | ||
|
|
||
| void printNone(TaggedStringStream& stmt, const Node* node) { |
[jit] Make NoneType <: Optional[T] Previously we had a different None object for each type T so that unwrap optional could still recover the type T from it. After a few months of having this conversion behavior, it has become clear that only the unwrap optional operators cause this problem. Furthermore, it is beneficial to have NoneType <: Optional[T] because this is how IValues work (in particular the None IValue is not tagged). This patch makes the necessary changes to do this. In particular it special cases unwrap optional in export so that it annotates the None to make sure we can recover the type. This also changes how matching and evaluating type values work so that we can consider None matchable to type Optional[T], eventhough we cannot derive T from that match. gh-metadata: pytorch pytorch 25361 gh/zdevito/98/head
[jit] Make NoneType <: Optional[T] Previously we had a different None object for each type T so that unwrap optional could still recover the type T from it. After a few months of having this conversion behavior, it has become clear that only the unwrap optional operators cause this problem. Furthermore, it is beneficial to have NoneType <: Optional[T] because this is how IValues work (in particular the None IValue is not tagged). This patch makes the necessary changes to do this. In particular it special cases unwrap optional in export so that it annotates the None to make sure we can recover the type. This also changes how matching and evaluating type values work so that we can consider None matchable to type Optional[T], eventhough we cannot derive T from that match. gh-metadata: pytorch pytorch 25361 gh/zdevito/98/head temp
[jit] Make NoneType <: Optional[T] Previously we had a different None object for each type T so that unwrap optional could still recover the type T from it. After a few months of having this conversion behavior, it has become clear that only the unwrap optional operators cause this problem. Furthermore, it is beneficial to have NoneType <: Optional[T] because this is how IValues work (in particular the None IValue is not tagged). This patch makes the necessary changes to do this. In particular it special cases unwrap optional in export so that it annotates the None to make sure we can recover the type. This also changes how matching and evaluating type values work so that we can consider None matchable to type Optional[T], eventhough we cannot derive T from that match. gh-metadata: pytorch pytorch 25361 gh/zdevito/98/head temp
zdevito
added a commit
to zdevito/ATen
that referenced
this pull request
Sep 4, 2019
Summary: Pull Request resolved: pytorch/pytorch#25361 Previously we had a different None object for each type T so that unwrap optional could still recover the type T from it. After a few months of having this conversion behavior, it has become clear that only the unwrap optional operators cause this problem. Furthermore, it is beneficial to have NoneType <: Optional[T] because this is how IValues work (in particular the None IValue is not tagged). This patch makes the necessary changes to do this. In particular it special cases unwrap optional in export so that it annotates the None to make sure we can recover the type. This also changes how matching and evaluating type values work so that we can consider None matchable to type Optional[T], eventhough we cannot derive T from that match. Test Plan: Imported from OSS Differential Revision: D17103072 Pulled By: zdevito fbshipit-source-id: 37678ed3e5ce54f2eb3ee4dff2734a39f0bee028
Contributor
This was referenced Sep 11, 2019
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.
Stack from ghstack:
Previously we had a different None object for each type T so that
unwrap optional could still recover the type T from it. After a few
months of having this conversion behavior, it has become clear that
only the unwrap optional operators cause this problem. Furthermore, it
is beneficial to have NoneType <: Optional[T] because this is how IValues
work (in particular the None IValue is not tagged). This patch makes the
necessary changes to do this. In particular it special cases unwrap optional
in export so that it annotates the None to make sure we can recover the type.
This also changes how matching and evaluating type values work so that we
can consider None matchable to type Optional[T], eventhough we cannot
derive T from that match.
Differential Revision: D17103072