Skip to content

Conversation

datumbox
Copy link
Contributor

Resolving #6053 (comment)

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

LGTM.

For my own understanding, do we need a Permute module, or could we just rely on torch.permute? Looks like tihs Permute is a very simple wrapper

@datumbox
Copy link
Contributor Author

datumbox commented May 20, 2022

@NicolasHug you are right that Permute is nothing but a wrapper for torch.permute. Unfortunately the latter can't be used as part of sequentials, which are very common in many of our models. I thought that instead of rewriting this over and over, it would be useful to have it as a shortcut on ops but I don't have strong opinions. Thoughts?

Another alternative is to move it to some _utils and make it private. That's also possible.

@datumbox
Copy link
Contributor Author

As discussed offline with @NicolasHug, it's OK to have this as an op.

@datumbox datumbox merged commit d57f929 into pytorch:main May 20, 2022
@datumbox datumbox deleted the ops/permute branch May 20, 2022 09:09
facebook-github-bot pushed a commit that referenced this pull request Jun 1, 2022
Reviewed By: NicolasHug

Differential Revision: D36760916

fbshipit-source-id: e71606bcdb48b74adf65c1def01c8f1ded22f8b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants