Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An example of gan implemented by DL4J #1030

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

zdl010
Copy link

@zdl010 zdl010 commented Apr 24, 2021

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.

Copy link
Member

@treo treo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ensure that examples like this are actually runable.

Comment on lines +54 to +57
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)));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants