add lisht kernel #529
add lisht kernel #529
Conversation
WindQAQ
requested review from
facaiy,
seanpmorgan and
a team
as code owners
| namespace tensorflow { | ||
| namespace addons { | ||
|
|
||
| using CPUDevice = Eigen::ThreadPoolDevice; |
There was a problem hiding this comment.
Just out of curiosity. Is there any difference between these two? I do suppose our ops are compiled with c++11 standard.
Quote from cppreference.
There is no difference between a type alias declaration and typedef declaration.
There was a problem hiding this comment.
Good question. Their difference is: the alias declaration is compatible with templates, whereas the C style typedef is not. My first thought is for consistency with tf core.
There was a problem hiding this comment.
|
|
||
| #if GOOGLE_CUDA | ||
|
|
||
| using GPUDevice = Eigen::GpuDevice; |
|
|
||
|
|
||
| ## Contribution Guidelines | ||
| #### Standard API | ||
| In order to conform with the current API standard, all activations | ||
| must: | ||
| * Be a `tf.function`. | ||
| * Have the signature `fn(input, axis=-1, name=None)`. |
There was a problem hiding this comment.
Not needed in this PR, but wondering if we should remove the name parameter from sparsemax activation
There was a problem hiding this comment.
I think it would be better to remove it (at least the style is consistent in the same submodule). But not sure if other operations in image and text etc should keep it.
There was a problem hiding this comment.
I prefer to keep name in other modules :-)
There was a problem hiding this comment.
Sure, I will create another PR to clean up activation functions (name arg, duplicated tests etc.)
There was a problem hiding this comment.
Looks good. cc @Squadrick @fsx950223 if they have any concerns about this change?
| REGISTER_OP("Addons>Lisht") | ||
| .Input("features: T") | ||
| .Output("activations: T") | ||
| .Attr("T: {half, float, double}") |
There was a problem hiding this comment.
Would be a good feature if we could support bfloat16. I will investigate it later and make all acitvations support bfloat16 in the following PRs. Thanks for the suggestions!
| .Input("gradients: T") | ||
| .Input("features: T") | ||
| .Output("backprops: T") | ||
| .Attr("T: {half, float, double}") |
| "@local_config_tf//:tf_header_lib", | ||
| ] + if_cuda_is_configured([ | ||
| "@local_config_cuda//cuda:cuda_libs", | ||
| "@local_config_cuda//cuda:cuda_headers", |
There was a problem hiding this comment.
Maybe there should be a bazel function wrap the cuda config
tfa.activations.*