Skip to content

Conversation

@gchanan
Copy link
Contributor

@gchanan gchanan commented Oct 22, 2019

Stack from ghstack:

Differential Revision: D18063328

gchanan added a commit that referenced this pull request Oct 22, 2019
ghstack-source-id: 851c516
Pull Request resolved: #28425
@gchanan gchanan requested a review from mruberry October 22, 2019 17:35
return result.copy_(self);
}

bool is_set_to(const Tensor &self, const Tensor& src) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tensor & vs Tensor&?

self.storage_offset() == src.storage_offset() &&
self.dim() == src.dim())
{
for (int64_t d = 0; d < self.dim(); ++d)
Copy link
Collaborator

Choose a reason for hiding this comment

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

for (auto d = decltype(self.dim()){0}; d < ...

auto result = at::empty_like(self, self.options(), memory_format);
return result.copy_(self);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this function need a comment? The name isn't super clear.

for (int64_t d = 0; d < self.dim(); ++d)
{
if (self.size(d) != src.size(d) || self.stride(d) != src.stride(d)) {
return true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

not return false here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right.

Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

Need to verify return values. Style comments optional (but comments are always nice).

@mruberry mruberry self-requested a review October 22, 2019 18:22
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

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

Approving (assuming you'll fix lint).

zdevito pushed a commit to zdevito/ATen that referenced this pull request Oct 24, 2019
Summary: Pull Request resolved: pytorch/pytorch#28425

Test Plan: Imported from OSS

Differential Revision: D18063328

Pulled By: gchanan

fbshipit-source-id: 86af01a630d88c30947b8c85d1fac86dd7b40585
@facebook-github-bot
Copy link
Contributor

@gchanan merged this pull request in 4f0a350.

@facebook-github-bot facebook-github-bot deleted the gh/gchanan/97/head branch October 28, 2019 22:13
thiagocrepaldi pushed a commit to thiagocrepaldi/pytorch that referenced this pull request Feb 4, 2020
Summary: Pull Request resolved: pytorch#28425

Test Plan: Imported from OSS

Differential Revision: D18063328

Pulled By: gchanan

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants