[fix] Dropout2d-3d no-batch-dim by kshitij12345 · Pull Request #69885 · pytorch/pytorch

kshitij12345 · 2021-12-14T06:36:45Z

TODO:

Update C++ API

cc @albanD @mruberry @jbschlosser @walterddr @kshitij12345

pytorch-probot · 2021-12-14T06:36:48Z

CI Flow Status

⚛️ CI Flow

Ruleset - Version: v1
Ruleset - File: https://github.com/kshitij12345/pytorch/blob/1aad29b84da3f835206aed75da9125fc22e20846/.github/generated-ciflow-ruleset.json
PR ciflow labels: ciflow/default

Workflows	Labels (bold enabled)	Status
Triggered Workflows
linux-bionic-py3.6-clang9	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/noarch`, `ciflow/xla`	✅ triggered
linux-docs	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/docs`, `ciflow/linux`	✅ triggered
linux-vulkan-bionic-py3.6-clang9	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/vulkan`	✅ triggered
linux-xenial-cuda11.3-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/default`, `ciflow/linux`	✅ triggered
linux-xenial-py3-clang5-mobile-build	`ciflow/all`, `ciflow/default`, `ciflow/linux`, `ciflow/mobile`	✅ triggered
linux-xenial-py3-clang5-mobile-custom-build-static	`ciflow/all`, `ciflow/default`, `ciflow/linux`, `ciflow/mobile`	✅ triggered
linux-xenial-py3.6-clang7-asan	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/sanitizers`	✅ triggered
linux-xenial-py3.6-clang7-onnx	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/onnx`	✅ triggered
linux-xenial-py3.6-gcc5.4	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`	✅ triggered
linux-xenial-py3.6-gcc7	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`	✅ triggered
linux-xenial-py3.6-gcc7-bazel-test	`ciflow/all`, `ciflow/bazel`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`	✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single	`ciflow/all`, `ciflow/android`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`	✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single-full-jit	`ciflow/all`, `ciflow/android`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`	✅ triggered
win-vs2019-cpu-py3	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/win`	✅ triggered
win-vs2019-cuda11.3-py3	`ciflow/all`, `ciflow/cuda`, `ciflow/default`, `ciflow/win`	✅ triggered
Skipped Workflows
caffe2-linux-xenial-py3.6-gcc5.4	`ciflow/all`, `ciflow/cpu`, `ciflow/linux`	🚫 skipped
docker-builds	`ciflow/all`	🚫 skipped
ios-12-5-1-arm64	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-arm64-coreml	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-arm64-custom-ops	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-arm64-full-jit	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-arm64-metal	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-x86-64	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-x86-64-coreml	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-x86-64-full-jit	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
libtorch-linux-xenial-cuda10.2-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`	🚫 skipped
libtorch-linux-xenial-cuda11.3-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`	🚫 skipped
linux-bionic-cuda10.2-py3.9-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/slow`	🚫 skipped
linux-docs-push	`ciflow/all`, `ciflow/cpu`, `ciflow/linux`, `ciflow/scheduled`	🚫 skipped
macos-10-15-py3-arm64	`ciflow/all`, `ciflow/macos`	🚫 skipped
macos-10-15-py3-lite-interpreter-x86-64	`ciflow/all`, `ciflow/macos`	🚫 skipped
macos-11-py3-x86-64	`ciflow/all`, `ciflow/macos`	🚫 skipped
parallelnative-linux-xenial-py3.6-gcc5.4	`ciflow/all`, `ciflow/cpu`, `ciflow/linux`	🚫 skipped
periodic-libtorch-linux-bionic-cuda11.5-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`, `ciflow/scheduled`	🚫 skipped
periodic-libtorch-linux-xenial-cuda11.1-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`, `ciflow/scheduled`	🚫 skipped
periodic-linux-bionic-cuda11.5-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/scheduled`	🚫 skipped
periodic-linux-xenial-cuda10.2-py3-gcc7-slow-gradcheck	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/scheduled`, `ciflow/slow`, `ciflow/slow-gradcheck`	🚫 skipped
periodic-linux-xenial-cuda11.1-py3.6-gcc7-debug	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/scheduled`	🚫 skipped
periodic-win-vs2019-cuda11.1-py3	`ciflow/all`, `ciflow/cuda`, `ciflow/scheduled`, `ciflow/win`	🚫 skipped
periodic-win-vs2019-cuda11.5-py3	`ciflow/all`, `ciflow/cuda`, `ciflow/scheduled`, `ciflow/win`	🚫 skipped

