From f90ffaaad1ae2a1f2abde6964bd813344e7fc37f Mon Sep 17 00:00:00 2001 From: Lin Yuan Date: Wed, 22 Aug 2018 09:49:51 -0700 Subject: [PATCH 1/8] Fix the dtype mismatch in derived _zeros node --- src/executor/infer_graph_attr_pass.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/executor/infer_graph_attr_pass.cc b/src/executor/infer_graph_attr_pass.cc index 0abee04b59d5..49c4ec46c99c 100644 --- a/src/executor/infer_graph_attr_pass.cc +++ b/src/executor/infer_graph_attr_pass.cc @@ -254,7 +254,8 @@ nnvm::Graph InferAttr(nnvm::Graph &&ret, dispatch_mode = &dispatch_modes[nid]; if (dispatch_modes[nid] == DispatchMode::kUndefined) forward_known = false; } - auto finfer = finfer_shape.get(inode.source->op(), fdefault); + auto finfer = (inode.source->op() == Op::Get("_zeros")) ? fdefault : + finfer_shape.get(inode.source->op(), fdefault); if (!forward_known) { if (finfer != nullptr) { // Call inference function of the operator. From a7da018d2b51980a14bc24f82281c7a19fe86c4b Mon Sep 17 00:00:00 2001 From: Lin Yuan Date: Wed, 22 Aug 2018 10:17:35 -0700 Subject: [PATCH 2/8] Add unittest for infer dtype --- tests/python/unittest/test_infer_type.py | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 tests/python/unittest/test_infer_type.py diff --git a/tests/python/unittest/test_infer_type.py b/tests/python/unittest/test_infer_type.py new file mode 100644 index 000000000000..6fccf4eae173 --- /dev/null +++ b/tests/python/unittest/test_infer_type.py @@ -0,0 +1,36 @@ +# 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. + +# pylint: skip-file +import mxnet as mx +import numpy as np +from common import models +from mxnet import autograd +from nose.tools import * + +def test_infer_multiout_op(): + data = mx.nd.arange(16, dtype='float64').reshape((4, 4)) + data.attach_grad() + + with autograd.record(): + y = mx.nd.split(data, axis=0, num_outputs=2) + y[0].backward() + assert data.grad.dtype == np.float64 + + +if __name__ == "__main__": + test_infer_multiout_op() \ No newline at end of file From 1f15b4da1e80449f872e6611054fb2961e9462dc Mon Sep 17 00:00:00 2001 From: Lin Yuan Date: Wed, 22 Aug 2018 11:10:02 -0700 Subject: [PATCH 3/8] Add one more unit test --- tests/python/unittest/test_infer_type.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/python/unittest/test_infer_type.py b/tests/python/unittest/test_infer_type.py index 6fccf4eae173..db90dfce4230 100644 --- a/tests/python/unittest/test_infer_type.py +++ b/tests/python/unittest/test_infer_type.py @@ -31,6 +31,25 @@ def test_infer_multiout_op(): y[0].backward() assert data.grad.dtype == np.float64 +def test_infer_multiout_op2(): + def test_func(a): + q, l = mx.nd.linalg.gelqf(a) + return mx.nd.sum(l) + + data32 = mx.nd.random.normal(shape=(2, 3), ctx=mx.cpu(), dtype='float32') + data32.attach_grad() + with autograd.record(): + test32 = test_func(data32) + test32.backward() + + data64 = mx.nd.Cast(data32, dtype='float64') + data64.attach_grad() + with autograd.record(): + test64 = test_func(data64) + test64.backward() + assert_almost_equal(data64.grad.asnumpy().all(), data32.grad.asnumpy().all()) + if __name__ == "__main__": - test_infer_multiout_op() \ No newline at end of file + test_infer_multiout_op() + test_infer_multiout_op2() \ No newline at end of file From 1677bd0f8016f4a9b02b7c66b11b1a9b931cc75c Mon Sep 17 00:00:00 2001 From: Lin Yuan Date: Wed, 22 Aug 2018 11:22:31 -0700 Subject: [PATCH 4/8] Add nose runmodule --- tests/python/unittest/test_infer_type.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/python/unittest/test_infer_type.py b/tests/python/unittest/test_infer_type.py index db90dfce4230..15795515247b 100644 --- a/tests/python/unittest/test_infer_type.py +++ b/tests/python/unittest/test_infer_type.py @@ -23,7 +23,7 @@ from nose.tools import * def test_infer_multiout_op(): - data = mx.nd.arange(16, dtype='float64').reshape((4, 4)) + data = mx.nd.arange(16, dtype=np.float64).reshape((4, 4)) data.attach_grad() with autograd.record(): @@ -36,13 +36,13 @@ def test_func(a): q, l = mx.nd.linalg.gelqf(a) return mx.nd.sum(l) - data32 = mx.nd.random.normal(shape=(2, 3), ctx=mx.cpu(), dtype='float32') + data32 = mx.nd.random.normal(shape=(2, 3), ctx=mx.cpu(), dtype=np.float32) data32.attach_grad() with autograd.record(): test32 = test_func(data32) test32.backward() - data64 = mx.nd.Cast(data32, dtype='float64') + data64 = mx.nd.Cast(data32, dtype=np.float64) data64.attach_grad() with autograd.record(): test64 = test_func(data64) @@ -50,6 +50,6 @@ def test_func(a): assert_almost_equal(data64.grad.asnumpy().all(), data32.grad.asnumpy().all()) -if __name__ == "__main__": - test_infer_multiout_op() - test_infer_multiout_op2() \ No newline at end of file +if __name__ == '__main__': + import nose + nose.runmodule() \ No newline at end of file From dcc5f7877ec1fad7ad4c1f3d2ab3a3c5f52909ca Mon Sep 17 00:00:00 2001 From: Lin Yuan Date: Tue, 11 Sep 2018 23:20:06 -0700 Subject: [PATCH 5/8] Add a zero operator with no default dtype --- src/executor/graph_executor.cc | 4 ++-- src/executor/infer_graph_attr_pass.cc | 3 +-- src/operator/tensor/init_op.cc | 12 ++++++++++++ src/operator/tensor/init_op.cu | 3 +++ src/operator/tensor/init_op.h | 18 ++++++++++++++++++ 5 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/executor/graph_executor.cc b/src/executor/graph_executor.cc index 265554ab3918..3bbf57f11216 100644 --- a/src/executor/graph_executor.cc +++ b/src/executor/graph_executor.cc @@ -141,8 +141,8 @@ nnvm::NodeEntry AggregateGradient(std::vector&& v) { if (v.empty()) { nnvm::NodePtr ng = nnvm::Node::Create(); - ng->attrs.op = zeros_op; - ng->attrs.name = "zeros"; + ng->attrs.op = Op::Get("_zeros_no_default"); + ng->attrs.name = "zeros_no_default"; ng->attrs.op->attr_parser(&(ng->attrs)); return nnvm::NodeEntry{ng, 0, 0}; } diff --git a/src/executor/infer_graph_attr_pass.cc b/src/executor/infer_graph_attr_pass.cc index 49c4ec46c99c..0abee04b59d5 100644 --- a/src/executor/infer_graph_attr_pass.cc +++ b/src/executor/infer_graph_attr_pass.cc @@ -254,8 +254,7 @@ nnvm::Graph InferAttr(nnvm::Graph &&ret, dispatch_mode = &dispatch_modes[nid]; if (dispatch_modes[nid] == DispatchMode::kUndefined) forward_known = false; } - auto finfer = (inode.source->op() == Op::Get("_zeros")) ? fdefault : - finfer_shape.get(inode.source->op(), fdefault); + auto finfer = finfer_shape.get(inode.source->op(), fdefault); if (!forward_known) { if (finfer != nullptr) { // Call inference function of the operator. diff --git a/src/operator/tensor/init_op.cc b/src/operator/tensor/init_op.cc index bb23f5d44f64..bd9e1fb89bea 100644 --- a/src/operator/tensor/init_op.cc +++ b/src/operator/tensor/init_op.cc @@ -30,9 +30,21 @@ namespace op { DMLC_REGISTER_PARAMETER(InitOpParam); DMLC_REGISTER_PARAMETER(InitOpWithScalarParam); +DMLC_REGISTER_PARAMETER(InitOpNoDefaultParam); DMLC_REGISTER_PARAMETER(RangeParam); DMLC_REGISTER_PARAMETER(EyeParam); +NNVM_REGISTER_OP(_zeros_no_default) +.describe("fill target with zeros with no default type") +.set_num_inputs(0) +.set_num_outputs(1) +.set_attr_parser(ParamParser) +.set_attr("FInferShape", InitShape) +.set_attr("FInferType", InitType) +.set_attr("FInferStorageType", InitStorageType) +.set_attr("FCompute", FillCompute) +.set_attr("FComputeEx", FillComputeZerosEx) +.add_arguments(InitOpNoDefaultParam::__FIELDS__()); NNVM_REGISTER_OP(_zeros) .describe("fill target with zeros") diff --git a/src/operator/tensor/init_op.cu b/src/operator/tensor/init_op.cu index 81d835ee3bd2..11e6090757b4 100644 --- a/src/operator/tensor/init_op.cu +++ b/src/operator/tensor/init_op.cu @@ -44,6 +44,9 @@ void FillZerosCsrImpl(mshadow::Stream *s, const NDArray& dst) { }); } +NNVM_REGISTER_OP(_zeros_no_default) +.set_attr("FCompute", FillCompute) +.set_attr("FComputeEx", FillComputeZerosEx); NNVM_REGISTER_OP(_zeros) .set_attr("FCompute", FillCompute) diff --git a/src/operator/tensor/init_op.h b/src/operator/tensor/init_op.h index 304911a02a78..8069223f1665 100644 --- a/src/operator/tensor/init_op.h +++ b/src/operator/tensor/init_op.h @@ -61,6 +61,24 @@ struct InitOpParam : public dmlc::Parameter { } }; +struct InitOpNoDefaultParam : public dmlc::Parameter { + TShape shape; + std::string ctx; + int dtype; + DMLC_DECLARE_PARAMETER(InitOpNoDefaultParam) { + DMLC_DECLARE_FIELD(shape) + .set_default(TShape()) + .describe("The shape of the output"); + DMLC_DECLARE_FIELD(ctx) + .set_default("") + .describe("Context of output, in format [cpu|gpu|cpu_pinned](n)." + "Only used for imperative calls."); + DMLC_DECLARE_FIELD(dtype) + .set_default(-1) + .describe("Target data type."); + } +}; + struct EyeParam : public dmlc::Parameter { nnvm::dim_t N; nnvm::dim_t M; From 9c09a2e21c4372bedbd71db2380bfd07bea3ee71 Mon Sep 17 00:00:00 2001 From: Lin Yuan Date: Wed, 12 Sep 2018 09:59:26 -0700 Subject: [PATCH 6/8] Rename variables --- src/executor/graph_executor.cc | 4 ++-- src/operator/tensor/init_op.cc | 17 +++++++++-------- src/operator/tensor/init_op.h | 4 ++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/executor/graph_executor.cc b/src/executor/graph_executor.cc index 3bbf57f11216..54a8d224ff42 100644 --- a/src/executor/graph_executor.cc +++ b/src/executor/graph_executor.cc @@ -141,8 +141,8 @@ nnvm::NodeEntry AggregateGradient(std::vector&& v) { if (v.empty()) { nnvm::NodePtr ng = nnvm::Node::Create(); - ng->attrs.op = Op::Get("_zeros_no_default"); - ng->attrs.name = "zeros_no_default"; + ng->attrs.op = Op::Get("_zeros_without_dtype"); + ng->attrs.name = "zeros_without_dtype"; ng->attrs.op->attr_parser(&(ng->attrs)); return nnvm::NodeEntry{ng, 0, 0}; } diff --git a/src/operator/tensor/init_op.cc b/src/operator/tensor/init_op.cc index bd9e1fb89bea..8554ba854178 100644 --- a/src/operator/tensor/init_op.cc +++ b/src/operator/tensor/init_op.cc @@ -30,21 +30,22 @@ namespace op { DMLC_REGISTER_PARAMETER(InitOpParam); DMLC_REGISTER_PARAMETER(InitOpWithScalarParam); -DMLC_REGISTER_PARAMETER(InitOpNoDefaultParam); +DMLC_REGISTER_PARAMETER(InitOpWithoutDTypeParam); DMLC_REGISTER_PARAMETER(RangeParam); DMLC_REGISTER_PARAMETER(EyeParam); -NNVM_REGISTER_OP(_zeros_no_default) -.describe("fill target with zeros with no default type") +NNVM_REGISTER_OP(_zeros_without_dtype) +.describe("fill target with zeros without default dtype") .set_num_inputs(0) .set_num_outputs(1) -.set_attr_parser(ParamParser) -.set_attr("FInferShape", InitShape) -.set_attr("FInferType", InitType) -.set_attr("FInferStorageType", InitStorageType) +.set_attr_parser(ParamParser) +.set_attr("FInferShape", InitShape) +.set_attr("FInferType", InitType) +.set_attr("FInferStorageType", + InitStorageType) .set_attr("FCompute", FillCompute) .set_attr("FComputeEx", FillComputeZerosEx) -.add_arguments(InitOpNoDefaultParam::__FIELDS__()); +.add_arguments(InitOpWithoutDTypeParam::__FIELDS__()); NNVM_REGISTER_OP(_zeros) .describe("fill target with zeros") diff --git a/src/operator/tensor/init_op.h b/src/operator/tensor/init_op.h index 8069223f1665..1a4790acdb26 100644 --- a/src/operator/tensor/init_op.h +++ b/src/operator/tensor/init_op.h @@ -61,11 +61,11 @@ struct InitOpParam : public dmlc::Parameter { } }; -struct InitOpNoDefaultParam : public dmlc::Parameter { +struct InitOpWithoutDTypeParam : public dmlc::Parameter { TShape shape; std::string ctx; int dtype; - DMLC_DECLARE_PARAMETER(InitOpNoDefaultParam) { + DMLC_DECLARE_PARAMETER(InitOpWithoutDTypeParam) { DMLC_DECLARE_FIELD(shape) .set_default(TShape()) .describe("The shape of the output"); From b979ce5d91f292278c357e96a3c9149e47346ad8 Mon Sep 17 00:00:00 2001 From: Lin Yuan Date: Wed, 12 Sep 2018 16:49:27 -0700 Subject: [PATCH 7/8] fix a bug: rename operator for gpu --- src/operator/tensor/init_op.cu | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/operator/tensor/init_op.cu b/src/operator/tensor/init_op.cu index 11e6090757b4..902b567516bd 100644 --- a/src/operator/tensor/init_op.cu +++ b/src/operator/tensor/init_op.cu @@ -44,7 +44,7 @@ void FillZerosCsrImpl(mshadow::Stream *s, const NDArray& dst) { }); } -NNVM_REGISTER_OP(_zeros_no_default) +NNVM_REGISTER_OP(_zeros_without_dtype) .set_attr("FCompute", FillCompute) .set_attr("FComputeEx", FillComputeZerosEx); From 5da3117028b050bd4855db4a70b668102f3586cc Mon Sep 17 00:00:00 2001 From: Lin Yuan Date: Fri, 14 Sep 2018 12:38:30 -0700 Subject: [PATCH 8/8] Increase atol and rtol to avoid flakiness --- tests/python/unittest/test_infer_type.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/python/unittest/test_infer_type.py b/tests/python/unittest/test_infer_type.py index 15795515247b..bad83f3ef01b 100644 --- a/tests/python/unittest/test_infer_type.py +++ b/tests/python/unittest/test_infer_type.py @@ -18,10 +18,12 @@ # pylint: skip-file import mxnet as mx import numpy as np -from common import models +from common import models, with_seed from mxnet import autograd from nose.tools import * +from mxnet.test_utils import assert_almost_equal +@with_seed() def test_infer_multiout_op(): data = mx.nd.arange(16, dtype=np.float64).reshape((4, 4)) data.attach_grad() @@ -31,6 +33,7 @@ def test_infer_multiout_op(): y[0].backward() assert data.grad.dtype == np.float64 +@with_seed() def test_infer_multiout_op2(): def test_func(a): q, l = mx.nd.linalg.gelqf(a) @@ -47,7 +50,7 @@ def test_func(a): with autograd.record(): test64 = test_func(data64) test64.backward() - assert_almost_equal(data64.grad.asnumpy().all(), data32.grad.asnumpy().all()) + assert_almost_equal(data64.grad.asnumpy(), data32.grad.asnumpy(), atol=1e-5, rtol=1e-5) if __name__ == '__main__':