listen_and_serv use local scope #10663
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Xreki
reviewed
May 15, 2018
| std::map<std::string, const LoDTensor*>* feed_targets, | ||
| std::map<std::string, LoDTensor*>* fetch_targets, | ||
| bool create_vars, const std::string& feed_holder_name, | ||
| bool create_local_scope, bool create_vars, |
There was a problem hiding this comment.
create_vars为false时，create_local_scope为true或者false都没有意义啦，不会再创建local_scope。create_vars为true时，同时创建local_scope比较好。- 这个接口目前应该只有inference用到，用户对于是否创建
local_scope没有必要感知。你们需要调用到这个接口吗？如果不需要的话，就暂时不要加这个参数吧。。。
There was a problem hiding this comment.
我觉得保持接口一致会比较友好，开始是计划去掉create_vars这个参数，不过inference看起来也在用了，所以就保留了和Run一致的接口。如果create_local_scope和create_vars总是同时为true的话，是否可以改成一个参数：create_local_scope_and_vars?
There was a problem hiding this comment.
发现merge这两个参数之后会有问题，while_op中用了create_local_scope=false并且并且create_vars=true还是revert到两个参数的版本了。
| std::map<std::string, const LoDTensor*>* feed_targets, | ||
| std::map<std::string, LoDTensor*>* fetch_targets, bool create_vars, | ||
| const std::string& feed_holder_name, const std::string& fetch_holder_name) { | ||
| std::map<std::string, LoDTensor*>* fetch_targets, bool create_local_scope, |
typhoonzero
force-pushed
the
remove_create_vars_in_executor
branch
from
606e8c5 to
5db564f
Compare
… remove_create_vars_in_executor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Provide another fix to #10626 (review)