Warn about poor performance creating Tensor from list of numpy.array's #51680

kurtamohler · 2021-02-03T23:23:10Z

Fixes #13918

facebook-github-bot · 2021-02-03T23:23:27Z

💊 CI failures summary and remediations

As of commit 65216b8 (more details on the Dr. CI page and at hud.pytorch.org/pr/51680):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

Preview docs built from this PR

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.

Click here to manually regenerate this comment.

torch/csrc/utils/tensor_new.cpp

test/test_torch.py

kurtamohler · 2021-02-04T18:21:19Z

Looks like clang-tidy failure is upstream. Otherwise, I think this is ready to go, @mruberry

codecov · 2021-02-04T21:38:13Z

Codecov Report

Merging #51680 (7450521) into master (001ff3a) will increase coverage by 0.30%.
The diff coverage is 92.00%.

❗ Current head 7450521 differs from pull request most recent head 65216b8. Consider uploading reports for the commit 65216b8 to get more accurate results

@@            Coverage Diff             @@
##           master   #51680      +/-   ##
==========================================
+ Coverage   80.60%   80.91%   +0.30%     
==========================================
  Files        1879     1948      +69     
  Lines      202892   212345    +9453     
==========================================
+ Hits       163543   171818    +8275     
- Misses      39349    40527    +1178

test/test_numpy_interop.py

torch/csrc/utils/tensor_new.cpp

mruberry · 2021-03-15T16:45:10Z

Either this or #51731, depending on which gets in first, will need to update to account for the other.

kurtamohler · 2021-03-23T18:59:53Z

Either this or #51731, depending on which gets in first, will need to update to account for the other.

The description of that PR says that it speeds up torch.tensor() to be "at least as fast (usually faster)" compared to torch.tensor(numpy.stack(list_of_arrays)). The benchmark results seem to support that claim. So I think that PR will make this one unnecessary, won't it?

ezyang · 2021-07-19T13:53:37Z

@mruberry @kurtamohler what ever happened to this PR?

kurtamohler · 2021-07-21T18:04:11Z

@ezyang, I paused this because of #51731, but it seems that PR has paused too. I'll update this PR today

facebook-github-bot · 2021-07-22T14:12:46Z

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

facebook-github-bot · 2021-07-22T19:05:05Z

@ezyang merged this pull request in 5a00152.

kurtamohler added module: performance Issues related to performance, either of kernel code or framework glue module: numpy Related to numpy support, and also numpy compatibility of our operators labels Feb 3, 2021

kurtamohler requested a review from ezyang February 3, 2021 23:23

facebook-github-bot added the cla signed label Feb 3, 2021

kurtamohler commented Feb 3, 2021

View reviewed changes

torch/csrc/utils/tensor_new.cpp Outdated Show resolved Hide resolved

pytorchbot added the open source label Feb 3, 2021

kurtamohler force-pushed the list-of-ndarray-warning-13918 branch from c4fb89d to e1c46b1 Compare February 3, 2021 23:53

mruberry requested review from mruberry and removed request for ezyang February 4, 2021 03:57

mruberry added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Feb 4, 2021