[GitHub] [incubator-mxnet] rondogency commented on a change in pull request #17270: Dynamic custom operator GPU support

2020-01-26 Thread GitBox
rondogency commented on a change in pull request #17270: Dynamic custom 
operator GPU support
URL: https://github.com/apache/incubator-mxnet/pull/17270#discussion_r371045630
 
 

 ##
 File path: example/extensions/lib_custom_op/relu_lib.cu
 ##
 @@ -0,0 +1,193 @@
+/*
+ * 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.
+ */
+
+/*!
+ * Copyright (c) 2020 by Contributors
+ * \file relu_lib.cu
+ * \brief simple custom relu operator implemented using CUDA function
+ */
+
+#include 
+#include "lib_api.h"
+
+__global__ void relu_gpu_forward(float *out, float *in, int64_t N) {
+int tid = blockIdx.x * blockDim.x + threadIdx.x;
+if (tid < N)
+out[tid] = in[tid] > 0 ? in[tid] : 0;
+}
+
+__global__ void relu_gpu_backward(float *out, float *in, int64_t N) {
+int tid = blockIdx.x * blockDim.x + threadIdx.x;
+if (tid < N)
+out[tid] = in[tid] > 0 ? 1 : 0;
 
 Review comment:
   thanks for pointing out the error. I have made the fix


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-mxnet] rondogency commented on a change in pull request #17270: Dynamic custom operator GPU support

2020-01-26 Thread GitBox
rondogency commented on a change in pull request #17270: Dynamic custom 
operator GPU support
URL: https://github.com/apache/incubator-mxnet/pull/17270#discussion_r371045994
 
 

 ##
 File path: example/extensions/lib_custom_op/test_relu.py
 ##
 @@ -0,0 +1,69 @@
+#!/usr/bin/env python3
+
+# 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.
+
+# coding: utf-8
+# pylint: disable=arguments-differ
+
+# This test checks dynamic loading of custom library into MXNet
+# and checks end to end compute of a simple 2D gemm custom op
+
+import mxnet as mx
+import os
+import time
+
+#load library
+if (os.name=='posix'):
+path = os.path.abspath('librelu_lib.so')
+mx.library.load(path)
+
+a = mx.nd.array([[-2,-1],[1,2]], ctx=mx.cpu())
+b = mx.nd.array([[-2,-1],[1,2]], ctx=mx.gpu())
+
+print("start ndarray compute-")
+print(mx.nd.my_relu(a))
+print(mx.nd.my_relu(b))
+print(mx.nd.my_state_relu(a))
+print(mx.nd.my_state_relu(b))
+
+print("start symbolic compute")
+c = mx.sym.Variable('c')
+d = mx.sym.my_relu(c)
+in_grad = [mx.nd.empty((2,2), ctx=mx.gpu())]
+exe = d.bind(ctx=mx.gpu(), args={'c':b}, args_grad=in_grad)
+out = exe.forward()
+print(out)
+
+print("start backward compute")
+out_grad = mx.nd.ones((2,2), ctx=mx.gpu())
+exe.backward([out_grad])
+print(in_grad)
+
+print("start stress test-")
+a = mx.nd.uniform(shape=(1000,1000,100), ctx=mx.cpu())
+b = mx.nd.uniform(shape=(1000,1000,100), ctx=mx.gpu())
+t1 = time.time()
+r1 = mx.nd.my_relu(a)
+t2 = time.time()
+r2 = mx.nd.my_relu(b)
+t3 = time.time()
+print("CPU running time:")
+print(t2 - t1)
 
 Review comment:
   thanks for the pointing out the error. I didn't pay attention as we would do 
more tests including benchmark tests in the coming PRs. I have made the fix now.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-mxnet] rondogency commented on a change in pull request #17270: Dynamic custom operator GPU support

2020-01-29 Thread GitBox
rondogency commented on a change in pull request #17270: Dynamic custom 
operator GPU support
URL: https://github.com/apache/incubator-mxnet/pull/17270#discussion_r372604166
 
 

 ##
 File path: Makefile
 ##
 @@ -664,11 +664,19 @@ cpplint:
 pylint:
python3 -m pylint --rcfile=$(ROOTDIR)/ci/other/pylintrc 
--ignore-patterns=".*\.so$$,.*\.dll$$,.*\.dylib$$" python/mxnet
 
