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

Added and updated README files for vision models #218 #305

Merged
merged 48 commits into from
Sep 28, 2021

Conversation

aditkumar72
Copy link
Contributor

I have added README files for cdcgan_mnist, conv_mnist, dcgan_mnist, mlp_mnist, vgg_cifar10 and updated README file of vae_mnist. I have also added abspath code in the training files of cdcgan_mnist, conv_mnist, dcgan_mnist, mlp_mnist, vgg_cifar10 .

vision/cdcgan_mnist/cGAN_mnist.jl Outdated Show resolved Hide resolved
vision/cdcgan_mnist/cGAN_mnist.jl Outdated Show resolved Hide resolved
vision/cdcgan_mnist/cGAN_mnist.jl Outdated Show resolved Hide resolved
vision/vae_mnist/README.md Outdated Show resolved Hide resolved
vision/vae_mnist/README.md Outdated Show resolved Hide resolved
vision/dcgan_mnist/README.md Outdated Show resolved Hide resolved
@CarloLucibello
Copy link
Member

thanks, this is nice work! just added some minor comments

@aditkumar72
Copy link
Contributor Author

aditkumar72 commented Jun 16, 2021

I have made all the changes can you please review @CarloLucibello .

Copy link
Member

@ToucheSir ToucheSir left a comment

Choose a reason for hiding this comment

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

Thanks @aditkumar72! I've done a first pass and left some comments. The main points here are a) determining where the images come from and (assuming we have permission to use them) providing attribution as required, and b) re-formatting the references to be more detailed and structured.

@@ -0,0 +1,24 @@
# LeNet-5

![LeNet-5](../conv_mnist/docs/LeNet-5.png)
Copy link
Member

Choose a reason for hiding this comment

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

Where does this image come from? Does it require attribution?

vision/conv_mnist/README.md Outdated Show resolved Hide resolved
vision/conv_mnist/README.md Outdated Show resolved Hide resolved
Comment on lines 36 to 38
[Unsupervised Representation Learning with Deep Convolutional Generative Adversarial Networks by Soumith Chintala et al.](https://arxiv.org/pdf/1511.06434v2.pdf)

[pytorch.org/tutorials/beginner/dcgan_faces_tutorial](https://pytorch.org/tutorials/beginner/dcgan_faces_tutorial.html)
Copy link
Member

Choose a reason for hiding this comment

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

Ditto comment about reference format.

vision/dcgan_mnist/README.md Outdated Show resolved Hide resolved
@@ -0,0 +1,18 @@
# Multilayer Perceptron (MLP)

![mlp](../mlp_mnist/docs/mlp.svg)
Copy link
Member

Choose a reason for hiding this comment

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

Similar comments about image sourcing and reference formats for this file.

vision/vae_mnist/README.md Outdated Show resolved Hide resolved
vision/vgg_cifar10/README.md Show resolved Hide resolved
@aditkumar72
Copy link
Contributor Author

All the model images except of dcgan are taken from references mentioned in their respective READMEs. The dcgan image is taken from google image. I will add its source in its README file.

@aditkumar72
Copy link
Contributor Author

aditkumar72 commented Jun 16, 2021

Thanks @ToucheSir and @CarloLucibello for reviewing. I will soon come up with the proposed changes.

@aditkumar72
Copy link
Contributor Author

@ToucheSir I have made all the changes can you please review.

Copy link
Member

@ToucheSir ToucheSir left a comment

Choose a reason for hiding this comment

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

Pinging @CarloLucibello to have a final look, but I think this is almost ready to go.

vision/cdcgan_mnist/README.md Outdated Show resolved Hide resolved

* [Y. Lecun, L. Bottou, Y. Bengio and P. Haffner, "Gradient-based learning applied to document recognition," in Proceedings of the IEEE, vol. 86, no. 11, pp. 2278-2324, Nov. 1998, doi: 10.1109/5.726791.](http://yann.lecun.com/exdb/publis/pdf/lecun-01a.pdf)

* [@book
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem to render correctly for me. Can you confirm it's Github compatible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the preview it seems like rendering properly. Should I need to change anything?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe that's your local markdown editor, because GitHub renders the raw bibtex. I'd recommend just using the plaintext citation like you have for the reference above.

vision/vae_mnist/README.md Outdated Show resolved Hide resolved

## Model Info

A DC-GAN is a direct extension of the GAN, except that it explicitly uses convolutional and transposed convolutions layers in the discriminator and generator, respectively. _The discriminator is made up of strided convolution layers, batch norm layers, and LeakyReLU activations. The generator is comprised of transposed convolutions layers, batch norm layers, and ReLU activations_.
Copy link
Member

Choose a reason for hiding this comment

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

  • I think "transposed convolutional" would read better than "transposed convolutions", especially next to "convolutional".
  • What is the intent of italicizing the text at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intent of italicizing is to highlight the layers info. Should I remove it?

@aditkumar72
Copy link
Contributor Author

@ToucheSir I have made all the changes can you please review.

Copy link
Member

@ToucheSir ToucheSir left a comment

Choose a reason for hiding this comment

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

Apologies for not getting back to you on this @aditkumar72! Just some very minor changes around reference formats and I think it's good to go. I'd make them myself, but I'm not sure what citation format you used so it'd be great if you could do it :)


## Results

2000 training step
Copy link
Member

Choose a reason for hiding this comment

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

"steps" should be plural here as well.


* [Y. Lecun, L. Bottou, Y. Bengio and P. Haffner, "Gradient-based learning applied to document recognition," in Proceedings of the IEEE, vol. 86, no. 11, pp. 2278-2324, Nov. 1998, doi: 10.1109/5.726791.](http://yann.lecun.com/exdb/publis/pdf/lecun-01a.pdf)

* [@book
Copy link
Member

Choose a reason for hiding this comment

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

Maybe that's your local markdown editor, because GitHub renders the raw bibtex. I'd recommend just using the plaintext citation like you have for the reference above.


## Reference

* [@book
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above about this just rendering as bibtex.

* [Simonyan, K. and Zisserman, A., “Very Deep Convolutional Networks for Large-Scale Image Recognition”, <i>arXiv e-prints</i>, 2015.
](https://arxiv.org/pdf/1409.1556v4.pdf)

* [@book
Copy link
Member

Choose a reason for hiding this comment

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

Ditto here RE the link text being raw bibtex.

@aditkumar72
Copy link
Contributor Author

@ToucheSir I have made all the needed changes can you please review.

Copy link
Member

@ToucheSir ToucheSir left a comment

Choose a reason for hiding this comment

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

Wonderful, thanks for your patience through this!

@ToucheSir ToucheSir merged commit 2bd86ff into FluxML:master Sep 28, 2021
@aditkumar72 aditkumar72 deleted the readmes branch September 29, 2021 05:53
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.

3 participants