Factor backend routing logic out of convolution forward #67790

jbschlosser · 2021-11-03T21:57:37Z

This PR introduces a new function _select_conv_backend that returns a ConvBackend enum representing the selected backend for a given set of convolution inputs and params.

The function and enum are exposed to python for testing purposes through torch/csrc/Module.cpp (please let me know if there's a better place to do this).

A new set of tests validates that the correct backend is selected for several sets of inputs + params. Some backends aren't tested yet:

nnpack (for mobile)
xnnpack (for mobile)
winograd 3x3 (for mobile)

Some flowcharts for reference:

pytorch-probot · 2021-11-03T21:57:40Z

CI Flow Status

⚛️ CI Flow

Ruleset - Version: v1
Ruleset - File: https://github.com/jbschlosser/pytorch/blob/a8c7cc89775d87fe8823920045d7c29bf03f2a56/.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-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-dynamic	`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-xenial-py3-clang5-mobile-code-analysis	`ciflow/all`, `ciflow/linux`, `ciflow/mobile`	🚫 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-10-15-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-xenial-cuda11.1-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `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	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/scheduled`	🚫 skipped
periodic-win-vs2019-cuda11.1-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.

facebook-github-bot · 2021-11-03T21:57:43Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/67790
📄 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 a8c7cc8 (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.

jbschlosser · 2021-11-04T18:06:27Z

aten/src/ATen/native/ConvUtils.h

Moved here from Convolution.cpp

jbschlosser · 2021-11-04T18:07:25Z

aten/src/ATen/native/ConvUtils.h

There's two overloads right now, which isn't great. The one exposed to python purposefully matches the signature of convolution

jbschlosser · 2021-11-04T18:07:49Z

torch/csrc/Module.cpp

Exposes ConvBackend and _select_conv_backend to python

albanD

Sounds good. Only some comments and spurious public API.

aten/src/ATen/native/ConvUtils.h

test/test_nn.py

test/test_public_bindings.py

zou3519 · 2021-11-08T22:44:12Z

Some backends aren't tested yet:

miopen (for AMD ROCm)
nnpack (for mobile)
xnnpack (for mobile)
winograd 3x3 (for mobile)

I just started reading through this, but what is the plan for those? Is it fine to ignore them in this PR or do they need to be handled here?

zou3519 · 2021-11-08T22:48:33Z

aten/src/ATen/native/ConvUtils.h

nit: You don't need to fix this because this is a move, but this looks like a bug: groups seems like it should be int64_t?

Yeah I agree with you - seems like a bug. Think it's worth opening an issue for?

aten/src/ATen/native/ConvUtils.h

aten/src/ATen/native/Convolution.cpp

zou3519 · 2021-11-09T00:02:47Z

test/test_nn.py

There appears to be torch.backends.xnnpack.enabled, not sure if that helps...

True, looks like I can get a test in for that backend by disabling cuDNN / mkldnn and checking torch.backends.xnnpack.enabled. My plan was to punt on mobile testing for this PR but I'll open an issue suggesting this

zou3519

seems reasonable to me

albanD

SGTM only richard's nit

jbschlosser · 2021-11-09T14:50:10Z

Some backends aren't tested yet:
miopen (for AMD ROCm)
nnpack (for mobile)
xnnpack (for mobile)
winograd 3x3 (for mobile)

I just started reading through this, but what is the plan for those? Is it fine to ignore them in this PR or do they need to be handled here?

Current plan is to ignore them for this PR but add a note that they should be handled at some point. I was able to get miopen tested, so that just leaves the mobile backends. I can open a mobile issue indicating that they're not tested and leave it to those folks to deal with it.

facebook-github-bot · 2021-11-09T15:16:06Z

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

facebook-github-bot · 2021-11-09T16:14:37Z

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

facebook-github-bot · 2021-11-09T22:40:38Z

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

Summary: This PR introduces a new function `_select_conv_backend` that returns a `ConvBackend` enum representing the selected backend for a given set of convolution inputs and params. The function and enum are exposed to python for testing purposes through `torch/csrc/Module.cpp` (please let me know if there's a better place to do this). A new set of tests validates that the correct backend is selected for several sets of inputs + params. Some backends aren't tested yet: * nnpack (for mobile) * xnnpack (for mobile) * winograd 3x3 (for mobile) Some flowcharts for reference: ![conv_routing_graph md](https://user-images.githubusercontent.com/75754324/140828957-1135b400-38c0-4c9f-87ef-4f33ceebeeae.png) ![conv_nogroup_routing_graph md](https://user-images.githubusercontent.com/75754324/140828977-ed223a4e-aa86-49f1-9925-c0f6b9ab36af.png) Pull Request resolved: pytorch#67790 Reviewed By: zou3519 Differential Revision: D32280878 Pulled By: jbschlosser fbshipit-source-id: 4b9946f09411993b1c1d2f6bca8b485be823aebe

facebook-github-bot · 2021-11-10T15:48:18Z

This pull request was exported from Phabricator. Differential Revision: D32280878

facebook-github-bot · 2021-11-10T15:55:49Z

@jbschlosser merged this pull request in 9a2db6f.

… linalg functions on GPU (#67980) Summary: Per title. This PR introduces a global flag that lets pytorch prefer one of the many backend implementations while calling linear algebra functions on GPU. Usage: ```python torch.backends.cuda.preferred_linalg_library('cusolver') ``` Available options (str): `'default'`, `'cusolver'`, `'magma'`. Issue #63992 inspired me to write this PR. No heuristic is perfect on all devices, library versions, matrix shapes, workloads, etc. We can obtain better performance if we can conveniently switch linear algebra backends at runtime. Performance of linear algebra operators after this PR should be no worse than before. The flag is set to **`'default'`** by default, which makes everything the same as before this PR. The implementation of this PR is basically following that of #67790. Pull Request resolved: #67980 Reviewed By: mruberry Differential Revision: D32849457 Pulled By: ngimel fbshipit-source-id: 679fee7744a03af057995aef06316306073010a6

pytorch-probot bot added the ciflow/default label Nov 3, 2021

facebook-github-bot added the cla signed label Nov 3, 2021

jbschlosser commented Nov 4, 2021

View reviewed changes

jbschlosser requested review from albanD and zou3519 November 4, 2021 18:11

jbschlosser force-pushed the conv_forward_refactor branch from 06571a1 to 6c777d6 Compare November 4, 2021 20:22

albanD reviewed Nov 4, 2021

View reviewed changes

jbschlosser force-pushed the conv_forward_refactor branch from 5dd4a97 to e53eff3 Compare November 8, 2021 22:33

jbschlosser marked this pull request as ready for review November 8, 2021 22:33

jbschlosser changed the title ~~[WIP] Factor backend routing logic out of convolution forward~~ Factor backend routing logic out of convolution forward Nov 8, 2021

jbschlosser requested a review from albanD November 8, 2021 22:43

zou3519 reviewed Nov 8, 2021

View reviewed changes

aten/src/ATen/native/ConvUtils.h Outdated Show resolved Hide resolved

zou3519 reviewed Nov 8, 2021

View reviewed changes

aten/src/ATen/native/Convolution.cpp Outdated Show resolved Hide resolved

zou3519 reviewed Nov 8, 2021

View reviewed changes

aten/src/ATen/native/Convolution.cpp Outdated Show resolved Hide resolved

zou3519 reviewed Nov 9, 2021

View reviewed changes

zou3519 approved these changes Nov 9, 2021

View reviewed changes

albanD approved these changes Nov 9, 2021

View reviewed changes

jbschlosser force-pushed the conv_forward_refactor branch from 61973c8 to c4a9816 Compare November 9, 2021 22:40

jbschlosser force-pushed the conv_forward_refactor branch from c4a9816 to a8c7cc8 Compare November 10, 2021 15:48

facebook-github-bot closed this in 9a2db6f Nov 10, 2021

facebook-github-bot added the Merged label Nov 10, 2021

xwang233 mentioned this pull request Nov 19, 2021

[Linalg] Add a runtime switch to let pytorch prefer a backend impl in linalg functions on GPU #67980

Closed

Factor backend routing logic out of convolution forward #67790

Factor backend routing logic out of convolution forward #67790

Uh oh!

Conversation

jbschlosser commented Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-probot bot commented Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚛️ CI Flow

Uh oh!

facebook-github-bot commented Nov 3, 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!

jbschlosser Nov 4, 2021

Choose a reason for hiding this comment

Uh oh!

jbschlosser Nov 4, 2021

Choose a reason for hiding this comment

Uh oh!

jbschlosser Nov 4, 2021

Choose a reason for hiding this comment

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zou3519 commented Nov 8, 2021

Uh oh!

zou3519 Nov 8, 2021

Choose a reason for hiding this comment

Uh oh!

jbschlosser Nov 9, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zou3519 Nov 9, 2021

Choose a reason for hiding this comment

Uh oh!

jbschlosser Nov 9, 2021

Choose a reason for hiding this comment

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

albanD left a comment

Choose a reason for hiding this comment

Uh oh!

jbschlosser commented Nov 9, 2021

Uh oh!

facebook-github-bot commented Nov 9, 2021

Uh oh!

facebook-github-bot commented Nov 9, 2021

Uh oh!

facebook-github-bot commented Nov 9, 2021

Uh oh!

facebook-github-bot commented Nov 10, 2021

Uh oh!

facebook-github-bot commented Nov 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jbschlosser commented Nov 3, 2021 •

edited

Loading

pytorch-probot bot commented Nov 3, 2021 •

edited

Loading

facebook-github-bot commented Nov 3, 2021 •

edited

Loading