-# sample lib for MXNet extension dynamically loading custom operator
-sample_lib:
-   $(CXX) -shared -fPIC -std=c++11 
example/extensions/lib_custom_op/gemm_lib.cc -o libsample_lib.so -I 
include/mxnet
+# MXNet extension dynamically loading libraries
+EXT_LIBS = custom_op_lib subgraph_lib
+ifeq ($(USE_CUDA), 1)
+   EXT_LIBS += custom_op_gpu_lib
+endif
+extension_libs: $(EXT_LIBS)
+
+custom_op_lib:
+   $(CXX) -shared -fPIC -std=c++11 
example/extensions/lib_custom_op/gemm_lib.cc -o build/libcustomop_lib.so -I 
include/mxnet
+custom_op_gpu_lib:
+   $(NVCC) -shared -std=c++11 -Xcompiler -fPIC 
example/extensions/lib_custom_op/relu_lib.cu -o build/libcustomop_gpu_lib.so -I 
include/mxnet
 
 Review comment:
   it is a very small library simply for illustration purpose, so it doesn't 
necessarily use the existing NVCC and CUDA_ARCH flags used for compile MXNet


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-mxnet] rondogency commented on a change in pull request #17270: Dynamic custom operator GPU support

2020-01-29 Thread GitBox
rondogency commented on a change in pull request #17270: Dynamic custom 
operator GPU support
URL: https://github.com/apache/incubator-mxnet/pull/17270#discussion_r372699354
 
 

 ##
 File path: include/mxnet/lib_api.h
 ##
 @@ -203,6 +214,16 @@ enum MXDType {
   kUNSET = 100,
 };
 
+/*!
+ * \brief Context info passing from MXNet OpContext
+ * dev_type is string repr of supported context, currently only "cpu" and "gpu"
+ * dev_id is the device index where the tensor locates
+ */
+typedef struct {
+  std::string dev_type;
 
 Review comment:
   no, this string only serves as an easy way for custom library to check the 
device type, the actual context names passed from custom library to MXNet are 
still in const char *. Line 971 in lib_api.h is what we passed through the 
boundary


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-mxnet] rondogency commented on a change in pull request #17270: Dynamic custom operator GPU support

2020-01-29 Thread GitBox
rondogency commented on a change in pull request #17270: Dynamic custom 
operator GPU support
URL: https://github.com/apache/incubator-mxnet/pull/17270#discussion_r372700554
 
 

 ##
 File path: include/mxnet/lib_api.h
 ##
 @@ -203,6 +214,16 @@ enum MXDType {
   kUNSET = 100,
 };
 
+/*!
+ * \brief Context info passing from MXNet OpContext
+ * dev_type is string repr of supported context, currently only "cpu" and "gpu"
+ * dev_id is the device index where the tensor locates
+ */
+typedef struct {
+  std::string dev_type;
 
 Review comment:
   I will appreciate if you can make another review!


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-mxnet] rondogency commented on a change in pull request #17270: Dynamic custom operator GPU support

2020-01-29 Thread GitBox
rondogency commented on a change in pull request #17270: Dynamic custom 
operator GPU support
URL: https://github.com/apache/incubator-mxnet/pull/17270#discussion_r372729337
 
 

 ##
 File path: example/extensions/lib_custom_op/test_relu.py
 ##
 @@ -0,0 +1,83 @@
+#!/usr/bin/env python3
+
+# 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.
+
+# coding: utf-8
+# pylint: disable=arguments-differ
+
+# This test checks dynamic loading of custom library into MXNet
+# and checks end to end compute of a simple 2D gemm custom op
+
+import mxnet as mx
+import os
+import time
+
+#load library
+if (os.name=='posix'):
+path = os.path.abspath('librelu_lib.so')
+mx.library.load(path)
+
+a = mx.nd.array([[-2,-1],[1,2]], ctx=mx.cpu())
+b = mx.nd.array([[-2,-1],[1,2]], ctx=mx.gpu())
+
+print("start ndarray compute-")
+print(mx.nd.my_relu(a))
+print(mx.nd.my_relu(b))
+print(mx.nd.my_state_relu(a))
+print(mx.nd.my_state_relu(b))
+
+print("start symbolic compute")
+c = mx.sym.Variable('c')
+d = mx.sym.Variable('d')
+e = mx.sym.my_relu(c)
+base = mx.sym.relu(d)
+in_grad = [mx.nd.empty((2,2), ctx=mx.gpu())]
+in_grad_base = [mx.nd.empty((2,2), ctx=mx.gpu())]
+exe = e.bind(ctx=mx.gpu(), args={'c':b}, args_grad=in_grad)
+exe_base = base.bind(ctx=mx.gpu(), args={'d':b}, args_grad=in_grad_base)
+out = exe.forward()
+out_base = exe_base.forward()
+print(out)
+print(out_base)
+
+print("start backward compute")
+out_grad = mx.nd.ones((2,2), ctx=mx.gpu())
+exe.backward([out_grad])
+exe_base.backward([out_grad])
+print(in_grad)
+print(in_grad_base)
+
+print("start testing larger ndarray-")
+a = mx.nd.uniform(shape=(100,100,100), ctx=mx.cpu())
+b = mx.nd.uniform(shape=(100,100,100), ctx=mx.gpu())
 
 Review comment:
   thanks for approving the PR. I am thinking if we can merge the PR now, since 
