Add skeletons of mul, rowwise_add, sigmoid, softmax ops
#2914
Conversation
* Implement InferShape and register them, give a stub Kernel method by LOG(INFO)
paddle/framework/ddim.cc
Outdated
| @@ -278,5 +278,11 @@ std::ostream& operator<<(std::ostream& os, const DDim& ddim) { | |||
| return os; | |||
| } | |||
|
|
|||
| ssize_t DDim::size() const { return product(*this); } | |||
|
|
|||
| DDim::DDim(std::initializer_list<int> init_list) { | |||
There was a problem hiding this comment.
This constructor could be significantly easier for Op developer to invoke Tensor::set_dims.
Just by
tensor.set_dims({100, 784})
paddle/framework/ddim.cc
Outdated
| @@ -278,5 +278,11 @@ std::ostream& operator<<(std::ostream& os, const DDim& ddim) { | |||
| return os; | |||
| } | |||
|
|
|||
| ssize_t DDim::size() const { return product(*this); } | |||
There was a problem hiding this comment.
Also, size() should be a member function for DDim. Invoke product is pretty strange to get the size.
| auto dim0 = inputs[0]->dims(); | ||
| auto dim1 = inputs[1]->dims(); | ||
|
|
||
| PADDLE_ENFORCE(dim0.size() == 2, "Input 0 must be matrix"); |
There was a problem hiding this comment.
Here, a matrix is a tensor with rank=2. It should be
PADDLE_ENFORCE(paddle::framework::arity(dim0)==2, "input 0 must be matrix");
There was a problem hiding this comment.
Done. Change size() to arity
| PADDLE_ENFORCE(inputs.size() == 2, "The mul op must take two inputs"); | ||
| auto dim0 = inputs[0]->dims(); | ||
| auto dim1 = inputs[1]->dims(); | ||
| PADDLE_ENFORCE(dim0.size() == 2 && dim1.size() == 2, |
There was a problem hiding this comment.
It's not the size of dim, but rank of dim.
| op_library(mul_op SRCS mul_op.cc mul_op.cu) | ||
| op_library(rowwise_add_op SRCS rowwise_add_op.cu rowwise_add_op.cc) | ||
| op_library(sigmoid_op SRCS sigmoid_op.cu sigmoid_op.cc) | ||
| op_library(softmax_op SRCS softmax_op.cc softmax_op.cu) |
There was a problem hiding this comment.
It's better to maintain a consistent sequence. .cc first and .cu follows.
There was a problem hiding this comment.
Just a unit-test here to test op_library is not caring about source sequence.
|
|
||
| #include <paddle/framework/op_registry.h> | ||
| #include <paddle/framework/tensor.h> | ||
| #include <paddle/operators/mul_op.h> |
There was a problem hiding this comment.
Please follow the code style, using
#include "paddle/framework/op_registry.h"
#include "paddle/framework/tensor.h"
#include "paddle/operators/mul_op.h"
* There is a merge conflict when merge PR PaddlePaddle#2914 * Develop and PR PaddlePaddle#2914 both add `DDim::size` method, but did not triger git merge conflict while merge.
by LOG(INFO)