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

Better version of PR #7985 (Modify load() for inference) #8024

Merged
merged 21 commits into from
Feb 8, 2018
Merged

Better version of PR #7985 (Modify load() for inference) #8024

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e556d23
Refine load
sidgoyal78 Feb 1, 2018
503f73a
Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…
sidgoyal78 Feb 2, 2018
749e33e
Address review comments: round 1
sidgoyal78 Feb 2, 2018
f8570e6
Make API consistent with python-save/load
sidgoyal78 Feb 2, 2018
076e0e5
Add another unit test
sidgoyal78 Feb 2, 2018
0692742
Remove commented function
sidgoyal78 Feb 2, 2018
b17af18
Fix GPU bug
sidgoyal78 Feb 2, 2018
e2b88c8
Address review comments
sidgoyal78 Feb 6, 2018
55f40e5
Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…
sidgoyal78 Feb 6, 2018
19343d4
Modify wrt PR 8147
sidgoyal78 Feb 6, 2018
4177474
Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…
sidgoyal78 Feb 6, 2018
a6f0b0f
Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…
sidgoyal78 Feb 6, 2018
e0f8e74
Fix filenames for combined case
sidgoyal78 Feb 6, 2018
8fb97bc
Fix typo
sidgoyal78 Feb 6, 2018
e813d13
Address review comments: round 2
sidgoyal78 Feb 7, 2018
5430ddd
Resolve merge conflict
sidgoyal78 Feb 7, 2018
83b4616
Unify TestInference by keeping default param in template
sidgoyal78 Feb 7, 2018
8a002ce
Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…
sidgoyal78 Feb 8, 2018
4d61569
Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…
sidgoyal78 Feb 8, 2018
805b415
Address review comment
sidgoyal78 Feb 8, 2018
33570f7
Fix spacing
sidgoyal78 Feb 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 56 additions & 6 deletions paddle/inference/io.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,24 +73,74 @@ void LoadPersistables(framework::Executor& executor,
delete load_program;
}

std::unique_ptr<framework::ProgramDesc> Load(framework::Executor& executor,
framework::Scope& scope,
const std::string& dirname) {
// This is called when we have a filen named "param_filename"
// which has all the persistable variables stored in a
// combined manner.
void LoadPersistables(framework::Executor& executor,
framework::Scope& scope,
const std::string& dirname,
const framework::ProgramDesc& main_program,
const std::string& param_filename) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There are two LoadPersistables. Can merge them into one?

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

const framework::BlockDesc& global_block = main_program.Block(0);

framework::ProgramDesc* load_program = new framework::ProgramDesc();
framework::BlockDesc* load_block = load_program->MutableBlock(0);
std::vector<std::string> paramlist;

for (auto* var : global_block.AllVars()) {
if (IsParameter(var, main_program)) {
VLOG(3) << "parameter's name: " << var->Name();

framework::VarDesc* new_var = load_block->Var(var->Name());
new_var->SetShape(var->Shape());
new_var->SetDataType(var->GetDataType());
new_var->SetType(var->GetType());
new_var->SetLoDLevel(var->GetLoDLevel());
new_var->SetPersistable(true);

paramlist.push_back(new_var->Name());
}
}

// sort paramlist to have consistent ordering
std::sort(paramlist.begin(), paramlist.end());

// append_op
framework::OpDesc* op = load_block->AppendOp();
op->SetType("load_combine");
op->SetOutput("Out", paramlist);
op->SetAttr("file_path", {dirname + "/" + param_filename});
op->CheckAttrs();
executor.Run(*load_program, &scope, 0, true, true);

VLOG(3) << "Ran loading successfully";
delete load_program;
}

std::unique_ptr<framework::ProgramDesc> Load(
framework::Executor& executor,
framework::Scope& scope,
const std::string& dirname,
const std::string& param_filename) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to keep two Load() interfaces as:

std::unique_ptr<framework::ProgramDesc> Load(
     framework::Executor& executor,
     framework::Scope& scope,
     const std::string& dirname);

std::unique_ptr<framework::ProgramDesc> Load(
     framework::Executor& executor,
     framework::Scope& scope,
     const std::string& prog_filename,
     const std::string& params_filename);

Users are free to rename the file which saving the program. The argument list of save_inference_model may be changed in the future. #7995 is merged, we need to test the new Load. Please use the new Load in the unittest instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for not describing my suggest clearly. I mean to define the interface to:

std::unique_ptr<framework::ProgramDesc> Load(
    framework::Executor& executor,
    framework::Scope& scope,
    const std::string& prog_filepath,
    const std::string& param_filepath);

No need of dirname any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I think you described your suggestion clearly: #8024 (comment)

But, I chose to keep another argument of dirname to make stuff consistent with the python side. More specifically, in save_vars() we have: https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/v2/fluid/io.py#L124, and in load_vars() we have: https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/v2/fluid/io.py#L202

Hence, I put that extra argument. But it seems that you would prefer an implementation without it, so I will modify it. Thanks.

std::string model_filename = dirname + "/__model__";
LOG(INFO) << "loading model from " << model_filename;
VLOG(3) << "loading model from " << model_filename;
std::ifstream inputfs(model_filename, std::ios::in | std::ios::binary);
std::string program_desc_str;
inputfs.seekg(0, std::ios::end);
program_desc_str.resize(inputfs.tellg());
inputfs.seekg(0, std::ios::beg);
LOG(INFO) << "program_desc_str's size: " << program_desc_str.size();
VLOG(3) << "program_desc_str's size: " << program_desc_str.size();
inputfs.read(&program_desc_str[0], program_desc_str.size());
inputfs.close();

std::unique_ptr<framework::ProgramDesc> main_program(
new framework::ProgramDesc(program_desc_str));

LoadPersistables(executor, scope, dirname, *main_program);
if (!param_filename.empty()) {
LoadPersistables(executor, scope, dirname, *main_program, param_filename);
} else {
LoadPersistables(executor, scope, dirname, *main_program);
}
return main_program;
}

Expand Down
14 changes: 11 additions & 3 deletions paddle/inference/io.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,17 @@ void LoadPersistables(framework::Executor& executor,
const std::string& dirname,
const framework::ProgramDesc& main_program);

std::unique_ptr<framework::ProgramDesc> Load(framework::Executor& executor,
framework::Scope& scope,
const std::string& dirname);
void LoadPersistables(framework::Executor& executor,
framework::Scope& scope,
const std::string& dirname,
const framework::ProgramDesc& main_program,
const std::string& param_filename);

std::unique_ptr<framework::ProgramDesc> Load(
framework::Executor& executor,
framework::Scope& scope,
const std::string& dirname,
const std::string& param_filename = "");

} // namespace inference
} // namespace paddle