You can add a comment to the PR and tag @pytorchbot with the following commands:

# ciflow rerun, "ciflow/default" will always be added automatically
@pytorchbot ciflow rerun

# ciflow rerun with additional labels "-l <ciflow/label_name>", which is equivalent to adding these labels manually and trigger the rerun
@pytorchbot ciflow rerun -l ciflow/scheduled -l ciflow/slow

For more information, please take a look at the CI Flow Wiki.

jbschlosser

Thanks for the fix!

FYI I think the failing checks are happening because the OpInfo for nn.functional.dropout2d is wrong - looks like it's generating inputs with dim in range [2, 4] instead of [3, 4].

kshitij12345 · 2021-12-15T05:41:23Z

Looks like JIT tests are also failing. Will have a look.

facebook-github-bot · 2021-12-15T06:31:46Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/69885
📄 Preview docs built from this PR
📄 Preview C++ docs built from this PR
🔧 Opt-in to CIFlow to control what jobs run on your PRs

💊 CI failures summary and remediations

As of commit 546af0d (more details on the Dr. CI page):

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

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

kshitij12345 · 2021-12-15T09:16:04Z

@jbschlosser I couldn't find a test for Dropout2d and Dropout3d where we check that channel is being zeroed, not sure as to how we can test that.

jbschlosser · 2021-12-15T19:47:15Z

@jbschlosser I couldn't find a test for Dropout2d and Dropout3d where we check that channel is being zeroed, not sure as to how we can test that.

One option is to input a tensor that is known to be fully non-zero and check that the output only has entire channels zeroed out (i.e. anywhere a zero is present, the entire channel should also be zero).

jbschlosser · 2021-12-15T22:53:47Z

+        for b, c in product(range(B), range(C)):
+            self.assertTrue(result[b, c].count_nonzero() in (0, channel_numel))
+
+    @expectedFailureXLA  # seems like freeze_rng_state is not honoured by XLA


kshitij12345 · 2021-12-16T08:10:09Z

        supports_out=False,
-        sample_inputs_func=partial(sample_inputs_dropout, train=True, min_input_dim=2),
+        # As per the docs, valid input dims are (4, 5)
+        sample_inputs_func=partial(sample_inputs_dropout, train=True, valid_input_dim=(4, 5)),


This is OpInfo entry for feature_alpha_dropout.

