enable merge grad unit test #4860
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
and add activation in unit tests
tensor-tang
force-pushed
the
merge_grad_gtest
branch
from
ef2d0fc
to
b487c62
Compare
luotao1
reviewed
Oct 17, 2017
paddle/gserver/tests/test_MKLDNN.cpp
Outdated
| std::vector<std::string> cases = {"conv_conv_concat", | ||
| "conv_conv_concat_32c", | ||
| "conv_conv_addto", | ||
| "conv_conv_addto_32c"}; |
There was a problem hiding this comment.
- XXX_32c的conf，和普通的conf有什么区别呢？从配置上怎么看出区别？
- PR的描述中写着：mkldnn_branches_xxx_32c.conf is for test cases that can surly use internal format. 但代码中没有这样的注释。用户不知道32c是做什么的？
- 新增的配置太多，这四个配置可以简化成两个：即outputs（concat, addto）。
- conv_conv_concat这个取名不是很好，请换一个。
There was a problem hiding this comment.
32c主要是为了测channel为32的时候，这样一般情况下可以测到mkldnn的内部格式。
这几个文件可以考虑合并下。
There was a problem hiding this comment.
这四个配置可以合并成一个：
- concat和addto, 利用outputs（concat, addto）合并成一个配置。
- 有无32c，利用get_config_args参数可以合并成一个配置。
|
|
||
| settings(batch_size=16) | ||
|
|
||
| data = data_layer(name ="input", size=32*16*16) |
There was a problem hiding this comment.
配置中可以加注释说明下，和普通的conf区别在哪里。
| @@ -411,4 +413,143 @@ void MKLDNNTester::run(const TestConfig& dnn, | |||
| } | |||
| } | |||
|
|
|||
| void MKLDNNTester::initArgument(DataIn& data, | |||
There was a problem hiding this comment.
initArgument，getOutResult和compareGradient，与test_NetworkCompare中的initArgument，calcGradient和compareGradient非常像，可以考虑复用下。
There was a problem hiding this comment.
是比较像，不过还是有很多不同点的：
- 参数
DataOut和DataIn的定义不一样。 - 这里考虑了iteration，默认跑 3个iteration，每个的输入数据是不一样的随机数。
test_NetworkCompare的20次，用的同样的随机数。 - 这里，最后比较的是forward的output value和backward且更新了，param的value。
- 关于
compareResult，最好还是用现在的方法，因为我考虑了，当data值都特别小的时候等等。
不过是可以考虑以后弄在一起。
There was a problem hiding this comment.
赞第二点中不同的随机数和第四点中data值特别小的情况。可以以后考虑优化test_NetworkCompare.cpp后，合并到一块。
tensor-tang
force-pushed
the
merge_grad_gtest
branch
from
cef4af3
to
60b8485
Compare
luotao1
approved these changes
Oct 17, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
if output has cpu layer, mkldnn layer should clear the grad.
mkldnn_branches_xxx_32c.confis for test cases that can surly use internal format.