Skip to content

[fix] torch.cat: Don't resize out if it is already of the correct size.#49937

Closed
kshitij12345 wants to merge 6 commits intopytorch:masterfrom
kshitij12345:fix/cat-resize
Closed

[fix] torch.cat: Don't resize out if it is already of the correct size.#49937
kshitij12345 wants to merge 6 commits intopytorch:masterfrom
kshitij12345:fix/cat-resize

Conversation

@kshitij12345
Copy link
Copy Markdown
Collaborator

Fixes #49878

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Dec 29, 2020

💊 CI failures summary and remediations

As of commit 2337b9c (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 to the (internal) Dr. CI Users group.

This comment has been revised 21 times.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 29, 2020

Codecov Report

Merging #49937 (2337b9c) into master (bf4fcab) will increase coverage by 0.19%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #49937      +/-   ##
==========================================
+ Coverage   80.49%   80.68%   +0.19%     
==========================================
  Files        1902     1902              
  Lines      206356   206357       +1     
==========================================
+ Hits       166099   166494     +395     
+ Misses      40257    39863     -394     

Comment thread aten/src/ATen/native/TensorShape.cpp Outdated
result.resize_(result_size, first_tensor_mem_format);

// skip resizing if size of result is same as expected
if ((result.sizes() != result_size) || result.suggest_memory_format() != first_tensor_mem_format){
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do you need second condition here? It would lead to restriding of result instead of respecting its strides?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The reason for second condition is that if result has the standard layout and if first_tensor_mem_format is say channels_last, then even if they have same shape, their strides would be different. In that case we would want to resize the result.

Sample:

>>> torch.ones((4, 3, 2, 5)).stride()
(30, 10, 5, 1)
>>> torch.ones((4, 3, 2, 5)).contiguous(memory_format=torch.channels_last).stride()
(30, 1, 15, 3)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

According to https://github.com/pytorch/pytorch/wiki/Developer-FAQ#how-does-out-work-in-pytorch, if out has the correct size it has to be used directly, even if it's strides are not what operation would naturally produce.

Comment thread test/test_tensor_creation_ops.py Outdated
Comment thread test/test_tensor_creation_ops.py Outdated
Comment thread test/test_tensor_creation_ops.py Outdated
* update condition to conform out= behaviour
@kshitij12345
Copy link
Copy Markdown
Collaborator Author

There looks to be merge conflict in CI. Pushing a new commit.

@kshitij12345 kshitij12345 marked this pull request as ready for review January 8, 2021 12:24
Copy link
Copy Markdown
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.

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

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@ngimel merged this pull request in 36ddb00.

laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
…e. (pytorch#49937)

Summary:
Fixes pytorch#49878

Pull Request resolved: pytorch#49937

Reviewed By: mruberry

Differential Revision: D25851564

Pulled By: ngimel

fbshipit-source-id: 9a78922642d5bace70d887a88fa9e92d88038120
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.

torch::cat_out unexpected result on views

4 participants