Skip to content

fix unflatten_dense_tensor when there is empty tensor inside#50321

Closed
zhaojuanmao wants to merge 1 commit intopytorch:masterfrom
zhaojuanmao:export-D25859503
Closed

fix unflatten_dense_tensor when there is empty tensor inside#50321
zhaojuanmao wants to merge 1 commit intopytorch:masterfrom
zhaojuanmao:export-D25859503

Conversation

@zhaojuanmao
Copy link
Copy Markdown
Contributor

Summary:
Quantization team reported that when there are two empty tensors are replicated among ranks, the two empty tensors start to share storage after resizing.

The root cause is unflatten_dense_tensor unflattened the empty tensor as view of flat tensor and thus share storage with other tensors.

This PR is trying to avoid unflatten the empty tensor as view of flat tensor so that empty tensor will not share storage with other tensors.

Test Plan: unit test

Differential Revision: D25859503

@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Jan 9, 2021

💊 CI failures summary and remediations

As of commit 98573e6 (more details on the Dr. CI page):


  • 1/1 failures introduced in this PR

1 failure not recognized by patterns:

Job Step Action
CircleCI pytorch_vulkan_linux_bionic_py3_6_clang9_build (Optional) Merge target branch 🔁 rerun

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.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D25859503

1 similar comment
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D25859503

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D25859503

@vkuzo
Copy link
Copy Markdown
Contributor

vkuzo commented Jan 11, 2021

Thanks for fixing this! Test plan LG, code lg as well but I'd hope to wait from someone who is more familiar with the code to accept.

@zhaojuanmao
Copy link
Copy Markdown
Contributor Author

@mrshenli @pritamdamania87 would you please help reviewing this diff? thanks!

Comment thread torch/csrc/utils/tensor_flatten.h Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: We don't need flat_tensor_view here right? We can just use flat.options()? Then we only need to create flat_tensor_view in the else block below?

Comment thread test/cpp/api/tensor_flatten.cpp Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we assert here that each one's data_ptr is 0 instead of asserting both data_ptrs are equal? This assertion seems to suggest both should share the same storage (which they shouldn't). They're equal just from the fact that they are empty for now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sounds good!

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D25859503

…#50321)

Summary:
Pull Request resolved: pytorch#50321

Quantization team reported that when there are two empty tensors are replicated among ranks, the two empty tensors start to share storage after resizing.

The root cause is unflatten_dense_tensor unflattened the empty tensor as view of flat tensor and thus share storage with other tensors.

This PR is trying to avoid unflatten the empty tensor as view of flat tensor so that empty tensor will not share storage with other tensors.

Test Plan: unit test

Reviewed By: pritamdamania87

Differential Revision: D25859503

fbshipit-source-id: 4cb9756911a3bcaf46daf66b5a66b08fe9f1bda5
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D25859503

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been merged in c9cae14.

laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
…#50321)

Summary:
Pull Request resolved: pytorch#50321

Quantization team reported that when there are two empty tensors are replicated among ranks, the two empty tensors start to share storage after resizing.

The root cause is unflatten_dense_tensor unflattened the empty tensor as view of flat tensor and thus share storage with other tensors.

This PR is trying to avoid unflatten the empty tensor as view of flat tensor so that empty tensor will not share storage with other tensors.

Test Plan: unit test

Reviewed By: pritamdamania87

Differential Revision: D25859503

fbshipit-source-id: 5b760b31af6ed2b66bb22954cba8d1514f389cca
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.

4 participants