This is good for the purposes of this PR, but eventually FeatureAlphaDropout should be documented and tested to work on dims besides 4 and 5 (see #69960 (comment))

kshitij12345 · 2021-12-16T08:10:54Z

        supports_out=False,
-        sample_inputs_func=partial(sample_inputs_dropout, min_input_dim=2),
+        # As per the docs, valid input dims are (3, 4)
+        sample_inputs_func=partial(sample_inputs_dropout, valid_input_dim=(3, 4)),


This is OpInfo entry for dropout2d.

facebook-github-bot · 2022-01-24T19:51:27Z

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

jbschlosser · 2022-01-25T15:58:14Z

Hey @kshitij12345, FYI there's a whole bunch of internal breakage from this change. It seems there are people using dropout2d with 2D inputs expecting normal dropout to occur, which does happen to be the case. There may also be people passing 3D inputs expecting something, and redefining 3D to mean no-batch-dim input would silently break them, which is worse than loudly breaking them. So I propose a tweak to this PR to handle this case:

Start by restricting input to 4D (N, C, H, W) for Dropout2d and throwing an error for other input dims. Change the docs back to indicate that this is the only supported shape. Do this for a full release to help remove invalid usages of 3D inputs with dropout2d.
After a release cycle, we can add no-batch-dim support by allowing 3D. This should help avoid the silent breakage.

(do the analogous thing for Dropout3d / 5D input as well)

wdyt about this?

kshitij12345 · 2022-01-26T17:33:17Z

Wouldn't this be weird? The support for no-batch-dim for these two modules was added in 1.10. We remove it in 1.11 and bring it back with new semantics in 1.12?

https://pytorch.org/docs/1.10.0/generated/torch.nn.Dropout2d.html?highlight=dropout2d#torch.nn.Dropout2d
https://pytorch.org/docs/1.10.0/generated/torch.nn.Dropout3d.html?highlight=dropout3d#torch.nn.Dropout3d

kshitij12345 · 2022-01-31T16:03:11Z

@jbschlosser as discussed offline, I have updated the PR to be less strict.

jbschlosser

Thanks for the update! Minor comments below for warning wording to make deprecations actionable

jbschlosser · 2022-01-31T16:09:06Z

+        warn_msg = ("dropout3d: 3-D input is being deprecated for Dropout3d."
+                    " Look at the docs for correct input shapes and usage.")


nit: deprecation should be actionable to remove the warning - in this case, switching to F.dropout is the way to go:

Received a 3D input to dropout3d, which is deprecated and will result in an error in a future release. To retain the behavior and silence this warning, please use dropout instead. Note that dropout3d exists to provide channel-wise dropout on inputs with 3 spatial dimensions, a channel dimension, and an optional batch dimension (i.e. 4D or 5D inputs).

jbschlosser · 2022-01-31T16:09:45Z

+        warn_msg = ("dropout2d: 2-D input is being deprecated for Dropout2d."
+                    " Look at the docs for correct input shapes and usage.")


Wording suggestion as below:

Received a 2D input to dropout2d, which is deprecated and will result in an error in a future release. To retain the behavior and silence this warning, please use dropout instead. Note that dropout2d exists to provide channel-wise dropout on inputs with 2 spatial dimensions, a channel dimension, and an optional batch dimension (i.e. 3D or 4D inputs).

facebook-github-bot · 2022-01-31T17:00:20Z

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

jbschlosser

Thanks for the update! One minor thing and we can get this in :)

jbschlosser · 2022-02-01T17:30:12Z

-    assert inp_dim in (2, 3, 4), msg
-    if inp_dim == 2:
+    if inp_dim not in (3, 4):
        warn_msg = ("dropout2d: Received a 2D input to dropout2d, which is deprecated "


nit: warning msg should correctly specify the dimension of the input now; here it's hardcoded to 2D

Sorry. Thanks :)

jbschlosser · 2022-02-01T17:30:42Z

