Switch default memory format of to (and similar) operators to Preserve by VitalyFedyunin · Pull Request #30088 · pytorch/pytorch

VitalyFedyunin · 2019-11-19T16:37:43Z

Stack from ghstack:

Fix .to operator to preserve strides when self should be returned #31227 Fix .to operator to preserve strides when self should be returned
Revert "Switch default memory format of to (and similar) operators to Preserve" #31226 Revert "Switch default memory format of to (and similar) operators to Preserve"
[WIP] Fix cudnn channels_last descriptio problem #31139 [WIP] Fix cudnn channels_last descriptio problem
Add memory_format support to zeros, ones, full #31131 Add memory_format support to zeros, ones, full (still need tests)
Switch default memory format of clone operator to Preserve #30089 Switch default memory format of clone operator to Preserve
Switch default memory format of to (and similar) operators to Preserve #30088 Switch default memory format of to (and similar) operators to Preserve
Switch default memory format of _like operators to Preserve #30087 Switch default memory format of _like operators to Preserve

Differential Revision: D18624984

[ghstack-poisoned]

…and similar) operators to Preserve" [ghstack-poisoned]

…perators to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

… of to (and similar) operators to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

…lar) operators to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

… operators to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

kostmo · 2019-12-12T17:17:51Z

CircleCI build failures summary

As of commit 2c146b9:

2/3 broken upstream at merge base 20a2e52 (see grid view)
- You may want to rebase on the viable/strict branch (see its recency history):
  - If your commit is newer than viable/strict, you can try basing on an older, stable commit:
```
git fetch viable/strict
git rebase --onto viable/strict $(git merge-base origin/master HEAD)
```
  - If your commit is older than viable/strict:
```
git fetch viable/strict
git rebase viable/strict
```
1/3 failures introduced in this PR

Detailed failure analysis

One may explore the probable reasons each build failed interactively on the Dr. CI website.

1 new failure recognized by patterns

The following build failures don't appear to be due to upstream breakage:

pytorch_xla_linux_xenial_py3_6_clang7_test (1/1)

Step: "Test" (details)

Dec 13 02:21:48 +++ eval 'extract_trap_cmd ' 
Dec 13 02:21:48 ++++ extract_trap_cmd 
Dec 13 02:21:48 ++++ printf '%s\n' '' 
Dec 13 02:21:48 +++ printf '%s\n' cleanup 
Dec 13 02:21:48 ++ trap -- ' 
Dec 13 02:21:48 cleanup' EXIT 
Dec 13 02:21:48 ++ which sccache 
Dec 13 02:21:48 ++ sccache --stop-server 
Dec 13 02:21:49 Stopping sccache server... 
Dec 13 02:21:49 error: couldn't connect to server 
Dec 13 02:21:49 caused by: Connection refused (os error 111) 
Dec 13 02:21:49 ++ true 
Dec 13 02:21:49 ++ rm /var/lib/jenkins/sccache_error.log 
Dec 13 02:21:49 ++ SCCACHE_ERROR_LOG=/var/lib/jenkins/sccache_error.log 
Dec 13 02:21:49 ++ SCCACHE_IDLE_TIMEOUT=1200 
Dec 13 02:21:49 ++ RUST_LOG=sccache::server=error 
Dec 13 02:21:49 ++ sccache --start-server 
Dec 13 02:21:49 Starting sccache server... 
Dec 13 02:21:49 ++ sccache --zero-stats 
Dec 13 02:21:49 Compile requests                 0 
Dec 13 02:21:49 Compile requests executed        0

2 failures not recognized by patterns:

Job	Step	Status
^{pytorch_linux_xenial_cuda9_cudnn7_py3_build}	^Build	🛑 Broken upstream
^{pytorch_libtorch_linux_xenial_cuda9_cudnn7_py3_build}	^Build	🛑 Broken upstream

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

Please report bugs/suggestions on the GitHub issue tracker.

This comment has been revised 7 times.

gchanan · 2019-12-12T17:22:41Z

aten/src/ATen/native/TensorConversions.cpp

 static inline Tensor to_impl(const Tensor& self, const TensorOptions& options, bool non_blocking, bool copy, c10::optional<c10::MemoryFormat> optional_memory_format) {
  auto memory_format =
-      optional_memory_format.value_or(MemoryFormat::Contiguous);
+      optional_memory_format.value_or(MemoryFormat::Preserve);


wasn't this broken before? Because if I didn't pass in a memory format, memory_format would be set to contiguous.

But then suggest_memory_format wouldn't be contiguous, so I create a new tensor and copy. But that doesn't seem like the correct semantics for the function.

Indeed, looks like (in master) if we have channels last tensor and call .to to same dtype etc. we will get contiguous output instead of self.

This PR will solve the issue.

is that behavior in 1.4? Should we introduce a patch to fix it?

it would probably be good to fix that in a separate patch -- in case this gets reverted (happens with BC changes), we can have the bug fix in without this.

and seems worthy of a test :).

