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

[Fix] torch.new_tensor will cause device error when pytorch>=1.7 #465

Merged
merged 1 commit into from
Aug 6, 2021

Conversation

nbei
Copy link
Collaborator

@nbei nbei commented Aug 5, 2021

No description provided.

@nbei nbei requested review from ckkelvinchan and Yshuo-Li August 5, 2021 12:14
@innerlee
Copy link
Contributor

innerlee commented Aug 5, 2021

Can this be tested (in cpu mode)?

@nbei
Copy link
Collaborator Author

nbei commented Aug 5, 2021

Can this be tested (in cpu mode)?

Sure. The modification aims to guarantee that two tensors are in the same device.

@innerlee
Copy link
Contributor

innerlee commented Aug 5, 2021

Okay then please add a test case for this ;) My understanding is that the current code is only tested on gpu mode, so it will slip through the ci

@nbei
Copy link
Collaborator Author

nbei commented Aug 5, 2021

Okay then please add a test case for this ;) My understanding is that the current code is only tested on gpu mode, so it will slip through the ci

No. Current code has been tested on CPU. It will not report error, because the tensors will be put on CPU by default.

@codecov
Copy link

codecov bot commented Aug 5, 2021

Codecov Report

Merging #465 (5f289a5) into master (79e7dea) will decrease coverage by 0.56%.
The diff coverage is 60.52%.

❗ Current head 5f289a5 differs from pull request most recent head 74f81e0. Consider uploading reports for the commit 74f81e0 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #465      +/-   ##
==========================================
- Coverage   80.92%   80.36%   -0.57%     
==========================================
  Files         189      190       +1     
  Lines       10149    10260     +111     
  Branches     1495     1533      +38     
==========================================
+ Hits         8213     8245      +32     
- Misses       1721     1783      +62     
- Partials      215      232      +17     
Flag Coverage Δ
unittests 80.36% <60.52%> (-0.55%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmedit/models/backbones/__init__.py 100.00% <ø> (ø)
...medit/models/backbones/sr_backbones/basicvsr_pp.py 60.10% <60.10%> (ø)
mmedit/models/backbones/sr_backbones/__init__.py 100.00% <100.00%> (ø)
mmedit/models/losses/gan_loss.py 100.00% <100.00%> (ø)
mmedit/datasets/builder.py 89.28% <0.00%> (-3.58%) ⬇️
...ackbones/encoder_decoders/pconv_encoder_decoder.py 43.47% <0.00%> (-2.36%) ⬇️
mmedit/models/base.py 82.75% <0.00%> (-2.09%) ⬇️
...ones/encoder_decoders/two_stage_encoder_decoder.py 53.84% <0.00%> (-1.16%) ⬇️
...dels/backbones/encoder_decoders/encoders/resnet.py 66.01% <0.00%> (-0.97%) ⬇️
...s/backbones/encoder_decoders/gl_encoder_decoder.py 81.48% <0.00%> (-0.67%) ⬇️
... and 25 more

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 79e7dea...74f81e0. Read the comment docs.

@innerlee innerlee merged commit f9c9a95 into open-mmlab:master Aug 6, 2021
@innerlee
Copy link
Contributor

innerlee commented Aug 6, 2021

yeah your right

Yshuo-Li pushed a commit to Yshuo-Li/mmediting that referenced this pull request Jul 15, 2022
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.

2 participants