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

remove usused ProtoDataProvider related codes #5345

Merged
merged 12 commits into from
Nov 17, 2017
Merged

remove usused ProtoDataProvider related codes #5345

merged 12 commits into from
Nov 17, 2017

Conversation

luotao1
Copy link
Contributor

@luotao1 luotao1 commented Nov 3, 2017

fix #5344

@luotao1
Copy link
Contributor Author

luotao1 commented Nov 15, 2017

ProtoDataProvider删除后,有四个单测过不了,分别用四个commit来修复:

  • test_PyDataProvider.cpp:bbeb826
    • 原因:单测中有PyData和ProtoData读数据一致性的检查。
    • 解决:将这个一致性检查删除。
  • test_CompareTwoOpts.cpp ebb22e5
    • 原因:单测中使用了ProtoData读取数据
    • 解决:改用SimpleData来读取数据
  • test_CompareSparse.cpp cd6d69a
    • 原因:单测中使用了ProtoData读取数据
    • 解决:改用PyData来读取数据。由于这个单测是检查sparse_update这个参数的正确性,因此单测中必须还有embeding层,不能用SimpleData。同时简化了测试的配置。
  • test_CompareTwoNets.cpp 212f6ea
    • 原因:单测中使用了ProtoData读取数据
    • 解决:改用PyData来读取数据。由于这个单测是检查recurrent_layerrecurrent_group两者的一致性,因此单测中必须有序列数据,不能用SimpleData。同时简化了测试的配置。

@qingqing01
Copy link
Contributor

qingqing01 commented Nov 17, 2017

test_CompareTwoOpts.cpp ebb22e5
原因:单测中使用了ProtoData读取数据
解决:改用SimpleData来读取数据

这个降mnist数据换成了 paddle/trainer/tests/sample_data.txt 较简单的数据,换了之后类别数没有改,貌似数据变的也没有意义了。

另外,sample_trainer_config_opt_a.conf, sample_trainer_config_opt_b.conf两个配置文件一模一样,再对比啥呢?

回到最初的改动这些配置的PR,https://github.com/PaddlePaddle/Paddle/pull/126/files ,貌似是在对比 sparse_momentummomentum,但是mnist是个稠密的数据,这个对比还有意义吗?

感觉是不是可以删掉?

@luotao1
Copy link
Contributor Author

luotao1 commented Nov 17, 2017

同意 @qingqing01 的看法,已经删掉了。

@luotao1 luotao1 merged commit ba86885 into PaddlePaddle:develop Nov 17, 2017
@luotao1 luotao1 deleted the ProtoDataProvider branch November 17, 2017 07:26
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.

remove usused ProtoDataProvider related codes
3 participants