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

SimpleData defined in config_parser.py and config_parser_extension.py #2291

Closed
wangkuiyi opened this issue May 26, 2017 · 6 comments
Closed
Assignees

Comments

@wangkuiyi
Copy link
Collaborator

In directory python/paddle/trainer, global function SimpleData are defined in both

  1. config_parser.py, and
  2. config_parser_extension.py.

The two definitions are almost the same.

Do we need both of them?

@jacquesqiao
Copy link
Member

我在svn日志里面搜了下python/paddle/trainer/config_parser_extension.py这个文件的提交记录,发现这个文件是骆涛2016-08-19从python/paddle/trainer/tests/config_parser_extension.py 移过来的,不是很确定具体用途,这个时间点大概是paddle开源之前整理代码的阶段,可能需要 @luotao1 帮忙看下。

这个文件目前只在paddle/paddle/trainer/tests/config_parser_test.py被用到:

if __name__ == '__main__':
    parse_config_and_serialize('trainer/tests/test_config.conf', '')
    parse_config_and_serialize(
        'trainer/tests/sample_trainer_config.conf',
        'extension_module_name=paddle.trainer.config_parser_extension')
    parse_config_and_serialize('gserver/tests/pyDataProvider/trainer.conf', '')

@luotao1
Copy link
Contributor

luotao1 commented May 28, 2017

SimpleData确实只是在单测中使用,当时留着的原因是,希望给想自定义数据的用户一个演示。当时定义的数据类似TrainDataTestData这种。

@wangkuiyi
Copy link
Collaborator Author

How about we remove duplicated code? @luotao1

@luotao1
Copy link
Contributor

luotao1 commented Jun 1, 2017

@wangkuiyi 仔细看了下,这两个code目前都不能简单地删掉:

  1. config_parser.py 中的simpleData, 主要用于test_TrainerOnePass中的两个单测配置sample_trainer_config.confsample_trainer_config_parallel.conf.conf读数据的部分。因为这个单测是测试训练后的精度是否正常,如果删掉,需要把配置升级成v2 api,且改该单测的逻辑。
  2. config_parser_extension.py中的SimpleData,用于config_parser_test.py中测试config_parser.py的extension_module是否正常。 @lcy-seso config_parser.py重构的时候还需要extension_module这个功能么?如果不需要的话,整个config_parser_extension.py可以删掉。

@lcy-seso
Copy link
Contributor

lcy-seso commented Jun 1, 2017

也看了一下代码,

  1. config_parser_extension.py 这个文件没有真正的功能上的作用,目前仅仅为了在单测paddle/trainer/tests/config_parser_extension.py 中测试config_parser.py的extension_module功能是否正常。这个文件放移动到paddle/trainer/tests下更合适。
  2. extension_model将用户自己定义的一个python脚本在config_parser.py作为一个module import,这样在网络结构配置中可以调用自己定义的一些函数。目前我们没有大的修改过config_parser.py,也不会删掉extension_module这个功能。这种情况下,config_parser_extension.py 这个单测最好不要删掉。
  3. 如果是为了避免代码重复,可以把config_parser_extension.py中的SimpleData换成另外的用户自定义函数。(但似乎意义不大?)

@lcy-seso
Copy link
Contributor

I close this issue due to inactivity. Please feel free to reopen it.

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

No branches or pull requests

7 participants