[GitHub] zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN
zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN URL: https://github.com/apache/incubator-mxnet/pull/9677#discussion_r176614376 ## File path: src/operator/nn/convolution.cc ## @@ -168,10 +454,51 @@ There are other options to tune the performance. the performance. )code" ADD_FILELINE) +.set_num_inputs([](const NodeAttrs& attrs) { + const ConvolutionParam& params = nnvm::get(attrs.parsed); + return params.no_bias ? 2 : 3; +}) +.set_num_outputs(1) +.set_attr_parser(ConvolutionParamParser) +.set_attr("FListInputNames", +[](const NodeAttrs& attrs) { + const ConvolutionParam& params = nnvm::get(attrs.parsed); + if (params.no_bias) +return std::vector{"data", "weight"}; + else +return std::vector{"data", "weight", "bias"}; +}) +.set_attr("FInferShape", ConvolutionShape) Review comment: Thanks This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN
zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN URL: https://github.com/apache/incubator-mxnet/pull/9677#discussion_r168627930 ## File path: CMakeLists.txt ## @@ -610,7 +611,18 @@ endif() if(USE_OPENCV) add_executable(im2rec "tools/im2rec.cc") - target_link_libraries(im2rec ${BEGIN_WHOLE_ARCHIVE} mxnet ${END_WHOLE_ARCHIVE} ${mxnet_LINKER_LIBS} ${OpenCV_LIBS} dmlc) + if(MSVC) +target_link_libraries(im2rec mxnet) + else() +target_link_libraries(im2rec ${BEGIN_WHOLE_ARCHIVE} mxnet_static ${END_WHOLE_ARCHIVE}) + endif() + target_link_libraries(im2rec +${mxnet_LINKER_LIBS} +${OpenCV_LIBS} +dmlc +${nnvm_LINKER_LIBS} +${pslite_LINKER_LIBS} +) Review comment: @cjolivier01 could you explain why you added this? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN
zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN URL: https://github.com/apache/incubator-mxnet/pull/9677#discussion_r168627149 ## File path: prepare_mkldnn.sh ## @@ -0,0 +1,114 @@ +#!/bin/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 Review comment: This script was originally provided by Intel people. I didn't modify it afterwards. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN
zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN URL: https://github.com/apache/incubator-mxnet/pull/9677#discussion_r168584501 ## File path: tests/python/gpu/test_gluon_model_zoo_gpu.py ## @@ -0,0 +1,171 @@ +# 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. + +from __future__ import print_function +import mxnet as mx +import numpy as np +import copy +from mxnet import autograd +from mxnet.gluon.model_zoo.vision import get_model +from mxnet.test_utils import assert_almost_equal +import sys +import unittest + +def eprint(*args, **kwargs): +print(*args, file=sys.stderr, **kwargs) + +VAL_DATA='data/val-5k-256.rec' +def download_data(): +return mx.test_utils.download( +'http://data.mxnet.io/data/val-5k-256.rec', VAL_DATA) + +@unittest.skip("test fails intermittently. temporarily disabled.") +def test_inference(): +all_models = ['resnet50_v1', 'vgg19_bn', 'alexnet', #'inceptionv3', + 'densenet201', 'squeezenet1.0', 'mobilenet0.25'] + +batch_size = 10 +download_data() +for model_name in all_models: +eprint('testing inference on %s'%model_name) + +data_shape = (3, 224, 224) if 'inception' not in model_name else (3, 299, 299) +dataIter = mx.io.ImageRecordIter( +path_imgrec= VAL_DATA, +label_width= 1, +preprocess_threads = 1, +batch_size = batch_size, +data_shape = data_shape, +label_name = 'softmax_label', +rand_crop = False, +rand_mirror= False) +data_batch = dataIter.next() +data = data_batch.data[0] +label = data_batch.label[0] +gpu_data = data.as_in_context(mx.gpu()) +gpu_label = label.as_in_context(mx.gpu()) + +# This is to create a model and run the model once to initialize +# all parameters. +cpu_model = get_model(model_name) +cpu_model.collect_params().initialize(ctx=mx.cpu()) +cpu_model(mx.nd.array(data, ctx=mx.cpu())) +gpu_model = get_model(model_name) +gpu_model.collect_params().initialize(ctx=mx.gpu()) +gpu_model(mx.nd.array(data, ctx=mx.gpu())) + +# Force the two models have the same parameters. +cpu_params = cpu_model.collect_params() +gpu_params = gpu_model.collect_params() +for k in cpu_params.keys(): +k = k.replace(cpu_params.prefix, '') +cpu_param = cpu_params.get(k) +gpu_param = gpu_params.get(k) +gpu_param.set_data(cpu_param.data().as_in_context(mx.gpu())) + +# Run inference. +with autograd.record(train_mode=False): +cpu_out = cpu_model(mx.nd.array(data, ctx=mx.cpu())) +gpu_out = gpu_model(gpu_data) +out = cpu_out.asnumpy() +max_val = np.max(out) +assert_almost_equal(out / max_val, gpu_out.asnumpy() / max_val, rtol=1e-3, atol=1e-3) + +def get_nn_model(name): +if "densenet" in name: +return get_model(name, dropout=0) +else: +return get_model(name) + +def test_training(): +# We use network models without dropout for testing. +# TODO(zhengda) mobilenet can't pass this test even without MKLDNN. Review comment: I didn't investigate why the native MXNet CPU code diverges with the GPU yet. This shouldn't be fixed in this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN
zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN URL: https://github.com/apache/incubator-mxnet/pull/9677#discussion_r168584029 ## File path: tests/python/gpu/test_gluon_model_zoo_gpu.py ## @@ -0,0 +1,171 @@ +# 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. + +from __future__ import print_function +import mxnet as mx +import numpy as np +import copy +from mxnet import autograd +from mxnet.gluon.model_zoo.vision import get_model +from mxnet.test_utils import assert_almost_equal +import sys +import unittest + +def eprint(*args, **kwargs): +print(*args, file=sys.stderr, **kwargs) + +VAL_DATA='data/val-5k-256.rec' +def download_data(): +return mx.test_utils.download( +'http://data.mxnet.io/data/val-5k-256.rec', VAL_DATA) + +@unittest.skip("test fails intermittently. temporarily disabled.") Review comment: This test is added by me in this PR. It's provided as additional tests. I also added tests in the operator levels when running training and inference with the models in the model zoo, and these tests have passed. These end-to-end tests fail occasionally, most likely caused by numerical errors. It requires more investigations. I think this shouldn't be a reason of blocking the PR merge. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN
zheng-da commented on a change in pull request #9677: Refactor operators and add MKLDNN URL: https://github.com/apache/incubator-mxnet/pull/9677#discussion_r166135426 ## File path: tests/cpp/include/test_core_op.h ## @@ -209,6 +209,13 @@ class CoreOpExecutor : public test::op::OperatorDataInitializer requested.emplace_back(r); } else if (req.type == ResourceRequest::kRandom) { requested.emplace_back(ResourceManager::Get()->Request(ctx->run_ctx.ctx, req)); +} else if (req.type == ResourceRequest::kParallelRandom) { + Resource rm = ResourceManager::Get()->Request(ctx->run_ctx.ctx, req); + if (ctx->run_ctx.ctx.dev_mask() == Context::kCPU) { +common::random::RandGenerator::AllocState( Review comment: I don't know. I got the code from Chris. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services