Skip to content

Fix narrow on empty tensors after negative size support.#9838

Closed
gchanan wants to merge 1 commit intopytorch:masterfrom
gchanan:empty_narrow
Closed

Fix narrow on empty tensors after negative size support.#9838
gchanan wants to merge 1 commit intopytorch:masterfrom
gchanan:empty_narrow

Conversation

@gchanan
Copy link
Contributor

@gchanan gchanan commented Jul 25, 2018

No description provided.

Copy link
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.

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

@gchanan
Copy link
Contributor Author

gchanan commented Jul 25, 2018

@pytorchbot retest this please.

Copy link
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for catching this @gchanan!

I'm not sure about the error message but that can be fixed later

auto cur_size = self.size(dim);
start = maybe_wrap_dim(start, cur_size);
if (start != cur_size) { // start being the end is valid, but not a valid dim specification.
start = maybe_wrap_dim(start, cur_size);

This comment was marked as off-topic.

This comment was marked as off-topic.

@bhushan23
Copy link
Contributor

can we also add test case for start = cur_size in test_narrow?

@gchanan
Copy link
Contributor Author

gchanan commented Jul 25, 2018

We have one if you compile with zero size dim support CFLAGS="-DUSE_TH_SIZE_ZERO_DIM" -- it's a little tricky without that because it would only be valid for a 1-dimensional tensor.

@gchanan
Copy link
Contributor Author

gchanan commented Jul 25, 2018

@pytorchbot retest this please.

1 similar comment
@gchanan
Copy link
Contributor Author

gchanan commented Jul 25, 2018

@pytorchbot retest this please.

@ezyang
Copy link
Contributor

ezyang commented Jul 26, 2018

@pytorchbot retest this please

@bhushan23
Copy link
Contributor

@gchanan MemongerTest.test_simple_memonger is asserting

@gchanan
Copy link
Contributor Author

gchanan commented Jul 26, 2018

that's a caffe2 test, I don't think it's related.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Jul 27, 2018
Summary: Pull Request resolved: pytorch/pytorch#9838

Differential Revision: D9002345

Pulled By: gchanan

fbshipit-source-id: 13f4bacff94d9d0ea31a3b73a75b9b3e774eabf5
jramseyer pushed a commit to jramseyer/pytorch that referenced this pull request Jul 30, 2018
Summary: Pull Request resolved: pytorch#9838

Differential Revision: D9002345

Pulled By: gchanan

fbshipit-source-id: 13f4bacff94d9d0ea31a3b73a75b9b3e774eabf5
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
Summary: Pull Request resolved: pytorch#9838

Differential Revision: D9002345

Pulled By: gchanan

fbshipit-source-id: 13f4bacff94d9d0ea31a3b73a75b9b3e774eabf5
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants