Add MXNet Gluon model functionality. #137

meissnereric · 2018-04-13T17:11:28Z

We have some models that are built using MXNet's Gluon interface (http://gluon.mxnet.io/), so I added another model class to handle that type of network.

Also, there is a bug in the mxnet codebase with using the mx.symbol.softmax_cross_entropy and it hasn't worked in a while so I fixed that in the existing MXNet code as well.

Just contributing back if others want to use MXNet models :) Feedback welcome.

Thanks,
Eric

jonasrauber · 2018-04-14T06:15:40Z

Hi @meissnereric, thanks a lot for your pull-request. It's great to have support for Gluon models! I am aware of the softmax_cross_entropy bug in MXNet; just last week I wanted to finally upgrade our tests to the latest MXNet and ran into it again: apache/mxnet#6874 so I really appreciate your workaround.

I am looking forward to merging this. Before that, we need to fix the code formatting and update the tests.

Here are some code-formatting issues that need to be fixed:

flake8 --ignore E402,E741 .
./foolbox/models/mxnet_gluon.py:77:14: E225 missing whitespace around operator
./foolbox/models/mxnet_gluon.py:84:80: E501 line too long (97 > 79 characters)
./foolbox/models/mxnet_gluon.py:89:14: E225 missing whitespace around operator
./foolbox/models/mxnet.py:7:1: E302 expected 2 blank lines, found 1
./foolbox/models/mxnet.py:66:80: E501 line too long (89 > 79 characters)
./foolbox/models/mxnet.py:68:9: E265 block comment should start with '# '

Also, you made some changes to adversarial.py. Could you remove them from the pull-request?

Finally, it would be great if you could upgrade MXNet in the travis config to the latest MXNet version so that we can see if your workaround passes the tests; see here: https://github.com/bethgelab/foolbox/pull/133/files

It would be great if you could to these changes, otherwise I can also do them myself as soon as I find time.

meissnereric · 2018-04-16T09:34:24Z

Cool, I fixed up the formatting, removed the adversarial.py change, and upgraded the travis file.

Thanks,
Eric

coveralls · 2018-04-16T09:39:44Z

Coverage decreased (-4.8%) to 95.247% when pulling 7bcae18 on meissnereric:mxnet_gluon into 701742e on bethgelab:master.

jonasrauber

In the end we will need tests like foolbox/tests/test_models_mxnet.py…

jonasrauber · 2018-04-16T20:00:02Z

foolbox/models/mxnet.py

