[JIT] Don't throw when type is used in TorchScript #28053
Closed
Conversation
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
eellison
force-pushed
the
check_abstract
branch
from
3e49c15 to
3a2500b
Compare
driazati
reviewed
Oct 15, 2019
| def fn(x): | ||
| return type(x) | ||
|
|
||
| with self.assertRaisesRegex(RuntimeError, "value of type type"): |
Contributor
There was a problem hiding this comment.
Can you put quotes around the output here so it doesn't look like a typo?
e.g.
value of type 'type'
torch/_jit_internal.py
Outdated
| # with an abstract method, in either case we cannot compile it as a class | ||
| if '__abstractmethods__' in names: | ||
| return False | ||
| fns = [getattr(cls, name) for name in names if inspect.isroutine(getattr(cls, name))] |
Contributor
There was a problem hiding this comment.
You can avoid special casing type here by changing the getattr to getattr(cls, name, None)
Contributor
There was a problem hiding this comment.
Or better yet change this whole thing to this which doesn't have the problem of accessing the attribute at all:
members = inspect.getmembers(cls)
members = map(lambda x: x[1], members)
fns = filter(inspect.isroutine, members)
Member
|
Just checking in here—any blockers? |
Contributor
Author
|
@pytorchbot rebase this please |
1 similar comment
Contributor
Author
|
@pytorchbot rebase this please |
driazati
approved these changes
Nov 5, 2019
Contributor
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@eellison is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Contributor
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.
Type objects in python have an attribute
__abstractmethods__that throws when it is accessed, so we were failing with an AttributeError whenever a type was used in TorchScript.This pr prevents that error from happening. We can't just throw when a type is used because it could be used to access a static method: #27163