-
Notifications
You must be signed in to change notification settings - Fork 122
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
add separate compilation for feature engineering #241
Conversation
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
@@ -20,8 +20,4 @@ | |||
'-I.', | |||
'--python_out=.', | |||
'--grpc_python_out=.', | |||
'metrics.proto', )) |
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.
can we do this with cmake?
make install | ||
``` | ||
|
||
3.生成grpc_pb |
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.
如果能在cmake里面完成protoc,那么这个步骤可以省略
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.
1.在cmake里没有编译c++ 的grpc;直接使用了python的grpc;
2.proto生成的metrics_pb2_grpc.py 必须是 from paddle_fl.feature_engineering.proto import metrics_pb , 或者 from . import metrics_pb2 ; 自动生成的前面一般没有这个from xxx,那么在pip install 时候会报错“no modulo named xxx”
综合考虑在这里使用手动生成的方法
@honshj 在最新的提交中,增加了cmake 编译grpc proto的方式,在单独编译时需要先编译protobuf和grpc。 |
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
Thanks for your comments #238 ! @honshj The main changes are as follows:
Thanks for your review and help! @honshj @kaih70