Skip to content

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Oct 14, 2022

As suggested in #6775 (comment), this PR replaces all .view(...) calls with .reshape(...). In the regular case, they do the same thing, but in case .view() would fail because the input is noncontiguous, .reshape fixes this and afterwards performs the specified operation.

Having this earlier would have prevented #6772 and #6775.

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM. Should we do one run with @vfdev-5 script to confirm we didn't introduce any serious speed regression?

@pmeier
Copy link
Collaborator Author

pmeier commented Oct 14, 2022

-     1208    0.005    0.000    0.005    0.000 {method 'reshape' of 'torch._C._TensorBase' objects}
-    10135    0.025    0.000    0.031    0.000 {method 'view' of 'torch._C._TensorBase' objects}
+     6725    0.023    0.000    0.031    0.000 {method 'reshape' of 'torch._C._TensorBase' objects}
+     4618    0.010    0.000    0.010    0.000 {method 'view' of 'torch._C._TensorBase' objects}

I think we are good. Even if the difference is not measuring tolerance, we are talking about ((0.023 + 0.010) - (0.005 + 0.025)) / 2000 ~= 1.5 µs per sample here.

@pmeier pmeier merged commit f467349 into pytorch:main Oct 15, 2022
@pmeier pmeier deleted the view-reshape branch October 15, 2022 06:16
facebook-github-bot pushed a commit that referenced this pull request Oct 17, 2022
Reviewed By: NicolasHug

Differential Revision: D40427450

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