fix: atrous_rates for deeplabv3_mobilenet_v3_large (fixes #7956) #8019

nvs-abhilash · 2023-10-05T03:32:49Z

This PR addresses the issue reffered in #7956

The PR proposes the following changes to address the issues:

Parameterize atrous_rates for deeplabv3 in DeepLabHead.
~~Send (6, 12, 18) atrous_rates for mobilenet backbone~~

pytorch-bot · 2023-10-05T03:32:51Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8019

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure, 14 Unrelated Failures

As of commit c65397d with merge base 7e2050f ():

NEW FAILURE - The following job has failed:

Tests / unittests-macos (3.11, macos-12) / macos-job (gh)

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

facebook-github-bot · 2023-10-05T03:32:54Z

Hi @nvs-abhilash!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

facebook-github-bot · 2023-10-05T04:58:19Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

NicolasHug

Thanks for the PR @nvs-abhilash .

I made a few comments below regarding preserving backward compatibility.

@fmassa , do you remember why the values [12, 24, 36] were used for the atrous rates in #820, instead of [6, 12, 18]?

NicolasHug · 2023-10-09T10:05:53Z

torchvision/models/segmentation/deeplabv3.py

@@ -220,7 +220,7 @@ def _deeplabv3_mobilenetv3(
    backbone = IntermediateLayerGetter(backbone, return_layers=return_layers)

    aux_classifier = FCNHead(aux_inplanes, num_classes) if aux else None
-    classifier = DeepLabHead(out_inplanes, num_classes)
+    classifier = DeepLabHead(out_inplanes, num_classes, atrous_rates=(6, 12, 18))


Unfortunately this would be BC-breaking. We'll have to preserve the values that are currently used here (relying on the default).

Suggested change

classifier = DeepLabHead(out_inplanes, num_classes, atrous_rates=(6, 12, 18))

classifier = DeepLabHead(out_inplanes, num_classes)

NicolasHug · 2023-10-09T10:07:58Z

torchvision/models/segmentation/deeplabv3.py

@@ -83,7 +83,7 @@ def forward(self, x: torch.Tensor) -> torch.Tensor:


 class ASPP(nn.Module):
-    def __init__(self, in_channels: int, atrous_rates: List[int], out_channels: int = 256) -> None:
+    def __init__(self, in_channels: int, atrous_rates: Tuple[int, int, int], out_channels: int = 256) -> None:


From my understanding, using List[int] instead of a fixed 3-tuple was intended (details in #2136).

Suggested change

def __init__(self, in_channels: int, atrous_rates: Tuple[int, int, int], out_channels: int = 256) -> None:

def __init__(self, in_channels: int, atrous_rates: List[int], out_channels: int = 256) -> None:

NicolasHug · 2023-10-09T10:08:21Z

torchvision/models/segmentation/deeplabv3.py

@@ -46,9 +46,9 @@ class DeepLabV3(_SimpleSegmentationModel):


 class DeepLabHead(nn.Sequential):
-    def __init__(self, in_channels: int, num_classes: int) -> None:
+    def __init__(self, in_channels: int, num_classes: int, atrous_rates: Tuple[int, int, int] = (12, 24, 36)) -> None:


Suggested change

def __init__(self, in_channels: int, num_classes: int, atrous_rates: Tuple[int, int, int] = (12, 24, 36)) -> None:

def __init__(self, in_channels: int, num_classes: int, atrous_rates: List[int] = [12, 24, 36]) -> None:

fmassa · 2023-10-09T11:32:30Z

Hi,

Thanks for the PR!

The dilation rates could have been an oversight when porting the DeepLab head into MobileNetV3.
That being said, using a larger dilation factor in the backbone (as done in the ResNet variants) makes the model much slower to run, so maybe the configuration done here (with the larger dilation in the head) actually yields better accuracies for a smaller computation budget (but this would need to be verified).

Changing the defaults will change the outputs produced by the models, so I agree with @NicolasHug to keep the same defaults as before.

nvs-abhilash · 2023-10-09T20:24:55Z

Thanks, @NicolasHug and @fmassa for reviewing my code.

Post your comments, the only change that seems to make sense is just to parameterise the atrous_rates parameter. I've updated the commit incorporating the same.

Also, what can be the path to solve #7956 ?

NicolasHug

Thanks @nvs-abhilash , LGTM with one last comment below

NicolasHug · 2023-10-10T08:03:45Z

torchvision/models/segmentation/deeplabv3.py

@@ -46,9 +46,9 @@ class DeepLabV3(_SimpleSegmentationModel):


 class DeepLabHead(nn.Sequential):
-    def __init__(self, in_channels: int, num_classes: int) -> None:
+    def __init__(self, in_channels: int, num_classes: int, atrous_rates: List[int] = [12, 24, 36]) -> None:


We should avoid setting mutables as default values in general as there's a risk that we will change that value inplace, and subsequent calls would be using the wrong default.

Suggested change

def __init__(self, in_channels: int, num_classes: int, atrous_rates: List[int] = [12, 24, 36]) -> None:

def __init__(self, in_channels: int, num_classes: int, atrous_rates: Sequence[int] = (12, 24, 36)) -> None:

Yeah, makes sense. I thought the same, therefore had made it tuple before. 😅

Updated the PR.

NicolasHug · 2023-10-10T08:05:35Z

Also, what can be the path to solve #7956 ?

Would documenting the atrous rates being used be enough?

NicolasHug

Thanks a lot @nvs-abhilash

github-actions · 2023-10-11T10:49:13Z

Hey @NicolasHug!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

) (#8019) Summary: Co-authored-by: Nicolas Hug <nh.nicolas.hug@gmail.com> Reviewed By: vmoens Differential Revision: D50789082 fbshipit-source-id: 02bbe21ba09522b2db1552c6a677b89ff4247392

facebook-github-bot added the cla signed label Oct 5, 2023

nvs-abhilash mentioned this pull request Oct 7, 2023

Misconfiguration of MobileNetV3-Large backbone for DeepLabV3 #7956

Closed

NicolasHug reviewed Oct 9, 2023

View reviewed changes

nvs-abhilash force-pushed the mobilenet-fix branch from 73ef094 to 17b530d Compare October 9, 2023 20:19

nvs-abhilash requested a review from NicolasHug October 9, 2023 20:27

NicolasHug reviewed Oct 10, 2023

View reviewed changes

feat: parameterize atrous_rates for deeplabv3 in DeepLabHead.

67dce75

nvs-abhilash force-pushed the mobilenet-fix branch from 17b530d to 67dce75 Compare October 10, 2023 16:20

nvs-abhilash requested a review from NicolasHug October 10, 2023 16:21

NicolasHug approved these changes Oct 11, 2023

View reviewed changes

Merge branch 'main' into mobilenet-fix

c65397d

NicolasHug merged commit 70a8e05 into pytorch:main Oct 11, 2023
34 of 49 checks passed

NicolasHug added enhancement module: models labels Oct 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: atrous_rates for deeplabv3_mobilenet_v3_large (fixes #7956) #8019

fix: atrous_rates for deeplabv3_mobilenet_v3_large (fixes #7956) #8019

nvs-abhilash commented Oct 5, 2023 •

edited

Loading

pytorch-bot bot commented Oct 5, 2023 •

edited

Loading

facebook-github-bot commented Oct 5, 2023

facebook-github-bot commented Oct 5, 2023

NicolasHug left a comment

NicolasHug Oct 9, 2023

NicolasHug Oct 9, 2023 •

edited

Loading

NicolasHug Oct 9, 2023

fmassa commented Oct 9, 2023

nvs-abhilash commented Oct 9, 2023

NicolasHug left a comment

NicolasHug Oct 10, 2023

nvs-abhilash Oct 10, 2023

nvs-abhilash Oct 10, 2023

NicolasHug commented Oct 10, 2023

NicolasHug left a comment

github-actions bot commented Oct 11, 2023

	classifier = DeepLabHead(out_inplanes, num_classes, atrous_rates=(6, 12, 18))
	classifier = DeepLabHead(out_inplanes, num_classes)

	def __init__(self, in_channels: int, atrous_rates: Tuple[int, int, int], out_channels: int = 256) -> None:
	def __init__(self, in_channels: int, atrous_rates: List[int], out_channels: int = 256) -> None:

	def __init__(self, in_channels: int, num_classes: int, atrous_rates: Tuple[int, int, int] = (12, 24, 36)) -> None:
	def __init__(self, in_channels: int, num_classes: int, atrous_rates: List[int] = [12, 24, 36]) -> None:

fix: atrous_rates for deeplabv3_mobilenet_v3_large (fixes #7956) #8019

fix: atrous_rates for deeplabv3_mobilenet_v3_large (fixes #7956) #8019

Conversation

nvs-abhilash commented Oct 5, 2023 • edited Loading

pytorch-bot bot commented Oct 5, 2023 • edited Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8019

❌ 1 New Failure, 14 Unrelated Failures

facebook-github-bot commented Oct 5, 2023

Action Required

Process

facebook-github-bot commented Oct 5, 2023

NicolasHug left a comment

Choose a reason for hiding this comment

NicolasHug Oct 9, 2023

Choose a reason for hiding this comment

NicolasHug Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

NicolasHug Oct 9, 2023

Choose a reason for hiding this comment

fmassa commented Oct 9, 2023

nvs-abhilash commented Oct 9, 2023

NicolasHug left a comment

Choose a reason for hiding this comment

NicolasHug Oct 10, 2023

Choose a reason for hiding this comment

nvs-abhilash Oct 10, 2023

Choose a reason for hiding this comment

nvs-abhilash Oct 10, 2023

Choose a reason for hiding this comment

NicolasHug commented Oct 10, 2023

NicolasHug left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 11, 2023

nvs-abhilash commented Oct 5, 2023 •

edited

Loading

pytorch-bot bot commented Oct 5, 2023 •

edited

Loading

NicolasHug Oct 9, 2023 •

edited

Loading