Skip to content

Add (constant size) TensorLists to JIT, use them in cat and stack nodes #9948

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

Add (constant size) TensorLists to JIT, use them in cat and stack nodes #9948

wants to merge 6 commits into from

Conversation

apaszke
Copy link
Contributor

@apaszke apaszke commented Jul 27, 2018

@apaszke apaszke force-pushed the jit_list_flatten branch from bed465e to 467d7d1 Compare July 27, 2018 19:43
facebook-github-bot
facebook-github-bot reviewed Jul 27, 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.

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

@apaszke apaszke force-pushed the jit_list_flatten branch from 467d7d1 to ed70fd8 Compare July 27, 2018 23:47
facebook-github-bot
facebook-github-bot reviewed Jul 27, 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.

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

@pjh5
Copy link
Contributor

pjh5 commented Jul 28, 2018

@pytorchbot retest this please

@apaszke apaszke mentioned this pull request Jul 28, 2018
Closed
zdevito
zdevito approved these changes Jul 30, 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! Cleans up a lot of stuff.

tools/jit/gen_jit_dispatch.py
# is_positional_arg is a boolean list the same length as decl['arguments']
# that indicates if the argument should come from the postional list
# of inputs. If false, the argument comes from the constant attributes
kw_assignments = []
arguments = []

This comment was marked as off-topic.

Sign in to view

tools/jit/gen_jit_dispatch.py

real_inputs = 0
for i, arg in enumerate(decl['arguments']):
# This conditional allows us to process argument lists with a flattened argument list

This comment was marked as off-topic.

Sign in to view

tools/jit/gen_jit_dispatch.py
# in some cases there are no inputs that are possibly attributes, so the
# variants are actually the same. If so avoid generating both to save compilation
# time.
if all_real_arguments_are_inputs != only_tensors_are_inputs:
variants += [',', emit_decl_variant(decl, only_tensors_are_inputs, has_tensorlist)]
variants += [',', emit_decl_variant(decl, only_tensors_are_inputs)]

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/ir.cpp
int64_t arg_pos = findArgument(schema(), name).first;
// XXX: we don't have a single value we could give for a Tensor[],

This comment was marked as off-topic.

Sign in to view

torch/csrc/jit/passes/graph_fuser.cpp
@@ -523,11 +541,14 @@ struct GraphFuser {
std::pair<graph_node_list::iterator, bool> scanNode(Node * consumer) {
auto stage_guard = block->owningGraph()->setStageTemporary(consumer->stage());
if(isFusableAsExitNode(consumer)) {
value_list inputs;
auto consumer_inputs = consumer->kind() == aten::cat ?

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
// Allow homogeneous tuples to be casted implicitly to lists of appropriate types
if (arg.type->kind() == TypeKind::ListType &&
value->type()->kind() == TypeKind::TupleType &&
value->type()->isSubtypeOf(arg.type)) {

This comment was marked as off-topic.

Sign in to view

@zdevito zdevito added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jul 30, 2018
@apaszke apaszke force-pushed the jit_list_flatten branch from c4b8a29 to 1b3503c Compare July 30, 2018 23:41
facebook-github-bot
facebook-github-bot reviewed Jul 31, 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.

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

jramseyer pushed a commit to jramseyer/pytorch that referenced this pull request Jul 31, 2018
@apaszke @jramseyer
Add (constant size) TensorLists to JIT, use them in cat and stack nod…
93af271 
…es (pytorch#9948)

Summary:
zdevito
Pull Request resolved: pytorch#9948

Reviewed By: ezyang

Differential Revision: D9033666

Pulled By: apaszke

fbshipit-source-id: 02d75e391ed6dee62500842df50f0b6ee5e38846
facebook-github-bot pushed a commit that referenced this pull request Jul 31, 2018
@apaszke @facebook-github-bot
Slightly relax the constraints on argument and return types to script…
1f13453 
… functions (#9969)

Summary:
This lays out initial support for taking and returning a richer set
of types than only tensors. Floats and ints are already valid, lists are
straightforward to add, tuples need some discussion.

Based on top of #9948. Review only the last commit.

zdevito
Pull Request resolved: #9969

Reviewed By: zdevito

Differential Revision: D9076973

Pulled By: apaszke

fbshipit-source-id: 5a1fe912ea6b79ab2bfd0dcce265eb05855b5ff0
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
@apaszke
Add (constant size) TensorLists to JIT, use them in cat and stack nod…
baa8f03 
…es (pytorch#9948)

Summary:
zdevito
Pull Request resolved: pytorch#9948

Reviewed By: ezyang

Differential Revision: D9033666

Pulled By: apaszke

fbshipit-source-id: 02d75e391ed6dee62500842df50f0b6ee5e38846
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
@apaszke
Slightly relax the constraints on argument and return types to script…
b456924 
… functions (pytorch#9969)

Summary:
This lays out initial support for taking and returning a richer set
of types than only tensors. Floats and ints are already valid, lists are
straightforward to add, tuples need some discussion.

Based on top of pytorch#9948. Review only the last commit.

zdevito
Pull Request resolved: pytorch#9969

Reviewed By: zdevito

Differential Revision: D9076973

Pulled By: apaszke

fbshipit-source-id: 5a1fe912ea6b79ab2bfd0dcce265eb05855b5ff0
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

@jamesr66a jamesr66a jamesr66a approved these changes

@anderspapitto anderspapitto Awaiting requested review from anderspapitto

@bddppq bddppq Awaiting requested review from bddppq

@colesbury colesbury Awaiting requested review from colesbury

@dzhulgakov dzhulgakov Awaiting requested review from dzhulgakov

@ezyang ezyang Awaiting requested review from ezyang

@gchanan gchanan Awaiting requested review from gchanan

@houseroad houseroad Awaiting requested review from houseroad

@smessmer smessmer Awaiting requested review from smessmer

@soumith soumith Awaiting requested review from soumith

@Yangqing Yangqing Awaiting requested review from Yangqing

Assignees
No one assigned
Labels
oncall: jit Add this issue/PR to JIT oncall triage queue open source
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@apaszke @pjh5 @zdevito @jamesr66a @facebook-github-bot @ezyang