…tors to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

facebook-github-bot · 2019-12-18T06:08:21Z

@VitalyFedyunin merged this pull request in fde3d70.

ghstack-source-id: 73c809d Pull Request resolved: pytorch/pytorch#30088

Summary: Pull Request resolved: pytorch#30088 Test Plan: Imported from OSS Differential Revision: D18624984 Pulled By: VitalyFedyunin fbshipit-source-id: 54901786d7496c7dce785140b0585ac9093b1d86

Switch default memory format of to (and similar) operators to Preserve

8d52c29

[ghstack-poisoned]

This was referenced Nov 19, 2019

Adding function to convert Module to channels last #28991

Closed

Switch default memory format of _like operators to Preserve #30087

Closed

Switch default memory format of clone operator to Preserve #30089

Closed

VitalyFedyunin added 3 commits November 20, 2019 13:45

rebase to master + fix tests on "Switch default memory format of to (…

ea6c3c0

…and similar) operators to Preserve" [ghstack-poisoned]

fix tests code on "Switch default memory format of to (and similar) o…

b918cd2

…perators to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

add experimental code to check model on "Switch default memory format…

68ce7d5

… of to (and similar) operators to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

VitalyFedyunin mentioned this pull request Nov 22, 2019

[WIP] Copy of #30174 Fix suggest_memory_format() for ambiguous cases #30325

Closed

experiments with tag on "Switch default memory format of to (and simi…

9c0d2cf

…lar) operators to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

This was referenced Dec 4, 2019

WIP #30741

Closed

Revert "[WIP] Copy of #30174 Fix suggest_memory_format() for ambiguous cases" #30742

Closed

[WIP] Add channels last tag #30743

Closed

[WIP] Add channels last tag #30745

Closed

3d t support #30992

Closed

[WIP] Fixing resize_ #30993

Closed

This was referenced Dec 11, 2019

Add memory_format support to zeros, ones, full #31131

Closed

[WIP] Fix cudnn channels_last descriptio problem #31139

Closed

rebase to master on "Switch default memory format of to (and similar)…

9493c0d

… operators to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

VitalyFedyunin requested a review from gchanan December 12, 2019 16:47

gchanan reviewed Dec 12, 2019

View reviewed changes

gchanan approved these changes Dec 12, 2019

View reviewed changes

Fix flake8 on "Switch default memory format of to (and similar) opera…

47757e3

…tors to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

This was referenced Dec 13, 2019

Revert "Switch default memory format of to (and similar) operators to Preserve" #31226

Closed

Fix .to operator to preserve strides when self should be returned #31227

Closed

Fix flake8 on "Switch default memory format of to (and similar) opera…

2c146b9

…tors to Preserve" Differential Revision: [D18624984](https://our.internmc.facebook.com/intern/diff/D18624984) [ghstack-poisoned]

facebook-github-bot closed this in fde3d70 Dec 15, 2019

facebook-github-bot added the merged label Dec 18, 2019

facebook-github-bot deleted the gh/VitalyFedyunin/88/head branch December 18, 2019 15:17

xxtEchjovs44 pushed a commit to xxtEchjovs44/pytorch that referenced this pull request Jan 29, 2020

Switch default memory format of to (and similar) operators to Preserve

bc329f2

ghstack-source-id: 73c809d Pull Request resolved: pytorch/pytorch#30088

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch default memory format of to (and similar) operators to Preserve#30088

Switch default memory format of to (and similar) operators to Preserve#30088
VitalyFedyunin wants to merge 8 commits intogh/VitalyFedyunin/88/basefrom
gh/VitalyFedyunin/88/head

VitalyFedyunin commented Nov 19, 2019 •

edited

Loading

Uh oh!

kostmo commented Dec 12, 2019 •

edited

Loading

Uh oh!

gchanan Dec 12, 2019

Uh oh!

VitalyFedyunin Dec 12, 2019

Uh oh!

gchanan Dec 12, 2019

Uh oh!

gchanan Dec 12, 2019

Uh oh!

gchanan Dec 12, 2019

Uh oh!

facebook-github-bot commented Dec 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

VitalyFedyunin commented Nov 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kostmo commented Dec 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CircleCI build failures summary

Detailed failure analysis

1 new failure recognized by patterns

pytorch_xla_linux_xenial_py3_6_clang7_test (1/1)

2 failures not recognized by patterns:

Uh oh!

gchanan Dec 12, 2019

Choose a reason for hiding this comment

Uh oh!

VitalyFedyunin Dec 12, 2019

Choose a reason for hiding this comment

Uh oh!

gchanan Dec 12, 2019

Choose a reason for hiding this comment

Uh oh!

gchanan Dec 12, 2019

Choose a reason for hiding this comment

Uh oh!

gchanan Dec 12, 2019

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Dec 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

VitalyFedyunin commented Nov 19, 2019 •

edited

Loading

kostmo commented Dec 12, 2019 •

edited

Loading