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

fix bugs of parsing network topology in text generation task #2384

Merged
merged 5 commits into from
Jun 12, 2017

Conversation

lcy-seso
Copy link
Contributor

@lcy-seso lcy-seso commented Jun 5, 2017

fix #2349. fix #2385

  1. fix the bug that eos_layer is not trimmed as a used layer in text generation.
  2. fix a bug in parsing the extra evaluator in SRL demo.
  3. move dropout_layer from paddle.network to paddle.layer
  4. export StaticInput and GeneratedInput. Currently, StaticInputV2 and GeneratedInputV2 are not available anymore, use StaticInput and GeneratedInput instead.

@lcy-seso lcy-seso requested review from luotao1 and livc June 5, 2017 13:04
@lcy-seso
Copy link
Contributor Author

lcy-seso commented Jun 5, 2017

Do not approve. beam search still cannot be parsed correctly in v2 API.

Miscs
=====

dropout_layer
Copy link
Contributor

Choose a reason for hiding this comment

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

v2 api中默认后面不加layer,dropout_layer后面为什么要加呢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经修改。将 drop 从 network 下移动到 layer 下,潜在会影响一些旧的配置。

@lcy-seso lcy-seso force-pushed the fix_config_parsing_bug branch 2 times, most recently from f6a2934 to 0e16aed Compare June 6, 2017 13:30
@lcy-seso lcy-seso requested a review from emailweixu June 6, 2017 13:33
@lcy-seso lcy-seso changed the title import missing configuration functions in v2 API. fix bugs of parsing network topology in text generation task Jun 6, 2017
@lcy-seso lcy-seso removed the request for review from livc June 6, 2017 13:43
@helinwang
Copy link
Contributor

Please ignore "TeamCity build failed", see: #2401

@lcy-seso
Copy link
Contributor Author

lcy-seso commented Jun 7, 2017

@helinwang I get it. Thank you.

Copy link
Contributor Author

@lcy-seso lcy-seso left a comment

Choose a reason for hiding this comment

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

fix a bug of parsing evaluator.

@@ -235,11 +235,10 @@ def __trim_submodel__(old_submodel, layer_names, input_layer_names,
def parse_network(output_layers, extra_layers=None):
if not isinstance(output_layers, collections.Sequence):
output_layers = [output_layers]
if extra_layers is not None and not isinstance(extra_layers,
collections.Sequence):
Copy link
Contributor Author

@lcy-seso lcy-seso Jun 7, 2017

Choose a reason for hiding this comment

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

  • Topology 的构造函数中会对 extra_layers 的类型进行检查
  • 检查的过程中,如果传入的extra_layer 不是 python list,会转化为 list
    https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/v2/topology.py#L43
  • Topology 构造函数调用 parse_network走到这里时,如果指定了 extra_layerextra_layer 是一个 python list
  • 这两个条件一个为 True,一个为False 与之后,会走到 else 分枝将 extra_layer 清空

for l in cp.g_config.model_config.layers:
if l.name not in layer_names:
continue
model_config.layers.extend([l])
if l.type == 'data':
if l.name in model_config.output_layer_names:
continue
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to add a comment. Otherwise it's very hard to understand.

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.

return [(nm, data_layers[nm].data_type)
for nm in self.proto().input_layer_names]
for nm in self.proto().input_layer_names if nm in data_layers]
Copy link
Collaborator

Choose a reason for hiding this comment

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

when is input_layer not a data_layer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a mistake. I have fixed this.

Copy link
Contributor Author

@lcy-seso lcy-seso left a comment

Choose a reason for hiding this comment

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

follow comments.

for l in cp.g_config.model_config.layers:
if l.name not in layer_names:
continue
model_config.layers.extend([l])
if l.type == 'data':
if l.name in model_config.output_layer_names:
continue
Copy link
Contributor Author

Choose a reason for hiding this comment

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

return [(nm, data_layers[nm].data_type)
for nm in self.proto().input_layer_names]
for nm in self.proto().input_layer_names if nm in data_layers]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a mistake. I have fixed this.

Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

LGTM

@lcy-seso lcy-seso merged commit 027c5db into PaddlePaddle:develop Jun 12, 2017
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.

beam search cannot be parsed correctly. StaticInputV2 and GeneratedInputV2 missing in paddle.v2.layer
4 participants