Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fluid bytecode c++ executor. #10220

Closed
wants to merge 16 commits into from
Closed

Fluid bytecode c++ executor. #10220

wants to merge 16 commits into from

Conversation

gongweibao
Copy link
Contributor

@gongweibao gongweibao commented Apr 26, 2018 •
edited
Loading

Fix #10221

panyx0718
panyx0718 requested changes Apr 26, 2018
View reviewed changes
Copy link
Contributor

@panyx0718 panyx0718 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems to add a binary to run a programdesc once. Should this be put in a directory called "tools"?

paddle/fluid/operators/detail/desc_executor.cc Outdated
bool read_from_file(const std::string& file, char** buf, int64_t* buf_len) {
FILE* f = fopen(file.c_str(), "rb");
if (NULL == f) {
printf("open %s error\n", file.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fprintf(stderr? or LOG(ERROR)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to fprintf(stderr.
Thanks.

paddle/fluid/operators/detail/desc_executor.cc Outdated

using namespace paddle; // NOLINT

framework::ProgramDesc* load_desc(const std::string& file) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return unique_ptr?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

paddle/fluid/operators/detail/desc_executor.cc Outdated
return (new framework::ProgramDesc(proto));
}

DEFINE_string(start_up_proto, "", "start up proto file");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put at the file top?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.Done.

paddle/fluid/operators/detail/desc_executor.cc Outdated
std::transform(place_str.begin(), place_str.end(), place_str.begin(),
[](unsigned char ch) { return toupper(ch); });

framework::Executor* exe = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unique_ptr>

paddle/fluid/operators/detail/desc_executor.cc Outdated
@@ -0,0 +1,126 @@
// Copyright (c) 2018 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this file by call execute_program_desc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

python/paddle/fluid/executor.py
@@ -333,6 +335,12 @@ def run(self,
fetch_var_name=fetch_var_name)

self._feed_data(program, feed, feed_var_name, scope)

# TODO(gongwb): does a program should be saved in run function?
if len(save_program_to_file) > 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should do the save here...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The programdesc is modified in run function.And sometimes a user can't get chance to save it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe run() shouldn't modify the programdesc? Or we should save after run is called?

@typhoonzero
Copy link
Contributor

Agree to put this to tools

panyx0718
panyx0718 reviewed May 4, 2018
View reviewed changes
python/paddle/fluid/executor.py
@@ -277,7 +277,8 @@ def run(self,
fetch_var_name='fetch',
scope=None,
return_numpy=True,
use_program_cache=False):
use_program_cache=False,
save_program_to_file=""):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't want to put "save_program_to_file" as a argument to Executor. Saving program should be in a separate API, such as "save_inference_model".

@gongweibao
Copy link
Contributor Author

gongweibao commented May 7, 2018 •
edited
Loading

I build debug-version Python and so close this PR.

@gongweibao gongweibao closed this May 7, 2018
@gongweibao gongweibao mentioned this pull request May 8, 2018
Closed
@gongweibao gongweibao reopened this May 16, 2018
@gongweibao gongweibao closed this May 16, 2018
@gongweibao gongweibao deleted the detectmemleak2 branch January 17, 2021 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@panyx0718 panyx0718 panyx0718 requested changes

@jacquesqiao jacquesqiao Awaiting requested review from jacquesqiao

@JiayiFeng JiayiFeng Awaiting requested review from JiayiFeng

@typhoonzero typhoonzero Awaiting requested review from typhoonzero

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gongweibao @typhoonzero @panyx0718