we will do more benchmark tests in the coming PRs and it is a small example, so 
it's not worth the CI run.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-mxnet] rondogency commented on a change in pull request #17270: Dynamic custom operator GPU support

2020-01-30 Thread GitBox
rondogency commented on a change in pull request #17270: Dynamic custom 
operator GPU support
URL: https://github.com/apache/incubator-mxnet/pull/17270#discussion_r373161746
 
 

 ##
 File path: example/extensions/lib_custom_op/test_relu.py
 ##
 @@ -0,0 +1,83 @@
+#!/usr/bin/env python3
+
+# 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.
+
+# coding: utf-8
+# pylint: disable=arguments-differ
+
+# This test checks dynamic loading of custom library into MXNet
+# and checks end to end compute of a simple 2D gemm custom op
+
+import mxnet as mx
+import os
+import time
+
+#load library
+if (os.name=='posix'):
+path = os.path.abspath('librelu_lib.so')
+mx.library.load(path)
+
+a = mx.nd.array([[-2,-1],[1,2]], ctx=mx.cpu())
+b = mx.nd.array([[-2,-1],[1,2]], ctx=mx.gpu())
+
+print("start ndarray compute-")
+print(mx.nd.my_relu(a))
+print(mx.nd.my_relu(b))
+print(mx.nd.my_state_relu(a))
+print(mx.nd.my_state_relu(b))
+
+print("start symbolic compute")
+c = mx.sym.Variable('c')
+d = mx.sym.Variable('d')
+e = mx.sym.my_relu(c)
+base = mx.sym.relu(d)
+in_grad = [mx.nd.empty((2,2), ctx=mx.gpu())]
+in_grad_base = [mx.nd.empty((2,2), ctx=mx.gpu())]
+exe = e.bind(ctx=mx.gpu(), args={'c':b}, args_grad=in_grad)
+exe_base = base.bind(ctx=mx.gpu(), args={'d':b}, args_grad=in_grad_base)
+out = exe.forward()
+out_base = exe_base.forward()
+print(out)
+print(out_base)
+
+print("start backward compute")
+out_grad = mx.nd.ones((2,2), ctx=mx.gpu())
+exe.backward([out_grad])
+exe_base.backward([out_grad])
+print(in_grad)
+print(in_grad_base)
+
+print("start testing larger ndarray-")
+a = mx.nd.uniform(shape=(100,100,100), ctx=mx.cpu())
+b = mx.nd.uniform(shape=(100,100,100), ctx=mx.gpu())
 
 Review comment:
   I have made a PR on CustomOp GPU support doc #17486 and I will made this fix 
on the example in that PR as soon as this PR gets merged.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-mxnet] rondogency commented on a change in pull request #17270: Dynamic custom operator GPU support

2020-01-30 Thread GitBox
rondogency commented on a change in pull request #17270: Dynamic custom 
operator GPU support
URL: https://github.com/apache/incubator-mxnet/pull/17270#discussion_r373278868
 
 

 ##
 File path: include/mxnet/lib_api.h
 ##
 @@ -594,26 +657,58 @@ class CustomOp {
 mutate_inputs = func;
 return *this;
   }
