Baunsgaard commented on code in PR #1848: URL: https://github.com/apache/systemds/pull/1848#discussion_r1272477319
########## src/main/python/systemds/operator/nn/__init__.py: ########## Review Comment: add license (i know it is stupid, but these do need it as well.) ########## src/main/python/systemds/operator/nn/neural_network.py: ########## Review Comment: I do not like that this is part of the nn package. This should be moved to the tests. ########## src/main/python/systemds/operator/nn/affine.py: ########## @@ -0,0 +1,114 @@ +# ------------------------------------------------------------- +# +# 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. +# +# ------------------------------------------------------------- +import os + +from systemds.context import SystemDSContext +from systemds.operator import Matrix, Source, MultiReturn +from systemds.utils.helpers import get_path_to_script_layers + + +class Affine: + _source: Source = None + _X: Matrix Review Comment: Same here, i do not like the input is stored inside. ########## src/main/python/tests/nn/test_affine.py: ########## @@ -0,0 +1,134 @@ +# ------------------------------------------------------------- +# +# 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. +# +# ------------------------------------------------------------- + +import unittest + +import numpy as np +from numpy.testing import assert_almost_equal + +from systemds.context import SystemDSContext +from systemds.script_building.script import DMLScript +from systemds.operator.nn.affine import Affine + +dim = 6 +n = 5 +m = 6 +X = np.array([[9., 2., 5., 5., 9., 6.], + [0., 8., 8., 0., 5., 7.], + [2., 2., 6., 3., 4., 3.], + [3., 5., 2., 6., 6., 0.], + [3., 8., 5., 2., 5., 2.]]) + +W = np.array([[8., 3., 7., 2., 0., 1.], + [6., 5., 1., 2., 6., 1.], + [2., 4., 7., 7., 6., 4.], + [3., 8., 9., 3., 5., 6.], + [3., 8., 0., 5., 7., 9.], + [7., 9., 7., 4., 5., 7.]]) +dout = np.array([[9., 5., 4., 0., 4., 1.], + [1., 2., 2., 3., 3., 9.], + [7., 4., 0., 8., 7., 0.], + [8., 7., 0., 6., 0., 9.], + [1., 6., 5., 8., 8., 9.]]) + + +class TestAffine(unittest.TestCase): + sds: SystemDSContext = None + + @classmethod + def setUpClass(cls): + cls.sds = SystemDSContext() + + @classmethod + def tearDownClass(cls): + cls.sds.close() + + def test_init(self): + affine = Affine(self.sds, dim, m, 10) + w = affine.weight.compute() + self.assertEqual(len(w), 6) + self.assertEqual(len(w[0]), 6) + + def test_forward(self): + Xm = self.sds.from_numpy(X) + Wm = self.sds.from_numpy(W) + bm = self.sds.full((1, 6), 0) + + # test class method + affine = Affine(self.sds, dim, m, 10) + out = affine.forward(Xm).compute() + self.assertEqual(len(out), 5) + self.assertEqual(len(out[0]), 6) + + # test static method + out = Affine.forward(Xm, Wm, bm).compute() + expected = np.matmul(X, W) + assert_almost_equal(out, expected) + + def test_backward(self): + Xm = self.sds.from_numpy(X) + Wm = self.sds.from_numpy(W) + bm = self.sds.full((1, 6), 0) + doutm = self.sds.from_numpy(dout) + + # test class method + affine = Affine(self.sds, dim, m, 10) + out = affine.forward(Xm) Review Comment: out is never used. ########## src/main/python/systemds/operator/nn/relu.py: ########## @@ -0,0 +1,74 @@ +# ------------------------------------------------------------- +# +# 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. +# +# ------------------------------------------------------------- +import os.path + +from systemds.context import SystemDSContext +from systemds.operator import Matrix, Source +from systemds.utils.helpers import get_path_to_script_layers + + +class ReLU: + _source: Source = None + _X: Matrix Review Comment: I do not like that X (the input) is stored inside ########## src/main/python/tests/nn/test_neural_network.py: ########## @@ -0,0 +1,94 @@ +# ------------------------------------------------------------- +# +# 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. +# +# ------------------------------------------------------------- +import unittest +import numpy as np + +from systemds.context import SystemDSContext +from systemds.operator.nn.neural_network import NeuralNetwork +from systemds.script_building.script import DMLScript + +# Seed for the input matrix +np.random.seed(42) + + +class TestNeuralNetwork(unittest.TestCase): + sds: SystemDSContext = None + + @classmethod + def setUpClass(cls): + cls.sds = SystemDSContext() + cls.X = np.random.rand(6, 1) + cls.exp_out = np.array([ + -0.37768756, -0.47785831, -0.95870362, + -1.21297214, -0.73814523, -0.933917, + -0.60368929, -0.76380049, -0.15732974, + -0.19905692, -0.15730542, -0.19902615 + ]) + cls.nn = NeuralNetwork(cls.sds, dim=1) + + @classmethod + def tearDownClass(cls): + cls.sds.close() + + def test_forward_pass(self): + + Xm = self.sds.from_numpy(self.X) + + # test forward pass through the network using static calls + static_out = self.nn.forward_static_pass(Xm)\ + .compute().flatten() + self.assertTrue(np.allclose(static_out, self.exp_out)) + + # test forward pass through the network using dynamic calls + dynamic_out = self.nn.forward_dynamic_pass(Xm)\ + .compute().flatten() + self.assertTrue(np.allclose(dynamic_out,self.exp_out)) + + def test_multiple_sourcing(self): + + Xm = self.sds.from_numpy(self.X) + + # test for verifying that affine and relu are each being sourced exactly once + network_out = self.nn.forward_static_pass(Xm) + scripts = DMLScript(self.sds) + scripts.build_code(network_out) + + self.assertEqual(1,self.count_sourcing(scripts.dml_script, layer_name="affine")) Review Comment: format, add a space after "," ########## src/main/python/systemds/operator/nn/affine.py: ########## @@ -0,0 +1,114 @@ +# ------------------------------------------------------------- +# +# 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. +# +# ------------------------------------------------------------- +import os + +from systemds.context import SystemDSContext +from systemds.operator import Matrix, Source, MultiReturn +from systemds.utils.helpers import get_path_to_script_layers + + +class Affine: + _source: Source = None + _X: Matrix + weight: Matrix + bias: Matrix + + def __new__(cls, *args, **kwargs): + return super().__new__(cls) + + def __init__(self, sds_context: SystemDSContext, d, m, seed=-1): + """ + sds_context: systemdsContext + d: number of features + m: number of neuron + """ + Affine._create_source(sds_context) + + # bypassing overload limitation in python + self.forward = self._instance_forward + self.backward = self._instance_backward + + # init weight and bias + self.weight = Matrix(sds_context, '') + self.bias = Matrix(sds_context, '') + params_dict = {'D': d, 'M': m, 'seed': seed} + out = [self.weight, self.bias] + op = MultiReturn(sds_context, "affine::init", output_nodes=out, named_input_nodes=params_dict) + self.weight._unnamed_input_nodes = [op] + self.bias._unnamed_input_nodes = [op] + op._source_node = self._source + + @staticmethod + def forward(X: Matrix, W: Matrix, b: Matrix): + """ + X: input matrix + W: weigth matrix + b: bias + return out: output matrix + """ + Affine._create_source(X.sds_context) + out = Affine._source.forward(X, W, b) + return out + + @staticmethod + def backward(dout, X: Matrix, W: Matrix, b: Matrix): + """ + dout: gradient of output, passed from the upstream + X: input matrix + W: weigth matrix + b: bias + return dX, dW,db: gradient of input, weights and bias, respectively + """ + sds = X.sds_context + Affine._create_source(sds) + params_dict = {'dout': dout, 'X': X, 'W': W, 'b': b} + dX = Matrix(sds, '') + dW = Matrix(sds, '') + db = Matrix(sds, '') + out = [dX, dW, db] + op = MultiReturn(sds, "affine::backward", output_nodes=out, named_input_nodes=params_dict) + dX._unnamed_input_nodes = [op] + dW._unnamed_input_nodes = [op] + db._unnamed_input_nodes = [op] + op._source_node = Affine._source + return op + + def _instance_forward(self, X: Matrix): + """ + X: input matrix + return out: output matrix + """ + self._X = X Review Comment: remove this. ########## src/main/python/tests/nn/test_affine.py: ########## @@ -0,0 +1,134 @@ +# ------------------------------------------------------------- +# +# 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. +# +# ------------------------------------------------------------- + +import unittest + +import numpy as np +from numpy.testing import assert_almost_equal + +from systemds.context import SystemDSContext +from systemds.script_building.script import DMLScript +from systemds.operator.nn.affine import Affine + +dim = 6 +n = 5 +m = 6 +X = np.array([[9., 2., 5., 5., 9., 6.], + [0., 8., 8., 0., 5., 7.], + [2., 2., 6., 3., 4., 3.], + [3., 5., 2., 6., 6., 0.], + [3., 8., 5., 2., 5., 2.]]) + +W = np.array([[8., 3., 7., 2., 0., 1.], + [6., 5., 1., 2., 6., 1.], + [2., 4., 7., 7., 6., 4.], + [3., 8., 9., 3., 5., 6.], + [3., 8., 0., 5., 7., 9.], + [7., 9., 7., 4., 5., 7.]]) +dout = np.array([[9., 5., 4., 0., 4., 1.], + [1., 2., 2., 3., 3., 9.], + [7., 4., 0., 8., 7., 0.], + [8., 7., 0., 6., 0., 9.], + [1., 6., 5., 8., 8., 9.]]) + + +class TestAffine(unittest.TestCase): + sds: SystemDSContext = None + + @classmethod + def setUpClass(cls): + cls.sds = SystemDSContext() + + @classmethod + def tearDownClass(cls): + cls.sds.close() + + def test_init(self): + affine = Affine(self.sds, dim, m, 10) + w = affine.weight.compute() + self.assertEqual(len(w), 6) + self.assertEqual(len(w[0]), 6) + + def test_forward(self): + Xm = self.sds.from_numpy(X) + Wm = self.sds.from_numpy(W) + bm = self.sds.full((1, 6), 0) + + # test class method + affine = Affine(self.sds, dim, m, 10) + out = affine.forward(Xm).compute() + self.assertEqual(len(out), 5) + self.assertEqual(len(out[0]), 6) + + # test static method + out = Affine.forward(Xm, Wm, bm).compute() + expected = np.matmul(X, W) + assert_almost_equal(out, expected) + + def test_backward(self): + Xm = self.sds.from_numpy(X) + Wm = self.sds.from_numpy(W) + bm = self.sds.full((1, 6), 0) + doutm = self.sds.from_numpy(dout) Review Comment: I do not like doutm is materialized here, can we not calculate it based on a forward to backward pass. ########## src/main/python/systemds/operator/nn/affine.py: ########## @@ -0,0 +1,114 @@ +# ------------------------------------------------------------- +# +# 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. +# +# ------------------------------------------------------------- +import os + +from systemds.context import SystemDSContext +from systemds.operator import Matrix, Source, MultiReturn +from systemds.utils.helpers import get_path_to_script_layers + + +class Affine: + _source: Source = None + _X: Matrix + weight: Matrix + bias: Matrix + + def __new__(cls, *args, **kwargs): + return super().__new__(cls) + + def __init__(self, sds_context: SystemDSContext, d, m, seed=-1): + """ + sds_context: systemdsContext + d: number of features + m: number of neuron + """ + Affine._create_source(sds_context) + + # bypassing overload limitation in python + self.forward = self._instance_forward + self.backward = self._instance_backward + + # init weight and bias + self.weight = Matrix(sds_context, '') + self.bias = Matrix(sds_context, '') + params_dict = {'D': d, 'M': m, 'seed': seed} + out = [self.weight, self.bias] + op = MultiReturn(sds_context, "affine::init", output_nodes=out, named_input_nodes=params_dict) + self.weight._unnamed_input_nodes = [op] + self.bias._unnamed_input_nodes = [op] + op._source_node = self._source + + @staticmethod + def forward(X: Matrix, W: Matrix, b: Matrix): + """ + X: input matrix + W: weigth matrix + b: bias + return out: output matrix + """ + Affine._create_source(X.sds_context) + out = Affine._source.forward(X, W, b) + return out Review Comment: collapse these to lines to not make out on the line above. ########## src/main/python/tests/nn/test_relu.py: ########## @@ -0,0 +1,101 @@ +# ------------------------------------------------------------- +# +# 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. +# +# ------------------------------------------------------------- + +import unittest + +import numpy as np + +from systemds.context import SystemDSContext +from systemds.script_building.script import DMLScript +from systemds.operator.nn.relu import ReLU + + +class TestRelu(unittest.TestCase): + sds: SystemDSContext = None + + @classmethod + def setUpClass(cls): + cls.sds = SystemDSContext() + cls.X = np.array([0, -1, -2, 2, 3, -5]) Review Comment: move this to the individual tests. ########## src/main/python/systemds/operator/nn/relu.py: ########## @@ -0,0 +1,74 @@ +# ------------------------------------------------------------- +# +# 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. +# +# ------------------------------------------------------------- +import os.path + +from systemds.context import SystemDSContext +from systemds.operator import Matrix, Source +from systemds.utils.helpers import get_path_to_script_layers + + +class ReLU: + _source: Source = None + _X: Matrix + + def __init__(self, sds: SystemDSContext): + ReLU._create_source(sds) + self.forward = self._instance_forward + self.backward = self._instance_backward + + @staticmethod + def forward(X: Matrix): + """ + X: input matrix + return out: output matrix + """ + ReLU._create_source(X.sds_context) + out = ReLU._source.forward(X) + return out + + @staticmethod + def backward(dout: Matrix, X: Matrix): + """ + dout: gradient of output, passed from the upstream + X: input matrix + return dX: gradient of input + """ + ReLU._create_source(dout.sds_context) + dX = ReLU._source.backward(dout, X) + return dX + + # forward = staticmethod(forward) + # backward = staticmethod(backward) Review Comment: remove commented code. -- 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. To unsubscribe, e-mail: dev-unsubscr...@systemds.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
