[GitHub] [incubator-mxnet] leezu commented on a change in pull request #20131: [2.0] Add cpp-package

[GitBox <gi...@apache.org>]

leezu commented on a change in pull request #20131:
URL: https://github.com/apache/incubator-mxnet/pull/20131#discussion_r630574196



##########
File path: cpp-package/tests/travis/run_test.sh
##########
@@ -0,0 +1,42 @@
+#!/bin/bash

Review comment:
       Please don't add unused files

##########
File path: cpp-package/tests/ci_test.sh
##########
@@ -0,0 +1,79 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -e # exit on the first error
+cd $(dirname $(readlink -f $0))/../example
+echo $PWD
+export LD_LIBRARY_PATH=$(readlink -f /work/build):$LD_LIBRARY_PATH
+echo $LD_LIBRARY_PATH
+ls -l /work/build/
+
+./get_data.sh
+
+cp /work/build/cpp-package/example/lenet .
+./lenet 1
+
+cp /work/build/cpp-package/example/alexnet .
+./alexnet 1
+
+cp /work/build/cpp-package/example/lenet_with_mxdataiter .
+./lenet_with_mxdataiter 1
+
+cp /work/build/cpp-package/example/resnet .
+./resnet 1
+
+cp /work/build/cpp-package/example/inception_bn .
+./inception_bn 1
+
+cp /work/build/cpp-package/example/mlp .
+./mlp 150
+
+cp /work/build/cpp-package/example/mlp_cpu .
+./mlp_cpu
+
+cp /work/build/cpp-package/example/mlp_gpu .
+./mlp_gpu
+
+cp /work/build/cpp-package/example/test_optimizer .
+./test_optimizer
+
+cp /work/build/cpp-package/example/test_kvstore .
+./test_kvstore
+
+cp /work/build/cpp-package/example/test_score .
+./test_score 0.93
+
+cp /work/build/cpp-package/example/test_ndarray_copy .
+./test_ndarray_copy
+
+# skippping temporarily, tracked by https://github.com/apache/incubator-mxnet/issues/20011
+#cp ../../build/cpp-package/example/test_regress_label .
+#./test_regress_label
+
+# sh unittests/unit_test_mlp_csv.sh

Review comment:
       Have you tried running?

##########
File path: cpp-package/README.md
##########
@@ -0,0 +1,67 @@
+<!--- Licensed to the Apache Software Foundation (ASF) under one -->
+<!--- or more contributor license agreements.  See the NOTICE file -->
+<!--- distributed with this work for additional information -->
+<!--- regarding copyright ownership.  The ASF licenses this file -->
+<!--- to you under the Apache License, Version 2.0 (the -->
+<!--- "License"); you may not use this file except in compliance -->
+<!--- with the License.  You may obtain a copy of the License at -->
+
+<!---   http://www.apache.org/licenses/LICENSE-2.0 -->
+
+<!--- Unless required by applicable law or agreed to in writing, -->
+<!--- software distributed under the License is distributed on an -->
+<!--- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -->
+<!--- KIND, either express or implied.  See the License for the -->
+<!--- specific language governing permissions and limitations -->
+<!--- under the License. -->
+
+# MXNet - C++ API
+
+The MXNet C++ Package provides C++ API bindings to the users of MXNet.  Currently, these bindings are not available as standalone package.
+The users of these bindings are required to build this package as mentioned below.
+
+## Building C++ Package
+
+The cpp-package directory contains the implementation of C++ API. As mentioned above, users are required to build this directory or package before using it.
+**The cpp-package is built while building the MXNet shared library, *libmxnet.so*.**

Review comment:
       There is no reason for that. cpp-package depends only on libmxnet.so C APIs. It would be better to keep a separate CMakeLists.txt for the cpp-package with the only requirement to find libmxnet.so. This implies removing the `USE_CPP_PACKAGE` in the main CMakeLists.txt

##########
File path: cpp-package/include/mxnet-cpp/.gitignore
##########
@@ -0,0 +1,2 @@
+# Rebuildable file(s)
+op.h

