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.

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

Copy Constructor for ProgramDesc #4895

Merged
merged 50 commits into from
Oct 19, 2017

Conversation

reyoung
Copy link
Collaborator

@reyoung reyoung commented Oct 18, 2017

Fix #4896

Note: #4894 is merged into this PR, please review and merge #4894 first.

reyoung and others added 30 commits October 11, 2017 12:08
and Rename `Sync` to `Flush`
Since lots of types can be cast to bool
@reyoung reyoung changed the title [WIP, ToTrigerCI] Feature/copy ctor program des Copy Constructor for ProgramDesc Oct 18, 2017
for (size_t i = 0; i < global_block->OpSize(); ++i) {
auto op_origin = global_block->Op(i);
auto op_copy = global_block->Op(i);
ASSERT_EQ(op_copy->Proto()->SerializeAsString(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we shall check they are not the same op first?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

assert start_index is not None
assert end_index is not None
assert start_index <= end_index
if len(self.ops) != 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting!

Copy link
Collaborator

@JiayiFeng JiayiFeng left a comment

Choose a reason for hiding this comment

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

LGTM

@reyoung reyoung merged commit 47f773d into PaddlePaddle:develop Oct 19, 2017
@reyoung reyoung deleted the feature/copy_ctor_program_des branch October 28, 2017 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants