[SYSTEMDS-3426] Python NN Builtin components #1848

thaivd1309 · 2023-06-19T14:04:31Z

I added the class Affine which represents a layer in a neural network.
Also, the class Source couldn't parse the file ''affine.dml'' because it it contains the comment block " /* */ " so I fixed it

Baunsgaard

Okay, most of the things are on the right track.

I would like to change the API a bit, to enable static calling:

This means you need to change the API in 2 ways
as an example Relu should be able to be called:

from systemds.operator.nn_nodes.relu as Relu

Relu.forward(X)
Relu.backward(X)

But we should also still be able to call the other way:

Relu().forward(X)
Relu().backward(X)

For the Affine since it have internal state i like how it is, but i still want the forward and backward static call. and internally in the class for Affine i still want the option to have the state.

Second lets change the liberary name to:

systemds.operator.nn.??

As a follow up task look at the complied plans, and figure out if the NN builtin are sourced multiple times. If they are fix this. Also add a test that verifies the compiled plans does not contain multiple sources of the same file. aka if you use affine twice or more.
To look at the compiled plans simply set the verbose flag when calling compute()

Baunsgaard · 2023-06-26T00:21:22Z

src/main/python/tests/nn_nodes/__init__.py

+# specific language governing permissions and limitations
+# under the License.
+#
+# -------------------------------------------------------------


Add line in end of file

src/main/python/systemds/operator/nodes/source.py

Baunsgaard · 2023-06-26T00:25:01Z

src/main/python/systemds/operator/nn_nodes/relu.py

+    _X: Matrix
+
+    def __init__(self):
+        self._sds_context = SystemDSContext()


This is not allowed since it starts up a new JVM.
just remove it and the variable for SDS.

we can on first call extract the context from the variable X on a forward or on a Backward pass.

Baunsgaard · 2023-06-26T00:27:59Z

src/main/python/tests/nn_nodes/test_affine.py

+    def tearDownClass(cls):
+        cls.sds.close()
+
+    def test_affine(self):


add dedicated tests for forward for backward.
Ideally you add multiple such tests.
Then write in what you expect to get out from the operation based on a explicit input.

src/main/python/tests/nn_nodes/test_affine.py

thaivd1309 · 2023-07-03T18:43:01Z

I added static methods call for Affine and ReLU. Since static call of forward and backward does not have internal states, the user needs to provide extra parameter, e.g. affine.forward(X) and Affine.forward(X, W, b).
I also made the _source object static, which means it is created only once. I believe this will make the system only import once. But for the time being I will write a test to verify this.

Baunsgaard

Overall i like it, there are 3 missing pieces for me to say that it is a pass:

Test if it does not source multiple times when called multiple times in a single script, you verify this via calling compute with verbose.
Verify it works across two different instance of SystemDSContext, to do this you need to start two SystemDSContext() and call into these with two different affine transforms. Note here you need to materialize an X for each of them and remember to call close on each.
Make a test that verify combined behavior of multiple instructions. i suggest a simple test for a network of affine(128) relu affine(64) relu affine (32) relu affine(2). here verify again, that we do not souce multiple times, even if we call multiple different layers.

These things should be enough for both of you to pass the coding, but i would like to have seen more instructions introduced.

src/main/python/tests/nn/test_relu.py

rahuljo · 2023-07-24T08:52:06Z

@Baunsgaard Added the new tests that we discussed. Could you please review it?

Baunsgaard

LGTM, this concludes your Programming assignment.

There are some minor things to improve in tests, where you move the variables to individual test cases, and please add at least one more test case with other inputs.

And I am still missing an experiment to verify behavior with multiple SDS contexts.

Before merging we need to Ensure that this API is what we want in general (more people have to indicate their opinion), i can imagine one comment that could be brought up is that the standard functions should be non static and the instance based should be renamed or moved slightly, for instance outside the class definition inside the same file.

Baunsgaard · 2023-07-24T16:12:32Z

src/main/python/systemds/operator/nn/__init__.py

add license (i know it is stupid, but these do need it as well.)

Baunsgaard · 2023-07-24T16:14:59Z

src/main/python/systemds/operator/nn/neural_network.py

I do not like that this is part of the nn package. This should be moved to the tests.

Baunsgaard · 2023-07-24T16:17:25Z

src/main/python/systemds/operator/nn/relu.py

+
+class ReLU:
+    _source: Source = None
+    _X: Matrix


I do not like that X (the input) is stored inside

I thought that the input X would be used in the backward pass anyways so I stored it. But I can remove it if you wish

you know what. You are right, lets keep it, since it is needed in backward passes

Baunsgaard · 2023-07-24T16:17:46Z

src/main/python/systemds/operator/nn/affine.py

+
+class Affine:
+    _source: Source = None
+    _X: Matrix


Same here, i do not like the input is stored inside.

Baunsgaard · 2023-07-24T16:20:30Z

src/main/python/tests/nn/test_affine.py

+        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)


I do not like doutm is materialized here, can we not calculate it based on a forward to backward pass.

Since an error-evaluating function hasn't been implemented yet, I couldn't calculate the gradient for the backward pass. So I have to materialize the output gradient like this.

Then we should leave a TODO specifying to use an loss function.
But i think it should be possible to naively use the outputs of the forward call.

Baunsgaard · 2023-07-24T16:23:15Z

src/main/python/tests/nn/test_neural_network.py

+        scripts = DMLScript(self.sds)
+        scripts.build_code(network_out)
+
+        self.assertEqual(1,self.count_sourcing(scripts.dml_script, layer_name="affine"))


format, add a space after ","

Baunsgaard · 2023-07-24T16:23:53Z