-    assert inp_dim in (3, 4, 5), msg
-    if inp_dim == 3:
+    if inp_dim not in (4, 5):
        warn_msg = ("dropout3d: Received a 3D input to dropout3d, which is deprecated "


facebook-github-bot · 2022-02-01T18:41:53Z

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

Summary: Fixes #69801 TODO: * [x] Update C++ API cc albanD mruberry jbschlosser walterddr kshitij12345 Pull Request resolved: #69885 Reviewed By: mruberry Differential Revision: D33175470 Pulled By: jbschlosser fbshipit-source-id: c9d7d9e0f59ba290a0157725c338a345f3d58b9f

github-actions · 2022-02-02T16:41:08Z

Hey kshitij12345.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

Summary: Fixes pytorch/pytorch#69801 TODO: * [x] Update C++ API cc albanD mruberry jbschlosser walterddr kshitij12345 Pull Request resolved: pytorch/pytorch#69885 Reviewed By: mruberry Differential Revision: D33175470 Pulled By: jbschlosser fbshipit-source-id: c9d7d9e0f59ba290a0157725c338a345f3d58b9f (cherry picked from commit 7e4271a)

Summary: Fixes pytorch#69801 TODO: * [x] Update C++ API cc albanD mruberry jbschlosser walterddr kshitij12345 Pull Request resolved: pytorch#69885 Reviewed By: mruberry Differential Revision: D33175470 Pulled By: jbschlosser fbshipit-source-id: c9d7d9e0f59ba290a0157725c338a345f3d58b9f (cherry picked from commit 7e4271a)

[fix] Dropout2d-3d no-batch-dim

a15f851

pytorch-probot Bot added the ciflow/default label Dec 14, 2021

kshitij12345 added the module: nn Related to torch.nn label Dec 14, 2021

jbschlosser reviewed Dec 14, 2021

View reviewed changes

Comment thread test/test_nn.py Outdated

facebook-github-bot added the cla signed label Dec 14, 2021

pytorchbot added the open source label Dec 14, 2021

kshitij12345 added 3 commits December 15, 2021 04:07

update samples for opinfo

cf2378e

remove unnecessary comment

83f434f

update also for feature_alpha_dropout

4fde8f0

update samples for jit tests

f1d0415

mark tests failing on XLA with expectedFailureXLA

4426c4a

handle inplace case

3a24729

kshitij12345 added 3 commits December 15, 2021 20:16

Merge branch 'master' into fix/dropout-2d-3d/no-batch-dim

fd9ffa5

add test for correctness

e2a7069

update comment

958e30e

jbschlosser reviewed Dec 15, 2021

View reviewed changes

update c++ api and tests

f77c95c

kshitij12345 commented Dec 16, 2021

View reviewed changes

update c++ tests

2db71e1

kshitij12345 commented Dec 16, 2021

View reviewed changes

Comment thread test/cpp/api/functional.cpp

increase the size of input in c++ test

1aad29b

kshitij12345 marked this pull request as ready for review December 16, 2021 14:21

kshitij12345 requested a review from albanD as a code owner December 16, 2021 14:21

albanD removed their request for review December 16, 2021 14:22

kshitij12345 added 3 commits January 31, 2022 15:41

Merge branch 'master' into fix/dropout-2d-3d/no-batch-dim

391a922

warn for invalid usage

dab0f99

add test for warning

efb4e44

jbschlosser reviewed Jan 31, 2022

View reviewed changes

kshitij12345 added 3 commits January 31, 2022 16:49

fix trailing spaces

2072066

update warning

816b2a4

update test msg

ad57742

jbschlosser reviewed Jan 31, 2022

View reviewed changes

Comment thread torch/nn/functional.py Outdated

allow all shapes

862556f

jbschlosser reviewed Feb 1, 2022

View reviewed changes

address review: mention inp_dim in warning

546af0d

pytorchmergebot closed this in 02f6226 Feb 2, 2022

jbschlosser added the topic: bug fixes topic category label Feb 2, 2022

zou3519 mentioned this pull request Jun 13, 2022

Did Dropout2d change for Pytorch 1.11? #77081

Closed

jbschlosser mentioned this pull request Jun 15, 2022

Revert behavior of Dropout2d on 3D inputs to 1D channel-wise dropout behavior & warn #79549

Closed

		warn_msg = ("dropout3d: 3-D input is being deprecated for Dropout3d."
		" Look at the docs for correct input shapes and usage.")

		warn_msg = ("dropout2d: 2-D input is being deprecated for Dropout2d."
		" Look at the docs for correct input shapes and usage.")

Conversation

kshitij12345 commented Dec 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-probot Bot commented Dec 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚛️ CI Flow

Uh oh!

jbschlosser left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kshitij12345 commented Dec 15, 2021

Uh oh!

facebook-github-bot commented Dec 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

💊 CI failures summary and remediations

Uh oh!

kshitij12345 commented Dec 15, 2021

Uh oh!

jbschlosser commented Dec 15, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-bot commented Jan 24, 2022

Uh oh!

jbschlosser commented Jan 25, 2022

Uh oh!

kshitij12345 commented Jan 26, 2022

Uh oh!

kshitij12345 commented Jan 31, 2022

Uh oh!

jbschlosser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jan 31, 2022

Uh oh!

Uh oh!

jbschlosser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Feb 1, 2022

Uh oh!

github-actions Bot commented Feb 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kshitij12345 commented Dec 14, 2021 •

edited

Loading

pytorch-probot Bot commented Dec 14, 2021 •

edited

Loading

jbschlosser left a comment •

edited

Loading

facebook-github-bot commented Dec 15, 2021 •

edited

Loading