-  CustomOp& setCreateOpState(createOpState_t func) {
-create_opstate = func;
+  CustomOp& setCreateOpState(createOpState_t func, const char* ctx) {
+if (create_op_ctx_map.count(ctx) > 0)
+  raiseDuplicateContextError();
+create_op_ctx_map[ctx] = func;
 return *this;
   }
   CustomOp& setIsSubgraphOp() {
 isSGop = true;
 return *this;
   }
+  void mapToVector() {
+for (auto kv : forward_ctx_map) {
+  forward_ctx_cstr.push_back(kv.first);
+  forward_fp.push_back(kv.second);
+}
+for (auto kv : backward_ctx_map) {
+  backward_ctx_cstr.push_back(kv.first);
+  backward_fp.push_back(kv.second);
+}
+for (auto kv : create_op_ctx_map) {
+  create_op_ctx_cstr.push_back(kv.first);
+  create_op_fp.push_back(kv.second);
+}
+  }
+  ~CustomOp() {}
 
   /*! \brief operator name */
   const char* name;
+
   /*! \brief operator functions */
-  fcomp_t forward;
-  fcomp_t backward;
   parseAttrs_t parse_attrs;
   inferType_t infer_type;
   inferShape_t infer_shape;
   mutateInputs_t mutate_inputs;
-  createOpState_t create_opstate;
   bool isSGop;
+
+  /*! \brief vector repr of ctx map to be easily loaded from c_api */
+  std::vector forward_ctx_cstr, backward_ctx_cstr, 
create_op_ctx_cstr;
+  std::vector forward_fp, backward_fp;
+  std::vector create_op_fp;
+
+ private:
+  void raiseDuplicateContextError() {
+std::string op_name_str(name);
+throw std::runtime_error(
+  "Error! Error! Cannot register multiple functions under same context for 
operator '"
+  + op_name_str + "'");
+  }
+
+  /*! \brief dedup context maps - static string ctx to custom function */
+  std::unordered_map forward_ctx_map, backward_ctx_map;
 
 Review comment:
   This map is used for dedup inside each library, and if loading multiple 
libraries it would be different operators, since for each custom library there 
will be one registry object created and custom operators are registered 
individually in each library.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-mxnet] rondogency commented on a change in pull request #17270: Dynamic custom operator GPU support

2020-02-04 Thread GitBox
rondogency commented on a change in pull request #17270: Dynamic custom 
operator GPU support
URL: https://github.com/apache/incubator-mxnet/pull/17270#discussion_r374977594
 
 

 ##
 File path: example/extensions/lib_custom_op/test_relu.py
 ##
 @@ -0,0 +1,83 @@
+#!/usr/bin/env python3
+
+# 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.
+
+# coding: utf-8
+# pylint: disable=arguments-differ
+
+# This test checks dynamic loading of custom library into MXNet
+# and checks end to end compute of a simple 2D gemm custom op
+
+import mxnet as mx
+import os
+import time
+
+#load library
+if (os.name=='posix'):
+path = os.path.abspath('librelu_lib.so')
+mx.library.load(path)
+
+a = mx.nd.array([[-2,-1],[1,2]], ctx=mx.cpu())
+b = mx.nd.array([[-2,-1],[1,2]], ctx=mx.gpu())
+
+print("start ndarray compute-")
+print(mx.nd.my_relu(a))
+print(mx.nd.my_relu(b))
+print(mx.nd.my_state_relu(a))
+print(mx.nd.my_state_relu(b))
+
+print("start symbolic compute")
+c = mx.sym.Variable('c')
+d = mx.sym.Variable('d')
+e = mx.sym.my_relu(c)
+base = mx.sym.relu(d)
+in_grad = [mx.nd.empty((2,2), ctx=mx.gpu())]
+in_grad_base = [mx.nd.empty((2,2), ctx=mx.gpu())]
+exe = e.bind(ctx=mx.gpu(), args={'c':b}, args_grad=in_grad)
+exe_base = base.bind(ctx=mx.gpu(), args={'d':b}, args_grad=in_grad_base)
+out = exe.forward()
+out_base = exe_base.forward()
+print(out)
+print(out_base)
+
+print("start backward compute")
+out_grad = mx.nd.ones((2,2), ctx=mx.gpu())
+exe.backward([out_grad])
+exe_base.backward([out_grad])
+print(in_grad)
+print(in_grad_base)
+
+print("start testing larger ndarray-")
+a = mx.nd.uniform(shape=(100,100,100), ctx=mx.cpu())
+b = mx.nd.uniform(shape=(100,100,100), ctx=mx.gpu())
 
 Review comment:
   The fix is in #17516 along with one more fix


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:
us...@infra.apache.org


With regards,
Apache Git Services