[Fix] Replace interpolate with resize #731

mmeendez8 · 2021-07-27T09:54:36Z

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

I am having problems to export mmsegmentation -> onnx -> tensorflow. There is a know limitation related with interpolate (see here). I tried to fix mmseg.ops.resize behavior to always use align_corners but I realize some modules are still using torch F.interpolate.
`

Modification

Use mmseg resize wrapper for all operations.

BC-breaking (Optional)

Does the modification introduce changes that break the backward-compatibility of the downstream repos?
If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.

Use cases (Optional)

If this PR introduces a new feature, it is better to list some use cases here, and update the documentation.

Checklist

Pre-commit or other linting tools are used to fix the potential lint issues.
The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
If the modification has potential influence on downstream projects, this PR should be tested with downstream projects, like MMDet or MMDet3D.
The documentation has been modified accordingly, like docstring or example tutorials.

CLAassistant · 2021-07-27T09:54:40Z

All committers have signed the CLA.

mmeendez8 · 2021-07-27T10:28:01Z

Just realized my problem was also related with FPN Upsample:

https://github.com/open-mmlab/mmsegmentation/blob/master/mmseg/models/decode_heads/fpn_head.py#L48

I wonder if we should replace this calls with upsample wrapper from ops:
https://github.com/open-mmlab/mmsegmentation/blob/master/mmseg/ops/wrappers.py#L29

codecov · 2021-07-27T10:52:40Z

Codecov Report

Merging #731 (5d7f09b) into master (6526593) will decrease coverage by 0.02%.
The diff coverage is 85.71%.

❗ Current head 5d7f09b differs from pull request most recent head acd5ee0. Consider uploading reports for the commit acd5ee0 to get more accurate results

@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
- Coverage   85.28%   85.26%   -0.03%     
==========================================
  Files         107      107              
  Lines        5817     5822       +5     
  Branches      952      952              
==========================================
+ Hits         4961     4964       +3     
- Misses        673      674       +1     
- Partials      183      184       +1

Flag	Coverage Δ
unittests	`85.26% <85.71%> (-0.01%)`	⬇️

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

Impacted Files	Coverage Δ
mmseg/models/backbones/swin.py	`86.94% <50.00%> (+0.04%)`	⬆️
mmseg/models/necks/fpn.py	`54.54% <66.66%> (+0.59%)`	⬆️
mmseg/models/backbones/unet.py	`94.91% <100.00%> (+0.04%)`	⬆️
mmseg/models/backbones/vit.py	`84.84% <100.00%> (ø)`
mmseg/models/decode_heads/fpn_head.py	`100.00% <100.00%> (ø)`
mmseg/models/decode_heads/setr_mla_head.py	`100.00% <100.00%> (ø)`
mmseg/models/decode_heads/setr_up_head.py	`100.00% <100.00%> (ø)`
mmseg/models/necks/multilevel_neck.py	`100.00% <100.00%> (ø)`
mmseg/datasets/builder.py	`86.84% <0.00%> (-2.64%)`	⬇️

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 6526593...acd5ee0. Read the comment docs.

Junjun2016 · 2021-07-27T10:53:03Z

Hi @mmeendez8
Thanks for your nice PR!

I wonder if we should replace this calls with upsample wrapper from ops:
https://github.com/open-mmlab/mmsegmentation/blob/master/mmseg/ops/wrappers.py#L29

Yes, we can replace it with upsample wrapper.
Update it, please!

Junjun2016 · 2021-07-27T11:00:36Z

Hi @grimoire and @AllentDan

I am having problems to export mmsegmentation -> onnx -> tensorflow. There is a know limitation related with interpolate (see here). I tried to fix mmseg.ops.resize behavior to always use align_corners but I realize some modules are still using torch F.interpolate.

mmeendez8 · 2021-07-27T11:05:38Z

Hi @grimoire and @AllentDan

I am having problems to export mmsegmentation -> onnx -> tensorflow. There is a know limitation related with interpolate (see here). I tried to fix mmseg.ops.resize behavior to always use align_corners but I realize some modules are still using torch F.interpolate.

Sorry, forgot to say that problem has been fixed. Tensorflow does not support torch.interpolate or torch.upsample when align_corners=False. So I basically fixed mmseg.ops.resizebehavior to always use align_corners=True, that's when I realize there was mixed calls to pytorch and mmseg.ops.

Junjun2016 · 2021-07-27T11:09:59Z

Hi @grimoire and @AllentDan

I am having problems to export mmsegmentation -> onnx -> tensorflow. There is a know limitation related with interpolate (see here). I tried to fix mmseg.ops.resize behavior to always use align_corners but I realize some modules are still using torch F.interpolate.

Sorry, forgot to say that problem has been fixed. Tensorflow does not support torch.interpolate or torch.upsample when align_corners=False. So I basically fixed mmseg.ops.resizebehavior to always use align_corners, that's when I realize there was mixed calls to pytorch and mmseg.ops.

I see, thanks again!

Junjun2016 · 2021-07-27T11:13:23Z

@mmeendez8

fixed mmseg.ops.resize behavior to always use align_corners

I am confused about this since he defaults args of resize are the same as F.interpolate.
The default setting of align_corners is None.

Junjun2016 · 2021-07-27T11:15:16Z

Hi @mmeendez8
Please fix the CI.

Junjun2016 · 2021-07-27T11:19:37Z

Please fix the unittest of test_interp_conv in test_unet.py.

mmeendez8 · 2021-07-27T11:22:15Z

Hi @mmeendez8
Please fix the CI.

Yes, sorry. I had already pushed fix

Junjun2016 · 2021-07-27T11:23:15Z

@mmeendez8

fixed mmseg.ops.resize behavior to always use align_corners

I am confused about this since he defaults args of resize are the same as F.interpolate.
The default setting of align_corners is None.

Could you please explain it?

Junjun2016 · 2021-07-27T11:23:47Z

Hi @mmeendez8
Please fix the CI.

Yes, sorry. I had already pushed fix

Thanks.

Junjun2016 · 2021-07-27T11:25:02Z

@mmeendez8

fixed mmseg.ops.resize behavior to always use align_corners

I am confused about this since he defaults args of resize are the same as F.interpolate.
The default setting of align_corners is None.

Do you mean that set align_corners to True or False instead of None?

mmeendez8 · 2021-07-27T11:29:42Z

@mmeendez8

fixed mmseg.ops.resize behavior to always use align_corners

I am confused about this since he defaults args of resize are the same as F.interpolate.
The default setting of align_corners is None.

You mean set align_corners to True or False instead of None?

I meant that I brute force align_corners=True. But I realized that this was not working properly, that's how I discover that nn.interpolate was being called in multiple places. Hope this can clarify it!

Junjun2016 · 2021-07-27T11:39:11Z

@mmeendez8

fixed mmseg.ops.resize behavior to always use align_corners

I am confused about this since he defaults args of resize are the same as F.interpolate.
The default setting of align_corners is None.

You mean set align_corners to True or False instead of None?

I meant that I brute force align_corners=True. But I realized that this was not working properly, that's how I discover that nn.interpolate was being called in multiple places. Hope this can clarify it!

Ok, I see, thanks!

Junjun2016

LGTM

* Replace interpolate with resize * Replace nn.Upsample with ops.Upsample * Fix test

…open-mmlab#731) This is to ensure that the final latent slices stay somewhat consistent as more changes are introduced into the library. Signed-off-by: James R T <jamestiotio@gmail.com> Signed-off-by: James R T <jamestiotio@gmail.com>

* Add back "registry.py" files with deprecation warnings. * add unittest for backward compatibility

* resolve comments * update changelog * add Kinetics CN README * add Kinetics CN README * move file * refine

Replace interpolate with resize

0b50600

Junjun2016 requested review from xvjiarui and Junjun2016 July 27, 2021 10:53

Replace nn.Upsample with ops.Upsample

b636d1a

Fix test

acd5ee0

Junjun2016 mentioned this pull request Jul 27, 2021

[Feature] add SFSegNet head #733

Open

Junjun2016 changed the title ~~Replace interpolate with resize~~ [Fix] Replace interpolate with resize Jul 28, 2021

Junjun2016 approved these changes Jul 28, 2021

View reviewed changes

Junjun2016 merged commit f800669 into open-mmlab:master Jul 28, 2021

mmeendez8 deleted the replace-interpolate-with-ops-resize branch July 28, 2021 15:25

Junjun2016 mentioned this pull request Jul 30, 2021

[Feature] Add segformer decode head and related train config #599

Merged

2 tasks

bowenroom pushed a commit to bowenroom/mmsegmentation that referenced this pull request Feb 25, 2022

[Fix] Replace interpolate with resize (open-mmlab#731)

50461ef

* Replace interpolate with resize * Replace nn.Upsample with ops.Upsample * Fix test

wjkim81 pushed a commit to wjkim81/mmsegmentation that referenced this pull request Dec 3, 2023

Add back "registry.py" files with deprecation warnings. (open-mmlab#731)

31d25a9

* Add back "registry.py" files with deprecation warnings. * add unittest for backward compatibility

sibozhang pushed a commit to sibozhang/mmsegmentation that referenced this pull request Mar 22, 2024

[Docs] Kinetics CN README (open-mmlab#731)

429f520

* resolve comments * update changelog * add Kinetics CN README * add Kinetics CN README * move file * refine

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] Replace interpolate with resize #731

[Fix] Replace interpolate with resize #731

mmeendez8 commented Jul 27, 2021

CLAassistant commented Jul 27, 2021 •

edited

mmeendez8 commented Jul 27, 2021 •

edited

codecov bot commented Jul 27, 2021 •

edited

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

mmeendez8 commented Jul 27, 2021 •

edited

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

mmeendez8 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021 •

edited

mmeendez8 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021 •

edited

Junjun2016 left a comment

[Fix] Replace interpolate with resize #731

[Fix] Replace interpolate with resize #731

Conversation

mmeendez8 commented Jul 27, 2021

Motivation

Modification

BC-breaking (Optional)

Use cases (Optional)

Checklist

CLAassistant commented Jul 27, 2021 • edited

mmeendez8 commented Jul 27, 2021 • edited

codecov bot commented Jul 27, 2021 • edited

Codecov Report

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

mmeendez8 commented Jul 27, 2021 • edited

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

mmeendez8 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021 • edited

mmeendez8 commented Jul 27, 2021

Junjun2016 commented Jul 27, 2021 • edited

Junjun2016 left a comment

Choose a reason for hiding this comment

CLAassistant commented Jul 27, 2021 •

edited

mmeendez8 commented Jul 27, 2021 •

edited

codecov bot commented Jul 27, 2021 •

edited

mmeendez8 commented Jul 27, 2021 •

edited

Junjun2016 commented Jul 27, 2021 •

edited

Junjun2016 commented Jul 27, 2021 •

edited