Add a padding operator. #95
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| #ifndef _OPERATORS_PADDING_OP_H_ | ||
| #define _OPERATORS_PADDING_OP_H_ | ||
|
|
||
| #include "smaug/core/backend.h" | ||
| #include "smaug/core/operator.h" | ||
| #include "smaug/core/tensor.h" | ||
| #include "smaug/core/workspace.h" | ||
| #include <google/protobuf/repeated_field.h> | ||
| using namespace google::protobuf; | ||
|
|
||
| namespace smaug { | ||
|
|
||
| /** \ingroup Operators | ||
| * \brief Pad a given tensor in any number of dimensions with arbitrary size. | ||
| * | ||
| * This has a software-based implementation. | ||
| * | ||
| * @tparam Backend The Backend that sets Alignment. | ||
| */ | ||
| template <typename Backend> | ||
| class PaddingOp : public Operator { | ||
| public: | ||
| PaddingOp(const std::string& name, Workspace* workspace) | ||
| : Operator(name, OpType::Padding, workspace) { | ||
| inputs.resize(kNumInputs, nullptr); | ||
| outputs.resize(kNumOutputs, nullptr); | ||
| } | ||
|
|
||
| /** | ||
| * Set the paddingSize of the Tensor along each dimension. | ||
| * The paddingSize is orgainized as <dim1_forward, dim1_backward, ... | ||
| * ,dimk_backward> | ||
| */ | ||
| void setPaddingSize(RepeatedField<google::protobuf::int32> const& val) { | ||
|
There was a problem hiding this comment. This should be Also, what does "forward" and "backward" mean? |
||
| std::vector<double> paddingSize(val.begin(), val.end()); | ||
|
There was a problem hiding this comment. Why converting to double here? There was a problem hiding this comment. I change it to int. |
||
| } | ||
|
|
||
| void setPaddingSize(std::vector<int> const& val) { paddingSize = val; } | ||
|
|
||
| std::vector<int> getPaddingSize() { return paddingSize; } | ||
|
There was a problem hiding this comment. This should be a const member function. |
||
|
|
||
| void run() override { | ||
| Tensor* input = getInput(0); | ||
|
There was a problem hiding this comment. Use the enums defined below ( |
||
| Tensor* output = getOutput(0); | ||
| int ndims = input->ndims(); | ||
| const std::vector<int> inputDims = input->getShape().dims(); | ||
|
There was a problem hiding this comment. both |
||
| const std::vector<int> outputDims = output->getShape().dims(); | ||
| int total_dim = 1; | ||
| for (int i : outputDims) { | ||
| total_dim *= i; | ||
| } | ||
| std::vector<float> vf(total_dim, 0); | ||
| output->fillData(vf.data(), vf.size()); | ||
| std::vector<int> destOrigin, paddingBegin, srcOrigin; | ||
| for (int i = 0; i < ndims; i++) { | ||
| paddingBegin.push_back(paddingSize[2 * i]); | ||
|
There was a problem hiding this comment. use |
||
| srcOrigin.push_back(0); | ||
| } | ||
| destOrigin = std::vector<int>(paddingBegin); | ||
|
There was a problem hiding this comment. There's no need to declare destOrigin earlier and then re-initialize it here - that causes the vector to be constructed twice. Just declare it here directly: |
||
| std::vector<int> regionSize = inputDims; | ||
|
There was a problem hiding this comment. Same here - no need to make a copy of inputDims, just use it directly. I think you're trying to make it more clear what each vector is representing in the copyTensorRegion call but there's no need since the API documents it very clearly already. |
||
| copyTensorRegion(output, input, destOrigin, srcOrigin, regionSize); | ||
| } | ||
|
|
||
| // Optional override for testing purposes. | ||
| void createAllTensors() override { | ||
| Tensor* input = getInput(0); | ||
| int ndims = input->ndims(); | ||
| std::vector<int> dims = input->getShape().dims(); | ||
| for (int i = 0; i < ndims; i++) { | ||
| dims[i] += (paddingSize[2 * i] + paddingSize[2 * i + 1]); | ||
| } | ||
| TensorShape shape( | ||
| dims, input->getShape().getLayout(), Backend::Alignment); | ||
| Tensor* output = new Tensor(name, shape); | ||
| workspace->addTensor(output); | ||
| outputs.at(0) = output; | ||
|
There was a problem hiding this comment. Same here - use enums instead of hardcoded constants. |
||
| } | ||
|
|
||
| // Optional but recommended function to verify operator parameters. | ||
| bool validate() override { | ||
| Tensor* input = getInput(0); | ||
| int ndims = input->ndims(); | ||
| if (paddingSize.size() != 2 * ndims) { | ||
| return false; | ||
| } | ||
| return Operator::validate(); | ||
| } | ||
|
|
||
| enum { kInputs, kNumInputs }; | ||
|
There was a problem hiding this comment. nit: |
||
| enum { kOutputs, kNumOutputs }; | ||
|
|
||
| private: | ||
| std::vector<int> paddingSize = {}; | ||
| }; | ||
|
|
||
| } // namespace smaug | ||
|
|
||
| #endif | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code rearrangement generally is confusing :) Moving forward, please move this to a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely appreciate you sorting all the includes in alphabetical order! It's certainly cleaner than before. But yes, refactoring and code cleanups should be done separately if it's not related to the main PR purpose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll fix this