Skip to content

Conversation

ZolotukhinM
Copy link

@ZolotukhinM ZolotukhinM commented Jul 15, 2020

Stack from ghstack:

Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

Differential Revision: D22543470

Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

[ghstack-poisoned]
@ZolotukhinM ZolotukhinM requested a review from apaszke as a code owner July 15, 2020 04:21
ZolotukhinM pushed a commit that referenced this pull request Jul 15, 2020
Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

ghstack-source-id: 458c317
Pull Request resolved: #41451
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jul 15, 2020
@dr-ci
Copy link

dr-ci bot commented Jul 15, 2020

💊 CI failures summary and remediations

As of commit 4c3383f (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 28 times.

@ZolotukhinM ZolotukhinM requested a review from bertmaher July 15, 2020 16:36
Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

Differential Revision: [D22543470](https://our.internmc.facebook.com/intern/diff/D22543470)

[ghstack-poisoned]
Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

Differential Revision: [D22543470](https://our.internmc.facebook.com/intern/diff/D22543470)

[ghstack-poisoned]
Mikhail Zolotukhin added 4 commits July 28, 2020 17:34
Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

Differential Revision: [D22543470](https://our.internmc.facebook.com/intern/diff/D22543470)

[ghstack-poisoned]
Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

Differential Revision: [D22543470](https://our.internmc.facebook.com/intern/diff/D22543470)

[ghstack-poisoned]
Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

Differential Revision: [D22543470](https://our.internmc.facebook.com/intern/diff/D22543470)

[ghstack-poisoned]
Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

Differential Revision: [D22543470](https://our.internmc.facebook.com/intern/diff/D22543470)

[ghstack-poisoned]
Copy link
Contributor

@eellison eellison left a comment

Choose a reason for hiding this comment

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

Accepting to unblock. We're going to need to think up a more comprehensive testing strategy before enabling TE Fuser on in master, which should hopefully flesh out any issues here. I also suspect a lot of this will need to be rethought and revisited if/when we enable TE with unknown dimensions.

@Krovatkin maybe you can take a look as well

static std::pair<std::vector<ExprHandle>, bool> broadcastShapes(
std::vector<ExprHandle> TensorExprKernel::broadcastShapes(
std::vector<std::vector<ExprHandle>> shapes) {
bool seenBroadcast = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

unused

Mikhail Zolotukhin added 2 commits July 31, 2020 15:57
Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

Differential Revision: [D22543470](https://our.internmc.facebook.com/intern/diff/D22543470)

[ghstack-poisoned]
Since TE operates on a limited subset of ops with a well-defined
semantics, we can easily infer shapes of intermediate and output tensors
given shapes of the inputs.

There is a couple of ops that are not yet supported in the shape
inference, once we add them we could relax the shape info requirements
in the TE fuser: currently it requires all values in the fusion group to
have shapes known and we can change it to only inputs.

Differential Revision: [D22543470](https://our.internmc.facebook.com/intern/diff/D22543470)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

@ZolotukhinM merged this pull request in 2deccce.

@facebook-github-bot facebook-github-bot deleted the gh/ZolotukhinM/273/head branch August 4, 2020 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merged oncall: jit Add this issue/PR to JIT oncall triage queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants