test code for issue #729 #734

hohdiy · 2016-12-05T13:11:34Z

test code for issue #729

rebase

reyoung · 2016-12-06T08:52:56Z

python/paddle/trainer_config_helpers/tests/configs/run_tests.sh

@@ -16,20 +16,23 @@ if [ -z $1 ]; then
  do
      base_protostr=$protostr/$file
      new_protostr=$protostr/$file.unittest
-      diff $base_protostr $new_protostr -u
+      diff $base_protostr $new_protostr -u &&
+      diff $protostr/$file $protostr/$file.non_file_config.unittest -u
  done
 else
  for file in ${configs[*]}
  do
    if ! $1 $protostr/$file.protostr $protostr/$file.protostr.unittest; then


Actually in this branch, if statement used for check whether is different between $protostr/$file.protostr $protostr/$file.protostr.unittest. The rest diff command just used to show where is the difference.

reyoung · 2016-12-06T08:53:16Z

python/paddle/trainer_config_helpers/tests/configs/run_tests.sh

@@ -16,20 +16,23 @@ if [ -z $1 ]; then
  do
      base_protostr=$protostr/$file
      new_protostr=$protostr/$file.unittest
-      diff $base_protostr $new_protostr -u
+      diff $base_protostr $new_protostr -u &&


because of set -e is added, && is not needed.

reyoung · 2016-12-06T08:55:01Z

python/paddle/trainer_config_helpers/tests/configs/run_tests.sh

    fi
  done

  for file in ${whole_configs[*]}
-  do
+do


bad indent.

reyoung · 2016-12-06T08:55:32Z

python/paddle/trainer_config_helpers/tests/configs/test_config_parser_for_non_file_config.py

+
+import sys
+import getopt
+


add

if __name__ == '__main__'

to python code.

After #450 PR, Developing Paddle need use pre-commit tools to check code style, auto-format code, etc. Please try to use pre-commit tool.

reyoung · 2016-12-06T09:01:22Z

python/paddle/trainer_config_helpers/tests/configs/test_config_parser_for_non_file_config.py

+
+cmdstr = """
+from paddle.trainer.config_parser import *
+from paddle.trainer_config_helpers import *


This implementation is too tricky. Maybe the simplest implementation is just to write a single file to test parse_config from a python function with a very simple neural network. Although the test-coverage is not full we could test this mechanism work well or not.

hohdiy · 2016-12-07T02:53:58Z

.//.git/hooks/pre-commit.sample 这个pre-commit tool 有现成可用的吗？给我发一个？
关于diff proto地方，看你是说有一个protobuf_equal程序，是说增加这个程序的执行来检查？
- 我查看了原有的diff proto是对debug string 进行diff，当有diff时会以diff格式打印出来，应该是满足需求的。
- 这个地方如果需要用你说的protobuf_equal，不太清楚应该怎么调用（因为那个bin是编译出来的，不是直接可用的）
另外关于我使用生成代码模式对所有已有的config_parser单测过一遍，我感觉是一个一劳永逸的方法，后续只需关注配置文件模式的单测。
- 我看你的建议是写一个独立的文件只测试一个简单的配置，但是会有单测覆盖的担心。
- 这个地方我想坚持这种做法，是否ok？

其他点都按照你说的修改。
@reyoung

reyoung · 2016-12-07T03:03:38Z

.//.git/hooks/pre-commit.sample 这个pre-commit tool 有现成可用的吗？给我发一个？

See #450; we use this tool to manage our pre-commit.

关于diff proto地方，看你是说有一个protobuf_equal程序，是说增加这个程序的执行来检查？

不是的。在这个分支中，是if语句在判断两个protofile是不是一致，而不是后面的diff语句。后面的diff语句仅仅是作为错误输出提示用户的。

所以，那个if语句，应该改成类似于 if $1 a.proto a.proto.unittest && $1 a.proto a.proto.by.function。

另外关于我使用生成代码模式对所有已有的config_parser单测过一遍，我感觉是一个一劳永逸的方法，后续只需关注配置文件模式的单测

这里有一个问题，是import的东西可能不仅仅是这两个包。用户也可以import numpy之类的第三方包。在那种情况下，这个script就错了。

其实，应该将生成的脚本改成类似于

import paddle.trainer.config_parser as parser

def config():
    # blow here, just copy user configuration file to this method WITH import statements.
    from paddle.trainer.trainer_config_helpers import *
    import numpy
    ....

if __name__ == '__main__':
    print parser.parse_config(config)

hohdiy · 2016-12-08T10:34:37Z

已经按照要求进行了修改。
这里需要说明一下，在自动生成代码时，import * 不能放在包含有闭包的函数中，所以我这边将import提到了最外面。对应的单测是：test_rnn_group.py
@reyoung

reyoung

LGTM. THX.

fix the kubectl address test=develop

hohdiy and others added 2 commits December 5, 2016 20:56

Merge pull request #2 from PaddlePaddle/develop

5777fae

rebase

test code for issue PaddlePaddle#729

7f78912

reyoung requested changes Dec 6, 2016

View reviewed changes

update cr for pull PaddlePaddle#734.

23d5733

fix bad ident

957794c

reyoung approved these changes Dec 12, 2016

View reviewed changes

reyoung merged commit 56c20f8 into PaddlePaddle:develop Dec 12, 2016

zhhsplendid pushed a commit to zhhsplendid/Paddle that referenced this pull request Sep 25, 2019

Merge pull request PaddlePaddle#734 from wopeizl/fix_deploy_ctr

868d40e

fix the kubectl address test=develop

lizexu123 pushed a commit to lizexu123/Paddle that referenced this pull request Feb 23, 2024

Skip leaves in graph when pruning (PaddlePaddle#734)

8eacc16

WAYKEN-TSE pushed a commit to WAYKEN-TSE/Paddle that referenced this pull request Dec 6, 2024

paddlemix v2.1 readme (PaddlePaddle#734)

dc16221

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test code for issue #729 #734

test code for issue #729 #734

hohdiy commented Dec 5, 2016

reyoung Dec 6, 2016

reyoung Dec 6, 2016

hohdiy Dec 8, 2016

reyoung Dec 6, 2016

hohdiy Dec 7, 2016

reyoung Dec 6, 2016

hohdiy Dec 7, 2016

reyoung Dec 6, 2016

reyoung Dec 6, 2016

hohdiy Dec 7, 2016

reyoung Dec 6, 2016

hohdiy commented Dec 7, 2016 •

edited

Loading

reyoung commented Dec 7, 2016

hohdiy commented Dec 8, 2016 •

edited

Loading

reyoung left a comment

test code for issue #729 #734

test code for issue #729 #734

Conversation

hohdiy commented Dec 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hohdiy commented Dec 7, 2016 • edited Loading

reyoung commented Dec 7, 2016

hohdiy commented Dec 8, 2016 • edited Loading

reyoung left a comment

Choose a reason for hiding this comment

hohdiy commented Dec 7, 2016 •

edited

Loading

hohdiy commented Dec 8, 2016 •

edited

Loading