Skip to content

[JIT] Plumb type annotations through script compilation (new)#9547

Closed
jamesr66a wants to merge 17 commits intopytorch:masterfrom
jamesr66a:type_annotations_lol
Closed

[JIT] Plumb type annotations through script compilation (new)#9547
jamesr66a wants to merge 17 commits intopytorch:masterfrom
jamesr66a:type_annotations_lol

Conversation

@jamesr66a
Copy link
Collaborator

@jamesr66a jamesr66a commented Jul 18, 2018

Supersedes #9405

@jamesr66a jamesr66a mentioned this pull request Jul 18, 2018
Closed
@jamesr66a jamesr66a force-pushed the type_annotations_lol branch 2 times, most recently from 323dab3 to 6632e46 Compare July 18, 2018 18:57
facebook-github-bot
facebook-github-bot reviewed Jul 18, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@jamesr66a has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

zdevito
zdevito approved these changes Jul 19, 2018
View reviewed changes
Copy link
Contributor

@zdevito zdevito left a comment

Choose a reason for hiding this comment

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

This looks good -- I have just some minor bugfix comments.

torch/csrc/jit/script/compiler.cpp Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/script/init.cpp Outdated

This comment was marked as off-topic.

Sign in to view

torch/jit/__init__.py Outdated

This comment was marked as off-topic.

Sign in to view

torch/jit/annotations.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

apaszke
apaszke previously requested changes Jul 19, 2018
View reviewed changes
Copy link
Contributor

@apaszke apaszke left a comment

Choose a reason for hiding this comment

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

I don't think we should represent returns as lists. If Python defines our reference semantics then it's not a good idea. Returning a single Tuple should be equivalent to returning multiple values (that automatically get packed into this tuple). That's what the type annotation parser did when I wrote it.

torch/csrc/jit/script/compiler.cpp Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/script/compiler.cpp Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/script/compiler.cpp Outdated

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/script/compiler.cpp Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/script/compiler.cpp Outdated

This comment was marked as off-topic.

Sign in to view

torch/jit/annotations.py Outdated

This comment was marked as off-topic.

Sign in to view

torch/jit/__init__.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

torch/jit/__init__.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

torch/jit/__init__.py Outdated

This comment was marked as off-topic.

Sign in to view

test/test_jit.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

facebook-github-bot
facebook-github-bot reviewed Jul 19, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@jamesr66a has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@jamesr66a jamesr66a added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jul 23, 2018
@jamesr66a jamesr66a force-pushed the type_annotations_lol branch from 8c4b436 to 46e8d90 Compare July 25, 2018 18:46
facebook-github-bot
facebook-github-bot reviewed Jul 25, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@jamesr66a has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@jamesr66a jamesr66a dismissed apaszke’s stale review July 25, 2018 18:56

per offline discussion

jramseyer pushed a commit to jramseyer/pytorch that referenced this pull request Jul 30, 2018
@jramseyer
Plumb type annotations through script compilation (new) (pytorch#9547)
15baae3 
Summary:
Supersedes pytorch#9405
Pull Request resolved: pytorch#9547

Reviewed By: zdevito

Differential Revision: D8900327

Pulled By: jamesr66a

fbshipit-source-id: a00a94615af4fbaec98ee3ede0cb54bcfd9108dd
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
Plumb type annotations through script compilation (new) (pytorch#9547)
5376b15 
Summary:
Supersedes pytorch#9405
Pull Request resolved: pytorch#9547

Reviewed By: zdevito

Differential Revision: D8900327

Pulled By: jamesr66a

fbshipit-source-id: a00a94615af4fbaec98ee3ede0cb54bcfd9108dd
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@zdevito zdevito zdevito approved these changes

@colesbury colesbury Awaiting requested review from colesbury

@ezyang ezyang Awaiting requested review from ezyang

@gchanan gchanan Awaiting requested review from gchanan

@soumith soumith Awaiting requested review from soumith

+1 more reviewer

@apaszke apaszke apaszke left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jamesr66a @zdevito @apaszke @facebook-github-bot @ezyang