add int32 support for param seq_length of op reverse_sequence #3774
Conversation
|
Can one of the admins verify this patch? |
|
Jenkins, test this please. |
| @@ -76,8 +76,8 @@ void CheckErrors(OpKernelContext* context, int batch_dim, int seq_dim) { | |||
| } | |||
| } | |||
|
|
|||
| template <> | |||
| void CheckErrors<GPUDevice>(OpKernelContext* context, int batch_dim, | |||
| template <typename Tlen> | |||
There was a problem hiding this comment.
why do you need to template CheckErrors on Tlen? I don't see you using Tlen anywhere.
There was a problem hiding this comment.
I'll remove it. I see Jenkins output, and I'll modify gpu version. What does //tensorflow/core/ops/compat:backwards_compatibility_test do?
There was a problem hiding this comment.
cpu version of CheckErrors need Tlen
|
what does |
| template <> | ||
| void CheckErrors<GPUDevice, int64>(OpKernelContext* context, int batch_dim, | ||
| int seq_dim) { | ||
| CheckErrorsGPU(context, batch_dim, seq_dim); |
There was a problem hiding this comment.
i still don't understand why you can't just do:
template <typename Tlen>
void CheckErrors<GPUDevice, Tlen>(...) { check the errors }
?
There was a problem hiding this comment.
that's the point, c++11 don't support partial specialization of function templates, really weird.
There was a problem hiding this comment.
Only full specialization is allowed for function templates in C++11. Partial specialization is only allowed for class templates.
http://en.cppreference.com/w/cpp/language/partial_specialization
|
Jenkins, test this please. |
|
@ebrevdo backwards_compatibility is fixed now. |
|
@tensorflow-jenkins test this please |
#3528