-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose Net to Python #2967
Expose Net to Python #2967
Conversation
reyoung
commented
Jul 19, 2017
- Expose PlainNet to Python, make python can add_op, complete_add_op
- Provide a low level api to manipulate Net
- Unittest for Net::DebugString
* Expose PlainNet to Python, make python can add_op, complete_add_op * Provide a low level api to manipulate Net * Unittest for Net::DebugString
paddle/pybind/pybind.cc
Outdated
#include <fstream> | ||
#include <vector> | ||
#include "paddle/framework/net.h" |
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.
add an empty line between <> and ""?
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.
done
paddle/pybind/pybind.cc
Outdated
}) | ||
.def("__str__", &ClassType::type::DebugString); | ||
} | ||
|
||
PYBIND11_PLUGIN(core) { | ||
py::module m("core", "C++ core of Paddle Paddle"); |
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.
PaddlePaddle
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.
done
paddle/pybind/pybind.cc
Outdated
net.def_static("create", | ||
[]() -> std::shared_ptr<pd::PlainNet> { | ||
auto retv = std::make_shared<pd::PlainNet>(); | ||
retv->type_ = "naive_net"; |
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.
naive_net -> plain_net
?
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.
done
paddle/framework/net.cc
Outdated
@@ -61,7 +61,10 @@ std::string PlainNet::DebugString() const { | |||
std::ostringstream os; | |||
os << this->type_ << ":" << std::endl; | |||
for (auto& op : ops_) { | |||
os << "\t" << op->DebugString() << std::endl; | |||
std::istringstream is(op->DebugString()); |
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.
这几行是什么意思呢?为什么不是
os << op->DebugString();
或者
string::Fprintf(os, op);
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.
net op是一个组合op,内部有很多其他op组成,所以打印的时候有个循环
for op in ops_:
print op.debugstring()
我理解这个diff是为了打印的更有层次结构,在子op的每一行debugstring前面加一个tab,类似这样
naive_net:
Op(add_two), inputs:(X, Y), outputs:(Out).
naive_net:
fc:
Op(mul), inputs:(X, w), outputs:(@TEMP@fc@0).
Op(sigmoid), inputs:(@TEMP@fc@0), outputs:(fc.out).
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.
LGTM! @reyoung please check if the name PlainNet
is proper.
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.
have undefined order problem~ trying to fix
for (auto& op : ops_) { | ||
os << "\t" << op->DebugString() << std::endl; | ||
std::istringstream is(op->DebugString()); |
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 don't get it -- why we turn to use a more complex way to output the debug string here? If we are not satisfied with the format generated by os << op->DebugString()
, it seems that we should edit the function Operator::DebugString
instead of augmenting its outputs?
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.
trying to find a better way~
paddle/framework/net.cc
Outdated
for (auto& opt : output_set) { | ||
if (Contains(temp_output, opt)) { | ||
tmp_index.push_back(idx); | ||
int output_len = (int)outputs_.size(); |
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.
static_cast<int>
instead of (int)
?
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.
done