Use c10::variant-based enums for Nonlinearity and FanMode#27933
Use c10::variant-based enums for Nonlinearity and FanMode#27933yf225 wants to merge 21 commits intogh/yf225/10/basefrom
Conversation
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
| } else if (nonlinearity == torch::nn::init::Nonlinearity::LeakyReLU) { | ||
| } else if (c10::get_if<enumtype::kLeakyReLU>(&nonlinearity)) { | ||
| return std::sqrt(2.0 / (1 + pow(param, 2))); // NOLINT | ||
| } |
There was a problem hiding this comment.
Hmm. If we ever add a new variant to the enum, will we then get an error saying we didn't handle all cases yet? If not, we should add a fallthrough case here...
There was a problem hiding this comment.
Yeah we should have a fallthrough case here - I will do it as part of the work for torch::nn::init::calculate_gain parity.
| LeakyReLU | ||
| }; | ||
|
|
||
| // This enum class is deprecated and will be removed in 1.5. |
There was a problem hiding this comment.
Hmm, I wonder if we can statically warn in these cases... does C10_DEPRECATED work here?
There was a problem hiding this comment.
It seems that adding C10_DEPRECATED prevents the enum class from being used in some CI configs such as pytorch_macos_10_13_py3_test, hence breaking BC-support. For now I'll keep the comment instead.
|
|
||
| #define COMPUTE_NONLINEARITY_ENUM(name) /* NOLINT(cppcoreguidelines-macro-usage) */ \ | ||
| case Nonlinearity::name: \ | ||
| TORCH_WARN( \ |
There was a problem hiding this comment.
I do want people to move to the new enum style sooner than later, because the old style will be deprecated soon :D I think it's probably okay for the warning to be a bit more annoying here, since moving to the new style is minimal effort.
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Use c10::variant-based enums for Nonlinearity and FanMode gh-metadata: pytorch pytorch 27933 gh/yf225/10/head
Summary: Pull Request resolved: pytorch#27933 Test Plan: Imported from OSS Differential Revision: D18009044 Pulled By: yf225 fbshipit-source-id: e88229ee30badf7a699f62af61d1e88debc0dc7d
Stack from ghstack:
Differential Revision: D18009044