src/main/python/tests/nn/test_relu.py

+    @classmethod
+    def setUpClass(cls):
+        cls.sds = SystemDSContext()
+        cls.X = np.array([0, -1, -2, 2, 3, -5])


move this to the individual tests.

Baunsgaard · 2023-07-24T16:25:34Z

src/main/python/systemds/operator/nn/relu.py

+        return dX
+
+    # forward = staticmethod(forward)
+    # backward = staticmethod(backward)


remove commented code.

Baunsgaard · 2023-07-24T16:26:15Z

src/main/python/systemds/operator/nn/affine.py

+        """
+        Affine._create_source(X.sds_context)
+        out = Affine._source.forward(X, W, b)
+        return out


collapse these to lines to not make out on the line above.

Baunsgaard · 2023-07-24T16:27:01Z

src/main/python/systemds/operator/nn/affine.py

+        X: input matrix
+        return out: output matrix
+        """
+        self._X = X


remove this.

This commit adds the new interface for easy usage of our neural network in python. The design take inspiration from other neural network frameworks. This specific commit contains the building blocks of Affine and Relu. Co-authored-by: Duc Thai Vu <[email protected]> Co-authored-by: Rahul Joshi <[email protected]> Closes apache#1848

Baunsgaard · 2023-10-23T10:06:15Z

Thanks for the PR @thaivd1309 and @rahuljo

I will take it from here, and moved the code to another PR #1929 ,
There is some bug that makes the source test work locally, but not in the cloud.

This commit adds the new interface for easy usage of our neural network in python. The design take inspiration from other neural network frameworks. This specific commit contains the building blocks of Affine and Relu. Co-authored-by: Duc Thai Vu <[email protected]> Co-authored-by: Rahul Joshi <[email protected]> Closes apache#1848

This commit adds the new interface for easy usage of our neural network in python. The design take inspiration from other neural network frameworks. This specific commit contains the building blocks of Affine and Relu. Closes apache#1848 Co-authored-by: Duc Thai Vu <[email protected]> Co-authored-by: Rahul Joshi <[email protected]> ...

This commit adds the new interface for easy usage of our neural network in python. The design take inspiration from other neural network frameworks. This specific commit contains the building blocks of Affine and Relu. Closes apache#1848 Closes apache#1929 Co-authored-by: Duc Thai Vu <[email protected]> Co-authored-by: Rahul Joshi <[email protected]>

This commit adds the new interface for easy usage of our neural network in python. The design take inspiration from other neural network frameworks. This specific commit contains the building blocks of Affine and Relu. Closes #1848 Closes #1929 Co-authored-by: Duc Thai Vu <[email protected]> Co-authored-by: Rahul Joshi <[email protected]>

thaivd1309 added 2 commits June 16, 2023 15:07

affine

9f84043

affine class

82db069

thaivd1309 changed the title ~~SYSTEMDS-3426 Student project~~ [SYSTEMDS-3426] Python NN Builtin components Jun 23, 2023

relu

ea1481d

Baunsgaard reviewed Jun 26, 2023

View reviewed changes

static methods in affine

bee2f29

Baunsgaard reviewed Jul 7, 2023

View reviewed changes

rahuljo reviewed Jul 10, 2023

View reviewed changes

src/main/python/tests/nn/test_relu.py Show resolved Hide resolved

rahuljo added 2 commits July 24, 2023 10:44

adding multiple sourcing test + neural network tests

a056217

adding multiple sourcing test + neural network tests

b4bd1a4

Baunsgaard reviewed Jul 24, 2023

View reviewed changes

add test with two different SystemContext() and other minor changes

fec7243

Baunsgaard mentioned this pull request Oct 20, 2023

[SYSTEMDS-3426] Python NN Builtin (Affine,Relu) #1929

Closed

Baunsgaard closed this Oct 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYSTEMDS-3426] Python NN Builtin components #1848

[SYSTEMDS-3426] Python NN Builtin components #1848

thaivd1309 commented Jun 19, 2023

Baunsgaard left a comment

Baunsgaard Jun 26, 2023

Baunsgaard Jun 26, 2023

Baunsgaard Jun 26, 2023 •

edited

Loading

thaivd1309 commented Jul 3, 2023

Baunsgaard left a comment

rahuljo commented Jul 24, 2023 •

edited

Loading

Baunsgaard left a comment

Baunsgaard Jul 24, 2023

Baunsgaard Jul 24, 2023

Baunsgaard Jul 24, 2023

thaivd1309 Jul 24, 2023

Baunsgaard Jul 24, 2023

Baunsgaard Jul 24, 2023

Baunsgaard Jul 24, 2023

thaivd1309 Jul 25, 2023

Baunsgaard Jul 25, 2023

Baunsgaard Jul 24, 2023

Baunsgaard Jul 24, 2023

Baunsgaard Jul 24, 2023

Baunsgaard Jul 24, 2023

Baunsgaard Jul 24, 2023

Baunsgaard commented Oct 23, 2023

[SYSTEMDS-3426] Python NN Builtin components #1848

[SYSTEMDS-3426] Python NN Builtin components #1848

Conversation

thaivd1309 commented Jun 19, 2023

Baunsgaard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Baunsgaard Jun 26, 2023 • edited Loading

Choose a reason for hiding this comment

thaivd1309 commented Jul 3, 2023

Baunsgaard left a comment

Choose a reason for hiding this comment

rahuljo commented Jul 24, 2023 • edited Loading

Baunsgaard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Baunsgaard commented Oct 23, 2023

Baunsgaard Jun 26, 2023 •

edited

Loading

rahuljo commented Jul 24, 2023 •

edited

Loading