An example of gan implemented by DL4J #1030
Conversation
There was a problem hiding this comment.
Thank you for submitting your example.
We have recently raised the standards for new example pull requests, so if you want your example to be included in the official examples, there is some more work that needs to be done.
For the code, I've commented in a few places on how things can be done better, or where I think it should be clearer.
But the code alone isn't all that useful, unless you already know how to build a GAN, and in that case you wouldn't need an example.
For this reason, I ask you to also add comments through out the code to explain why you are doing what you are doing there.
And finally, I don't think this should be an entire sub category of examples, esp. given that it is just a single example. It makes more sense to be in its own package within the dl4j-examples category, probably in the advanced section.
Edit: And you need to sign the Eclipse ECA for us to be able to accept any pull request from you at all.
|
|
||
| mvn package | ||
| cd target | ||
| java -cp deeplearning4j-examples-1.0.0-beta-bin.jar org.deeplearning4j.LenetMnistExample |
There was a problem hiding this comment.
Please ensure that examples like this are actually runable.
| BufferedImage bi = new BufferedImage(28, 28, BufferedImage.TYPE_BYTE_GRAY); | ||
| for (int i = 0; i < 784; i++) { | ||
| bi.getRaster().setSample(i % 28, i / 28, 0, (int) (255 * arr.getDouble(i))); | ||
| } |
There was a problem hiding this comment.
This is a very slow way of drawing the image.
You can use Java2DNativeImageLoader to turn an INDArray into an JavaCV Mat, then use OpenCVFrameConverter.ToMat to convert that into a Frame and Java2DFrameConverter to copy that frame into an already existing BufferedImage.
| .layer(3, new org.deeplearning4j.nn.conf.layers.misc.FrozenLayerWithBackprop(new DenseLayer.Builder().nIn(28 * 28).nOut(512).activation(Activation.RELU).build())) | ||
| .layer(4, new org.deeplearning4j.nn.conf.layers.misc.FrozenLayerWithBackprop(new DenseLayer.Builder().nIn(512).nOut(256).activation(Activation.RELU).build())) | ||
| .layer(5, new org.deeplearning4j.nn.conf.layers.misc.FrozenLayerWithBackprop(new DenseLayer.Builder().nIn(256).nOut(128).activation(Activation.RELU).build())) | ||
| .layer(6, new org.deeplearning4j.nn.conf.layers.misc.FrozenLayerWithBackprop(new OutputLayer.Builder(LossFunctions.LossFunction.XENT) |
There was a problem hiding this comment.
Why are you using a fully quallified name here instead of importing it?
| .layer(4, new org.deeplearning4j.nn.conf.layers.misc.FrozenLayerWithBackprop(new DenseLayer.Builder().nIn(512).nOut(256).activation(Activation.RELU).build())) | ||
| .layer(5, new org.deeplearning4j.nn.conf.layers.misc.FrozenLayerWithBackprop(new DenseLayer.Builder().nIn(256).nOut(128).activation(Activation.RELU).build())) | ||
| .layer(6, new org.deeplearning4j.nn.conf.layers.misc.FrozenLayerWithBackprop(new OutputLayer.Builder(LossFunctions.LossFunction.XENT) | ||
| .activation(Activation.SIGMOID).nIn(128).nOut(1).build())).build(); |
There was a problem hiding this comment.
For both networks, why don't you use the setInputType functionality, so you don't have to set nIn on every layer?
| ganNetwork.setListeners(new ScoreIterationListener(1)); | ||
| System.out.println(ganNetwork.summary()); | ||
|
|
||
| DataSetIterator train = new MnistDataSetIterator(30, true, 12345); |
There was a problem hiding this comment.
You are using the batch size in a few places, it would be better to have it as a variable, so it isn't just a magic number, but delivers some semantic meaning.
What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
Please review
https://github.com/eclipse/deeplearning4j/blob/master/CONTRIBUTING.md before opening a pull request.