Skip to content

Conversation

@gchanan
Copy link
Contributor

@gchanan gchanan commented Feb 23, 2018

This is likely not a robust fix because it special cases the case where both the indices and values are empty rather than handling each one separately. But this is currently blocking a change introducing devices to constructors.

This is likely not a robust fix because it special cases the case where both the indices and values are empty
rather than handling each one separately.  But this is currently blocking a change introducing devices to constructors.
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.

Looks good! I had one minor comment

// TODO: we may need to special case when only one of these are empty.
if (THLongTensor_nDimension(indices) == 0 && THTensor_(nDimension)(values) == 0) {
nDimI = 0;
nDimV = THLongStorage_size(sizes);

This comment was marked as off-topic.

This comment was marked as off-topic.

// TODO: we may need to special case when only one of these are empty.
if (THCudaLongTensor_nDimension(state, indices) == 0 && THCTensor_(nDimension)(state, values) == 0) {
nDimI = 0;
nDimV = THLongStorage_size(sizes);

This comment was marked as off-topic.

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!

@soumith soumith merged commit 2130070 into pytorch:master Feb 23, 2018
@soumith
Copy link
Contributor

soumith commented Feb 23, 2018

onnx failures are unrelated

jamesr66a pushed a commit to jamesr66a/pytorch that referenced this pull request Feb 24, 2018
* Handle copying empty sparse tensors to/from CPU, GPU.

This is likely not a robust fix because it special cases the case where both the indices and values are empty
rather than handling each one separately.  But this is currently blocking a change introducing devices to constructors.

* Guard sizes being NULL.
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.

3 participants