@@ -63,7 +63,11 @@ def __init__(
        label = mx.symbol.Variable('label')
        self._label_sym = label

-        loss = mx.symbol.softmax_cross_entropy(logits, label)
+        log_softmax = mx.sym.log_softmax(logits)


can you add a comment pointing out that this is a workaround for apache/mxnet#6874

jonasrauber · 2018-04-16T20:01:04Z

foolbox/models/mxnet_gluon.py

+
+
+class MXNetGluonModel(DifferentiableModel):
+    """Creates a :class:`Model` instance from existing `MXNet` symbols and weights.


can you update the documentation and the parameter list

jonasrauber · 2018-04-16T20:04:34Z

foolbox/models/mxnet_gluon.py

+            loss = mx.nd.softmax_cross_entropy(L, label)
+            loss.backward()
+        return np.squeeze(L.asnumpy(), axis=0),
+        self._process_gradient(data_array.grad.asnumpy())


indentation missing

jonasrauber · 2018-04-16T20:05:12Z

foolbox/models/mxnet_gluon.py

+            L = self._block(data_array)
+        return np.squeeze(L.asnumpy(), axis=0)
+
+    def predictions(self, image):


no need to implement predictions if batch_predictions is implemented (the base class will handle this)

jonasrauber · 2018-04-16T20:05:57Z

foolbox/models/mxnet_gluon.py

+    def num_classes(self):
+        return self._num_classes
+
+    def batch_predictions(self, image):


batch_predictions gets a batch of inputs and should be able to calculate predictions for all of them; this implementation only works for single inputs

jonasrauber · 2018-04-16T20:11:13Z

Thanks Eric!
I reviewed the code and listed some changes that are still necessary. In the end, we also need a small mean_brightness_net example for the tests, similar to foolbox/tests/test_models_mxnet.py.

meissnereric · 2018-05-01T21:18:44Z

Hey all,

Took a few weeks but I've added tests and addressed the feedback for the CR. Let me know if there's anything else!

Thanks,
Eric

meissnereric · 2018-05-14T08:17:15Z

Any updates?

jonasrauber · 2018-05-24T15:13:08Z

foolbox/models/mxnet_gluon.py

+
+
+class MXNetGluonModel(DifferentiableModel):
+    """Creates a :class:`Model` instance from existing `MXNet` symbols and weights.


This line of the documentation still needs to be updated

jonasrauber · 2018-05-24T15:15:26Z

Thanks a lot, @meissnereric! Looking forward to merging this as soon as that last small change in the documentation of the MXNetGluonModel documentation is fixed.

meissnereric · 2018-05-24T16:02:20Z

Should be good to go now!

jonasrauber · 2018-05-25T08:20:03Z

Great work! Thanks a lot!

Use manual softmax_cross_entropy instead of MXNet built-in due to bug.

3f9f58c

meissnereric added 2 commits April 16, 2018 10:29

Added MXNet Gluon Model.

f333f0a

Upgrade MXNet Travis config.

0b2a5b8

meissnereric force-pushed the mxnet_gluon branch from ce8d2b8 to 0b2a5b8 Compare April 16, 2018 09:32

jonasrauber requested changes Apr 16, 2018

View reviewed changes

meissnereric force-pushed the mxnet_gluon branch 5 times, most recently from 6f9a9b0 to 4928b65 Compare May 1, 2018 20:31

meissnereric force-pushed the mxnet_gluon branch from 4928b65 to 696c5aa Compare May 2, 2018 17:08

jonasrauber reviewed May 24, 2018

View reviewed changes

jonasrauber approved these changes May 24, 2018

View reviewed changes

Updated MXNetGluon docs, batch_predictions, and added unit tests.

7bcae18

meissnereric force-pushed the mxnet_gluon branch from 696c5aa to 7bcae18 Compare May 24, 2018 15:51

jonasrauber merged commit 6f3a637 into bethgelab:master May 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add MXNet Gluon model functionality. #137

Add MXNet Gluon model functionality. #137

meissnereric commented Apr 13, 2018

jonasrauber commented Apr 14, 2018

meissnereric commented Apr 16, 2018

coveralls commented Apr 16, 2018 •

edited

jonasrauber left a comment

jonasrauber Apr 16, 2018

jonasrauber Apr 16, 2018

jonasrauber Apr 16, 2018

jonasrauber Apr 16, 2018

jonasrauber Apr 16, 2018

jonasrauber commented Apr 16, 2018

meissnereric commented May 1, 2018

meissnereric commented May 14, 2018

jonasrauber May 24, 2018

jonasrauber commented May 24, 2018

meissnereric commented May 24, 2018

jonasrauber commented May 25, 2018



		class MXNetGluonModel(DifferentiableModel):
		"""Creates a :class:`Model` instance from existing `MXNet` symbols and weights.

Add MXNet Gluon model functionality. #137

Add MXNet Gluon model functionality. #137

Conversation

meissnereric commented Apr 13, 2018

jonasrauber commented Apr 14, 2018

meissnereric commented Apr 16, 2018

coveralls commented Apr 16, 2018 • edited

jonasrauber left a comment

Choose a reason for hiding this comment

jonasrauber Apr 16, 2018

Choose a reason for hiding this comment

jonasrauber Apr 16, 2018

Choose a reason for hiding this comment

jonasrauber Apr 16, 2018

Choose a reason for hiding this comment

jonasrauber Apr 16, 2018

Choose a reason for hiding this comment

jonasrauber Apr 16, 2018

Choose a reason for hiding this comment

jonasrauber commented Apr 16, 2018

meissnereric commented May 1, 2018

meissnereric commented May 14, 2018

jonasrauber May 24, 2018

Choose a reason for hiding this comment

jonasrauber commented May 24, 2018

meissnereric commented May 24, 2018

jonasrauber commented May 25, 2018

coveralls commented Apr 16, 2018 •

edited