change net to operator #2846
change net to operator #2846
Conversation
paddle/framework/net.h
Outdated
| * @brief Add an Operator according to `def`. | ||
| */ | ||
| virtual OpIndex AddOp(const OpProto &def) = 0; | ||
| virtual void AddOp(const OpDesc& def) = 0; |
There was a problem hiding this comment.
I think it is better to make OpCreator returns a std::shared_ptr<OpBase>, and we could alias it to OpPtr.
AddOp should take a OpPtr.
paddle/framework/net.h
Outdated
|
|
||
| /** | ||
| * @brief Add optimizer operators acctording to `attrs`. | ||
| */ | ||
| virtual void AddOptimizerOps(const OpAttrs &attrs) = 0; | ||
| virtual void AddOptimizerOps() = 0; |
There was a problem hiding this comment.
AddOperatorOps and AddBackwardOps is not a member function of Network.
paddle/framework/net.h
Outdated
|
|
||
| private: | ||
| // the operators owned by `Network`. | ||
| std::vector<Operator> ops_; | ||
| std::vector<std::unique_ptr<OperatorBase>> ops_; |
There was a problem hiding this comment.
std::shared_ptr is better.
The std::shared_ptr is the only pointer can expose to Python correctly.
paddle/framework/net.cc
Outdated
| PlainNet::PlainNet(const NetDesc& def) {} | ||
| void PlainNet::AddOp(const OpDesc& desc) { | ||
| std::unique_ptr<OperatorBase> op(OpRegistry::CreateOp(desc)); | ||
| ops_.push_back(std::move(op)); |
There was a problem hiding this comment.
emplace_back is better.
emplace_back vs push_back
There was a problem hiding this comment.
ok, I will take a look~~
paddle/framework/net.h
Outdated
| OpIndex AddOp(const std::string &type, const std::vector<std::string> &inputs, | ||
| const std::vector<std::string> &outputs, | ||
| const OpAttrs &attrs = OpAttrs()); | ||
| void AddBackwardOps() override {} |
There was a problem hiding this comment.
we decided to net = Backward(net). return net composed by gradient.
| @@ -103,18 +79,10 @@ class Net { | |||
| class PlainNet : public Net { | |||
There was a problem hiding this comment.
since we have reached an agreement in treating net as a composed Op, seems we do not need a PlainNet anymore. We can have two types of Net, Net, DAGNet.
There was a problem hiding this comment.
I think net will have a common interface AddOp, so I add the net base interface with only AddOp
* OperatorBase should not store OpDesc, because not All op contains an
OpDesc and not all ops create from OpDesc.
* Networks do not contain OpDesc, and do not created by OpDesc
* Do not register Network to OpRegistry.
* The network is directly created by user in Python. Not from
registry.
* Correctly handle the `inputs` and `outputs` of a Network.
* Add CompleteAddOp() methods
* Remove `AddOp(OpDesc&)`. All op are added by pointer.
* Rewrite unit test for truely tested what networks do.
* Remove `DemoOp` and `DemoOpTest` because it is useless and break the
CI
Refine NetOp
paddle/framework/operator_test.cc
Outdated
| @@ -1,135 +0,0 @@ | |||
| /* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve. | |||
There was a problem hiding this comment.
It seems that this file should not be removed?
fix: #2857
review: #2851 first