Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BC-breaking] Remove _new_empty_tensor op #3156

Merged
merged 2 commits into from
Dec 11, 2020

Conversation

datumbox
Copy link
Contributor

If we merge this PR, we should clean up the single FBCode ref post merge.

@codecov
Copy link

codecov bot commented Dec 11, 2020

Codecov Report

Merging #3156 (c4be6e5) into master (e337103) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3156      +/-   ##
==========================================
+ Coverage   72.75%   72.79%   +0.03%     
==========================================
  Files          99       98       -1     
  Lines        8979     8964      -15     
  Branches     1431     1430       -1     
==========================================
- Hits         6533     6525       -8     
+ Misses       1999     1992       -7     
  Partials      447      447              
Impacted Files Coverage Δ
torchvision/ops/__init__.py 100.00% <ø> (ø)
torchvision/ops/_register_onnx_ops.py 53.12% <ø> (+4.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e337103...90eb2f7. Read the comment docs.

@fmassa fmassa changed the title Remove _new_empty_tensor op [BC-breaking] Remove _new_empty_tensor op Dec 11, 2020
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -76,20 +76,6 @@ def to_numpy(tensor):
else:
raise

@unittest.skip("Disable test until Split w/ zero sizes is implemented in ORT")
def test_new_empty_tensor(self):
Copy link
Member

Choose a reason for hiding this comment

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

My initial reaction was to keep this test, as it's not using the new_empty_tensor op but instead tries to enforce that nn.Conv2d support empty batch sizes (which was the original use-case for having a new_empty_tensor)

But given that this test has been disabled for a long time, let's just remove it.

@fmassa fmassa merged commit 07a9c95 into pytorch:master Dec 11, 2020
@datumbox datumbox deleted the bugfix/remove_empty_tensor_op branch December 11, 2020 09:44
facebook-github-bot pushed a commit that referenced this pull request Dec 15, 2020
Summary: Co-authored-by: Francisco Massa <fvsmassa@gmail.com>

Reviewed By: datumbox

Differential Revision: D25531033

fbshipit-source-id: 735bdd211edfb49bc97fae8558049214d6a4b169
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants