-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[Example] fix cpp example inception-bn and training acc issue #13284
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the rest LGTM
@@ -172,7 +173,13 @@ int main(int argc, char const *argv[]) { | |||
auto val_iter = MXDataIter("MNISTIter"); | |||
setDataIter(&val_iter, "Label", data_files, batch_size); | |||
|
|||
Optimizer* opt = OptimizerRegistry::Find("ccsgd"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any reason to change the optimizer?
cpp-package/tests/ci_test.sh
Outdated
@@ -36,6 +36,9 @@ cp ../../build/cpp-package/example/lenet_with_mxdataiter . | |||
cp ../../build/cpp-package/example/resnet . | |||
./resnet 5 | |||
|
|||
cp ../../build/cpp-package/example/resnet . | |||
./inception-bn 5 | |||
|
|||
cp ../../build/cpp-package/example/mlp . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also add the mlp_csv example here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stu1130 ccsgd
was deprecated long time ago, actually, I will update to change ccsgd
in all other examples.
@roywei please take a look at the failed CI and re-trigger it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@mxnet-label-bot add [pr-awaiting-review] |
@marcoabreu according to dev list discussion, this PR will be blocked by #13344 as it's changing Jenkins file |
Description
Fix: [cpp-package] inception_bn.cpp is wrong #9417
Root Cause:
Passing correct padding (1, 1) in Pooling in InceptionFactoryB
Reference python implementation,
pad=(1, 1)
Tested example working fine.
fix some models not training: CPP examples training acc does not increase #13243, Strange behaviour/possible bug with mxnet::cpp::Symbol::Variable name argument #12966, [C++] Some Variable Name cause no Gradient #8108
Root Cause:
Wrong logic for gradient requirement: symbols with name length 3 and 4 won't have gradients
Changed to:
Symbols names contains "data" or "label" should not be required for gradients
Added parameter initialization same as python versions
added inception-bn to unit tests
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
see description
Comments