Use c10::variant-based enums for Reduction #27942

yf225 · 2019-10-14T21:57:30Z

Stack from ghstack:

Fix implementation of F::kl_div / F::mse_loss / F::binary_cross_entropy #28806 Fix implementation of F::kl_div / F::mse_loss / F::binary_cross_entropy
Use c10::variant-based enums for Reduction #27942 Use c10::variant-based enums for Reduction

Differential Revision: D18202857

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

CarMiranda · 2019-10-15T12:26:47Z

torch/csrc/api/include/torch/nn/options/loss.h

@@ -90,7 +92,7 @@ struct TORCH_API TripletMarginLossOptions {
  /// E. Riba et al. Default: False
  TORCH_ARG(bool, swap) = false;
  /// Specifies the reduction to apply to the output. Default: Mean
-  TORCH_ARG(Reduction::Reduction, reduction) = Reduction::Mean;
+  TORCH_ARG(10::variant<enumtype::kNone, enumtype::kMean, enumtype::kSum>, reduction) = torch::kMean;


Sorry I was looking if a PR of mine was in the list and found this PR which kind of concerns what I was doing :P I think you are missing a c here (should be c10) and in previous declarations too!

@CarMiranda Thanks a lot for the catch! I just fixed it. After this PR is merged, I will do a sweep to change all torch::nn layers that use Reduction to use the corresponding variant type. :D

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

…div / mse_loss / binary_cross_entropy" Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

ezyang · 2019-10-22T15:54:42Z

torch/csrc/api/include/torch/enum.h

+// ```
+// Tensor some_functional(
+//     const Tensor& input,
+//     const SomeOptions& options = {}) {


This smells wrong to me. Why don't you just take it by value? It's a very small struct.

I updated the comment here and will do a sweep to take Options by value in all functionals in a follow-up PR. Although it doesn't fix the problem that TORCH_OPTIONS_CTOR_VARIANT_ARG3/TORCH_OPTIONS_CTOR_VARIANT_ARG4 try to address though :(

ezyang · 2019-10-22T18:36:29Z

torch/csrc/api/include/torch/nn/functional/loss.h

+  return torch::l1_loss(
+    input,
+    target,
+    c10::visit(enumtype::_reduction_get_enum{}, options.reduction()));


Instead of manually typing out c10::visit everywhere, why not come up with a good API for doing this and call that instead? Especially since _reduction_get_enum is underscored...

Thanks for the suggestion! I added torch::enumtype::reduction_get_enum() API for this purpose :D

ezyang · 2019-10-22T18:37:54Z

Can you say more about what "fix F::kl_div / mse_loss / binary_cross_entropy" means?

…div / mse_loss / binary_cross_entropy" Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

yf225 · 2019-10-28T21:13:40Z

Can you say more about what "fix F::kl_div / mse_loss / binary_cross_entropy" means?

The original logic in F::kl_div / mse_loss / binary_cross_entropy doesn't match that of Python version. I moved the changes to another PR since it is not strictly related to the Reduction enum changes.

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

facebook-github-bot · 2019-10-30T01:04:29Z

@yf225 merged this pull request in e33b4b6.

Use c10::variant-based enums for Reduction

613969f

yf225 requested review from ebetica and goldsborough as code owners October 14, 2019 21:57

yf225 mentioned this pull request Oct 14, 2019

Use c10::variant-based enums for Nonlinearity and FanMode #27933

Closed

Will Feng added 5 commits October 14, 2019 18:06

Update on "Use c10::variant-based enums for Reduction"

156e86b

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

0d136ef

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

e68ee89

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

2721d31

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

d9d2a54

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

CarMiranda reviewed Oct 15, 2019

View reviewed changes

Will Feng added 20 commits October 15, 2019 12:23

Update on "Use c10::variant-based enums for Reduction"

a1779c6

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

89a4a52

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

dc434e2

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

1e63ddc

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

5b40d26

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

e34bdf8

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

03a5514

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

386ba2f

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

e239b30

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

31b73a7

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

db4bbe9

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

29a222f

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

974ce21

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

e4811e4

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

7c2a79f

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

77bfa44

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

7935d15

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

d74d432

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

a40a9a9

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

a94f742

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

yf225 mentioned this pull request Oct 21, 2019

rename macro #28382

Closed

Update on "Use c10::variant-based enums for Reduction, and fix F::kl_…

f49adc2

…div / mse_loss / binary_cross_entropy" Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

ezyang reviewed Oct 22, 2019

View reviewed changes

Update on "Use c10::variant-based enums for Reduction, and fix F::kl_…

651964a

…div / mse_loss / binary_cross_entropy" Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

yf225 mentioned this pull request Oct 28, 2019

Fix implementation of F::kl_div / F::mse_loss / F::binary_cross_entropy #28806

Closed

Will Feng added 2 commits October 28, 2019 15:21

Update on "Use c10::variant-based enums for Reduction, and fix F::kl_…

70f9648

…div / mse_loss / binary_cross_entropy" Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction, and fix F::kl_…

e814ce6

…div / mse_loss / binary_cross_entropy" Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

yf225 changed the title ~~Use c10::variant-based enums for Reduction, and fix F::kl_div / mse_loss / binary_cross_entropy~~ Use c10::variant-based enums for Reduction Oct 28, 2019

Will Feng added 8 commits October 28, 2019 16:27

Update on "Use c10::variant-based enums for Reduction"

0aedcdc

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

8ade4c0

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

7caada1

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

f239b83

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

4bb57b5

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

324fe92

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

ca25ef9

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

Update on "Use c10::variant-based enums for Reduction"

cee2f4b

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

yf225 requested a review from ezyang October 28, 2019 21:15

ezyang approved these changes Oct 29, 2019

View reviewed changes

Update on "Use c10::variant-based enums for Reduction"

ec5fe38

Use c10::variant-based enums for Reduction gh-metadata: pytorch pytorch 27942 gh/yf225/11/head

facebook-github-bot closed this in e33b4b6 Oct 29, 2019

facebook-github-bot added the merged label Oct 30, 2019

facebook-github-bot deleted the gh/yf225/11/head branch November 2, 2019 14:17

yf225 mentioned this pull request Nov 8, 2019

[C++ API] Add functional overloads for pixelshuffle, pooling, upsampling, vision #29359

Closed

yf225 added the module: bc-breaking Related to a BC-breaking change label Jan 8, 2020

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use c10::variant-based enums for Reduction #27942

Use c10::variant-based enums for Reduction #27942

Uh oh!

yf225 commented Oct 14, 2019 •

edited

Loading

Uh oh!

CarMiranda Oct 15, 2019

Uh oh!

yf225 Oct 15, 2019

Uh oh!

ezyang Oct 22, 2019

Uh oh!

yf225 Oct 28, 2019

Uh oh!

ezyang Oct 22, 2019

Uh oh!

yf225 Oct 28, 2019

Uh oh!

ezyang commented Oct 22, 2019

Uh oh!

yf225 commented Oct 28, 2019

Uh oh!

facebook-github-bot commented Oct 30, 2019

Uh oh!

Uh oh!

Use c10::variant-based enums for Reduction #27942

Use c10::variant-based enums for Reduction #27942

Uh oh!

Conversation

yf225 commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CarMiranda Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

yf225 Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

ezyang Oct 22, 2019

Choose a reason for hiding this comment

Uh oh!

yf225 Oct 28, 2019

Choose a reason for hiding this comment

Uh oh!

ezyang Oct 22, 2019

Choose a reason for hiding this comment

Uh oh!

yf225 Oct 28, 2019

Choose a reason for hiding this comment

Uh oh!

ezyang commented Oct 22, 2019

Uh oh!

yf225 commented Oct 28, 2019

Uh oh!

facebook-github-bot commented Oct 30, 2019

Uh oh!

Uh oh!

yf225 commented Oct 14, 2019 •

edited

Loading