Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[Example]Fix mlp_csv example #13273

Merged
merged 30 commits into from
Nov 27, 2018
Merged

[Example]Fix mlp_csv example #13273

merged 30 commits into from
Nov 27, 2018

Conversation

stu1130
Copy link
Contributor

@stu1130 stu1130 commented Nov 14, 2018 •
edited
Loading

Description

  1. Fix the README which contain the typo
  2. add instructions for users to get the data for running the example

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

N/A

Comments

@leleamol

@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
a9d40c4
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
429684a
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
602b7d0
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
85f9408
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
5a01234
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
8f58746
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
1531594
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
3e799de
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
f690c69
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
a091a97
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
f1359be
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
e360853
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
23199d1
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
166c2b6
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
d01e8cb
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
0454017
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
90f1dd8
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
2f019cd
@stu1130
Merge branch 'master' of https://github.com/apache/incubator-mxnet
ec075f5
@stu1130
add instruction to get the data and fix typo
4581acb
@stu1130
fix typo
0d4ecde
@stu1130
update file name
f40079d
@stu1130 stu1130 requested a review from nswamy as a code owner November 14, 2018 22:54
@stu1130 stu1130 changed the title Fix mlp_csv example [Example]Fix mlp_csv example Nov 14, 2018
kalyc
kalyc reviewed Nov 15, 2018
View reviewed changes
cpp-package/example/README.md
* To get the `mnist_training_set.csv` and `mnist_test_set.csv` please run the following command:
```python
# in incubator-mxnet/cpp-package/example directory
python mnist_to_csv.py ./data/mnist_data/train-images-idx3-ubyte ./data/mnist_data/train-labels-idx1-ubyte ./data/mnist_data/mnist_train.csv 60000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have instructions on how to get train-images & train-labels locally?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you run the mnist_to_csv.py following the instruction above you get the training.csv and testing.csv which contain both data and labels

@kalyc
Copy link
Contributor

kalyc commented Nov 15, 2018

mxnet-label-bot add [pr-awaiting-merge]

roywei
roywei reviewed Nov 16, 2018
View reviewed changes
Copy link
Member

@roywei roywei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this to ci_tests.sh?

@kalyc
Copy link
Contributor

kalyc commented Nov 16, 2018

@mxnet-label-bot add [pr-awaiting-testing]

@marcoabreu marcoabreu added the pr-awaiting-testing PR is reviewed and waiting CI build and test label Nov 16, 2018
@stu1130 stu1130 requested a review from marcoabreu as a code owner November 19, 2018 22:30
roywei
roywei approved these changes Nov 20, 2018
View reviewed changes
Copy link
Member

@roywei roywei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stu1130
Copy link
Contributor Author

stu1130 commented Nov 20, 2018

@mxnet-label-bot update [pr-awaiting-merge]

@marcoabreu marcoabreu added pr-awaiting-merge Review and CI is complete. Ready to Merge and removed pr-awaiting-testing PR is reviewed and waiting CI build and test labels Nov 20, 2018
@roywei
Copy link
Member

roywei commented Nov 20, 2018

@marcoabreu according to dev list discussion, this PR will be blocked by #13344 as it's changing Jenkins file

nswamy
nswamy reviewed Nov 21, 2018
View reviewed changes
Jenkinsfile Outdated
@@ -35,7 +35,7 @@ mx_cmake_lib_debug = 'build/libmxnet.so, build/libmxnet.a, build/3rdparty/dmlc-c
mx_cmake_mkldnn_lib = 'build/libmxnet.so, build/libmxnet.a, build/3rdparty/dmlc-core/libdmlc.a, build/tests/mxnet_unit_tests, build/3rdparty/openmp/runtime/src/libomp.so, build/3rdparty/mkldnn/src/libmkldnn.so.0'
mx_mkldnn_lib = 'lib/libmxnet.so, lib/libmxnet.a, lib/libiomp5.so, lib/libmkldnn.so.0, lib/libmklml_intel.so, 3rdparty/dmlc-core/libdmlc.a, 3rdparty/tvm/nnvm/lib/libnnvm.a'
mx_tensorrt_lib = 'lib/libmxnet.so, lib/libnvonnxparser_runtime.so.0, lib/libnvonnxparser.so.0, lib/libonnx_proto.so, lib/libonnx.so'
mx_lib_cpp_examples = 'lib/libmxnet.so, lib/libmxnet.a, 3rdparty/dmlc-core/libdmlc.a, 3rdparty/tvm/nnvm/lib/libnnvm.a, 3rdparty/ps-lite/build/libps.a, deps/lib/libprotobuf-lite.a, deps/lib/libzmq.a, build/cpp-package/example/lenet, build/cpp-package/example/alexnet, build/cpp-package/example/googlenet, build/cpp-package/example/lenet_with_mxdataiter, build/cpp-package/example/resnet, build/cpp-package/example/mlp, build/cpp-package/example/mlp_cpu, build/cpp-package/example/mlp_gpu, build/cpp-package/example/test_score, build/cpp-package/example/test_optimizer'
mx_lib_cpp_examples = 'lib/libmxnet.so, lib/libmxnet.a, 3rdparty/dmlc-core/libdmlc.a, 3rdparty/tvm/nnvm/lib/libnnvm.a, 3rdparty/ps-lite/build/libps.a, deps/lib/libprotobuf-lite.a, deps/lib/libzmq.a, build/cpp-package/example/lenet, build/cpp-package/example/alexnet, build/cpp-package/example/googlenet, build/cpp-package/example/lenet_with_mxdataiter, build/cpp-package/example/resnet, build/cpp-package/example/mlp, build/cpp-package/example/mlp_cpu, build/cpp-package/example/mlp_gpu, build/cpp-package/example/mlp_csv, build/cpp-package/example/test_score, build/cpp-package/example/test_optimizer'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move this to a different PR

@szha szha added pr-awaiting-review PR is waiting for code review and removed pr-awaiting-merge Review and CI is complete. Ready to Merge labels Nov 21, 2018
@vandanavk
Copy link
Contributor

@mxnet-label-bot update [pr-awaiting-merge].

The Jenkinsfile change has been removed from this PR

@marcoabreu marcoabreu added pr-awaiting-merge Review and CI is complete. Ready to Merge and removed pr-awaiting-review PR is waiting for code review labels Nov 27, 2018
@nswamy nswamy merged commit 4f8aa09 into apache:master Nov 27, 2018
@stu1130 stu1130 deleted the fix_mlp_csv_example branch February 7, 2020 04:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nswamy nswamy nswamy left review comments

@marcoabreu marcoabreu marcoabreu approved these changes

@kalyc kalyc kalyc approved these changes

@roywei roywei roywei approved these changes

Assignees
No one assigned
Labels
pr-awaiting-merge Review and CI is complete. Ready to Merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@stu1130 @kalyc @roywei @vandanavk @nswamy @marcoabreu @szha