Skip to content

Fix various sparse transpose issues; remove dead code from Declaratio…#7200

Merged
gchanan merged 7 commits intopytorch:masterfrom
gchanan:sparse_transpose
May 18, 2018
Merged

Fix various sparse transpose issues; remove dead code from Declaratio…#7200
gchanan merged 7 commits intopytorch:masterfrom
gchanan:sparse_transpose

Conversation

@gchanan
Copy link
Contributor

@gchanan gchanan commented May 2, 2018

…ns.yaml.

  1. Fixes some checks in t_, transpose_ that don't allow transposing empty sparse tensors.
  2. Remove out= variants from docs since they don't exist (and haven't since at least v0.3.1).
  3. Unify implementations of t_, transpose_, t, transpose.
  4. Move dead checking code from Declarations.cwrap to actual implementations.
  5. Fix test which never tested transpose_.

…ns.yaml.

1) Fixes some checks in t_, transpose_ that don't allow transposing empty sparse tensors.
2) Remove out= variants from docs since they don't exist (and haven't since at least v0.3.1).
3) Unify implementations of t_, transpose_, t, transpose.
4) Move dead checking code from Declarations.cwrap to actual implementations.
5) Fix test which never tested transpose_.
@gchanan
Copy link
Contributor Author

gchanan commented May 2, 2018

CC @zou3519.

Should address #7010.

Also, when this and #7194 are merged, there should be no more dead code in Declarations.yaml.

@ezyang ezyang requested a review from zou3519 May 2, 2018 21:07
row0.copy_(row1);
row1.copy_(tmp);
return self.sparse_raw_resize_(sizes, self._dimI(), self._dimV());
} else {

This comment was marked as off-topic.

This comment was marked as off-topic.

int64_t nDimI = self._dimI();
int64_t nDimV = self._dimV();
if (!(nDimI == 2 && nDimV == 0)) {
AT_ERROR(fn, " expects a 2D sparse tensor, but self is ", nDimI, "D indices and ", nDimV, "D values");

This comment was marked as off-topic.

This comment was marked as off-topic.

if (!(nDimI == 2 && nDimV == 0)) {
AT_ERROR(fn, " expects a 2D sparse tensor, but self is ", nDimI, "D indices and ", nDimV, "D values");
}
} else if (self.ndimension() != 2) {

This comment was marked as off-topic.

AT_ERROR(fn, " expects a 2D sparse tensor, but self is ", nDimI, "D indices and ", nDimV, "D values");
}
} else if (self.ndimension() != 2) {
AT_ERROR(fn, " expects a 2D tensor, but self is ", self.ndimension());

This comment was marked as off-topic.

x.t_()
self.assertEqual(torch.Size([3, 2]), x.size())
self.assertEqual(0, x._indices().numel())
self.assertEqual(0, x._values().numel())

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 as long as tests pass!

@ezyang
Copy link
Contributor

ezyang commented May 13, 2018

17:25:23 ERROR: test_cpp (test_jit.TestJit)
17:25:23 ----------------------------------------------------------------------
17:25:23 Traceback (most recent call last):
17:25:23   File "test_jit.py", line 923, in test_cpp
17:25:23     self.assertExpected(torch._C._jit_run_cpp_tests())
17:25:23 RuntimeError: t() expects a 2D tensor, but self is 4D (check_t at /var/lib/jenkins/workspace/aten/src/ATen/native/TensorShape.cpp:475)

@gchanan gchanan merged commit 4f20a0e into pytorch:master May 18, 2018
onnxbot added a commit to onnxbot/onnx-fb-universe that referenced this pull request May 18, 2018
weiyangfb pushed a commit to weiyangfb/pytorch that referenced this pull request Jun 11, 2018
pytorch#7200)

* Fix various sparse transpose issues; remove dead code from Declarations.yaml.

1) Fixes some checks in t_, transpose_ that don't allow transposing empty sparse tensors.
2) Remove out= variants from docs since they don't exist (and haven't since at least v0.3.1).
3) Unify implementations of t_, transpose_, t, transpose.
4) Move dead checking code from Declarations.cwrap to actual implementations.
5) Fix test which never tested transpose_.

* Add test for error with t, t_.

* Address review comments.

* Fix jit tests.

* Fix test_jit.
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