Review comment:
       Why is op.h placed inside the build dir? Usually we only support out of source build in cmake

##########
File path: src/operator/softmax_output-inl.h
##########
@@ -0,0 +1,476 @@
+/*

Review comment:
       These files were removed in https://github.com/apache/incubator-mxnet/pull/18531
   What's the intention of adding them back? If you like to reconsider the plan for supporting the Module API in MXNet 2, why just add back SoftmaxOutput and not the other loss function? Should this be discussed in the https://github.com/apache/incubator-mxnet/issues/16167 RFC (ie. providing an update that you want to support the Module API?)

##########
File path: ci/docker/runtime_functions.sh
##########
@@ -625,12 +625,16 @@ build_ubuntu_gpu_onednn_nocudnn() {
 build_ubuntu_gpu() {
     set -ex
     cd /work/build
+    ln -s -f /usr/local/cuda/targets/x86_64-linux/lib/stubs/libcuda.so libcuda.so.1
+    export LIBRARY_PATH=${LIBRARY_PATH}:/work/build
+    export LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:/work/build

Review comment:
       This should be removed after https://github.com/apache/incubator-mxnet/issues/17858
   Would you add a comment pointing out the temporary nature of this?

##########
File path: cpp-package/example/inference/unit_test_imagenet_inference.sh
##########
@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -ex
+# create ./model directory if not existed
+if [ ! -d model ]; then
+    mkdir -p model
+fi
+# create ./data directory if not existed
+if [ ! -d data ]; then
+    mkdir -p data
+fi
+# Downloading the data and model if not existed
+model_file=./model/Inception-BN-symbol.json
+params_file=./model/Inception-BN-0126.params
+if [ ! -f ${model_file} ] || [ ! -f ${params_file} ]; then
+    wget -nc http://data.mxnet.io/models/imagenet/inception-bn.tar.gz
+    tar -xvzf inception-bn.tar.gz -C model
+fi
+cd model
+wget -nc https://raw.githubusercontent.com/dmlc/gluon-cv/master/gluoncv/model_zoo/quantized/resnet50_v1_int8-symbol.json
+cd ../data
+wget -nc http://data.mxnet.io/data/val_256_q90.rec
+cd ..
+
+# Running inference on imagenet.
+if [ "$(uname)" == "Darwin" ]; then
+    echo ">>> INFO: FP32 real data"
+    DYLD_LIBRARY_PATH=${DYLD_LIBRARY_PATH}:../../../lib ./imagenet_inference --symbol_file "./model/Inception-BN-symbol.json" --params_file "./model/Inception-BN-0126.params" --dataset "./data/val_256_q90.rec" --rgb_mean "123.68 116.779 103.939" --batch_size 1 --num_skipped_batches 50 --num_inference_batches 500
+
+    echo ">>> INFO: FP32 dummy data"
+    DYLD_LIBRARY_PATH=${DYLD_LIBRARY_PATH}:../../../lib ./imagenet_inference --symbol_file "./model/Inception-BN-symbol.json" --batch_size 1 --num_inference_batches 500 --benchmark
+else
+    echo ">>> INFO: FP32 real data"
+    LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:../../../lib ./imagenet_inference --symbol_file "./model/Inception-BN-symbol.json" --params_file "./model/Inception-BN-0126.params" --dataset "./data/val_256_q90.rec" --rgb_mean "123.68 116.779 103.939" --batch_size 1 --num_skipped_batches 50 --num_inference_batches 500
+
+    echo ">>> INFO: FP32 dummy data"
+    LD_LIBRARY_PATH=${LD_LIBRARY_PATH}:../../../lib ./imagenet_inference --symbol_file "./model/Inception-BN-symbol.json" --batch_size 1 --num_inference_batches 500 --benchmark
+
+    lib_name=$(ls -a ../../../lib | grep -oE 'mkldnn' | tail -1)

Review comment:
       You need to check this. Likely should be onednn




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org