add remove_var from c++ end #9607
Conversation
|
I don't understand why do we need remove_op and remove_var? In my mind, the ProgramDesc is constructive -- built by adding ops and vars from nothing. |
|
@wangkuiyi Sorry for the description of background. The motivation is that we want to fuse batch normalization during inference, and we need an inference transpiler to implement it. As we need to remove batch_norm ops and unused variables from ProgramDesc, we need |
| @@ -96,6 +97,8 @@ class BlockDesc { | |||
| */ | |||
| void RemoveOp(size_t s, size_t e); | |||
|
|
|||
| void RemoveVar(const std::string &name) { vars_.erase(name); } | |||
There was a problem hiding this comment.
Note for myself: sync_with_cpp already add sync removed vars in previous PR.
| @@ -200,13 +202,19 @@ void BindBlockDesc(py::module &m) { | |||
| return self.FindVarRecursive(name); | |||
| }, | |||
| py::return_value_policy::reference) | |||
| .def("remove_var", | |||
There was a problem hiding this comment.
I began to think our previous design of XXXDesc maybe a wrong design. When we need to manipulate the ProgramDesc proto message, we have to write corresponding c++ implementation, then expose it to pybind, and sync the python holders of the c++ side objects, this makes the code complicated and not simple enough.
When comes with transpilers which need to add, cut, remove operators, variables, and blocks in the ProgramDesc, we must be very careful that the c++ side object reference may have already deleted.
A simpler way is to manipulate the ProgramDesc proto message directly in python, and give it to the c++ executor. For performance, python is slow indeed, but if we only use it in compile time and transpile time, it won't affect the total training performance.
There was a problem hiding this comment.
@wangkuiyi @reyoung can you please take a look at the above comment?
There was a problem hiding this comment.
I agree with @typhoonzero. It is so difficult for add, cut, insert, remove operators, variables, and blocks in the ProgramDesc for writing a transpiler, I must change sync_with_cpp method in framework.py at the same time.
Related PR: refine sync_with_cpp when remove ops or remove vars #9600
There was a problem hiding this comment.
Indeed need some rethink
remove_opmethod, this PR addremove_var(var_name)method for removing a particular varible.test_protobuf_desc.pywith refine sync_with_cpp when remove ops or remove vars #9600 (comment)block_desc.handprotobuf.ccwhengit commitandcpplintfails: