Skip to content

Conversation

@helinwang
Copy link
Contributor

@helinwang helinwang commented Jan 24, 2018

When running notest_dist_label_semantic_roles.py, "emb" is matched with
"embedding_1.w_0", but they are two irrevalent vars, causing the init op for "emb" inits "embedding_1.w_0" instead, so "emb" is not initialized.

Fixes: #7701

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 bug happened because "emb" is matched with "embedding_1.w_0" at this line.

Copy link
Contributor

Choose a reason for hiding this comment

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

Won't the logic remain same if we do not change this line?

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! Wanted to consolidate the code.

abhinavarora
abhinavarora previously approved these changes Jan 24, 2018
Copy link
Contributor

@abhinavarora abhinavarora left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for fixing this issue.

putcn
putcn previously approved these changes Jan 25, 2018
Copy link
Contributor

@putcn putcn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

In notest_dist_label_semantic_roles.py, "emb" is matched with
"embedding_1.w_0", but they are two irrevalent vars.

Fixes: PaddlePaddle#7701
@helinwang helinwang dismissed stale reviews from putcn and abhinavarora via 39b0fdc January 25, 2018 00:45
Copy link
Contributor

@putcn putcn left a comment

Choose a reason for hiding this comment

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

LGTM again

Copy link
Contributor

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for fixing this.

@helinwang helinwang merged commit d380ad0 into PaddlePaddle:develop Jan 25, 2018
@helinwang helinwang deleted the transpiler_fix branch January 25, 2018 03:03
@helinwang helinwang restored the transpiler_fix branch January 27, 2018 00:04
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.

4 participants