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

GANs mnist #172

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

GANs mnist #172

wants to merge 22 commits into from

Conversation

swaingotnochill
Copy link
Contributor

@swaingotnochill swaingotnochill commented Jul 25, 2021

@kartikdutt18 @zoq I have created this new pull request coz in the earlier branch a lot of stuff has been messed up.
To do:

  1. Document Model architecture( It's half done).
  2. Notebook to create samples( but this can be done after we train the model).

Please check the correctness of script, especially the Evaluate Function and model save method.
@zoq If this training file is correct, can you please train the model?
Right now, the model will on 657 cycles with 1 epoch per cycle. May be we can improve on this? Your thoughts.

P.S: Ignore the file path, and make file configurations, I will fix them later on.

Comment on lines 183 to 198
for( int i = 0; i < cycles; i++)
{
// Training the neural network. For first iteration, weights are random,
// thus using current values as starting point.
gan.Train(trainDataset,
optimizer,
ens::PrintLoss(),
ens::ProgressBar(),
ens::Report());

optimizer.ResetPolicy() = false;
std::cout << " Model Performance " <<
gan.Evaluate(gan.Parameters(), // Parameters of the network.
i, // Index of current input.
batchSize); // Batch size.
}
Copy link
Member

Choose a reason for hiding this comment

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

We don't need to use loops anymore, You can refer to mnist_cnn.

@swaingotnochill
Copy link
Contributor Author

@zoq @kartikdutt18 I forgot to mention that the split function is not working for me. Like it doesn't do anything, and the entire dataset is loaded. Can you check if it is the same for you?

Comment on lines 193 to 197
optimizer.ResetPolicy() = false;
std::cout << " Model Performance " <<
gan.Evaluate(gan.Parameters(), // Parameters of the network.
i, // Index of current input.
batchSize); // Batch size.
Copy link
Member

@kartikdutt18 kartikdutt18 Jul 27, 2021

Choose a reason for hiding this comment

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

For saving periodically, you can use https://github.com/mlpack/models/blob/master/ensmallen_utils/periodic_save.hpp
You can download the file and use it temporarily for testing / forming the notebook.

@zoq
Copy link
Member

zoq commented Jul 28, 2021

@swaingotnochill @Davidportlouis I just realized that we have a notebook that shows how you can load images into an armadillo matrix with the format mlpack expects - https://github.com/mlpack/examples/blob/master/cifar10_transformation_with_pca/cifar-10-pca-cpp.ipynb. Let me know if you have any questions about the notebook.

mnist_gan/mnist_gan.cpp Outdated Show resolved Hide resolved
@swaingotnochill
Copy link
Contributor Author

785x42000 Dataset Loaded Train Dataset Size : (784, 42000) Validation Dataset Size : (784, 0) Training ...
I am not sure why....I did build mlpack from master...maybe I will create a new build and test it...

mnist_gan/mnist_gan.cpp Outdated Show resolved Hide resolved
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@swaingotnochill
Copy link
Contributor Author

The generate_image.py file is incomplete[I have yet to implement what kartik proposed]. Also the code needs a bit of cleaning. Please let me know if there is any logic error.

@swaingotnochill
Copy link
Contributor Author

@zoq @kartikdutt18 I am preparing the final notebook, but it seems that it crashes very often. Will it be alright if we can have the model definition, and the explanation part on the notebook.

@swaingotnochill
Copy link
Contributor Author

@zoq @kartikdutt18 Can you review the code? See if this can merge. Later, after the model is trained, we can just push the updated output. As for the notebook, I have just added the output code. The model architecture is written in comments in the mnist_gan.cpp file. Just let me know if we want to write the same model architecture in notebook too. Also, if there is anything to add, feel free to tell.

mnist_gan/mnist_gan.cpp Outdated Show resolved Hide resolved
mnist_gan/mnist_gan.cpp Outdated Show resolved Hide resolved
mnist_gan/mnist_gan.cpp Outdated Show resolved Hide resolved
mnist_gan/mnist_gan.cpp Outdated Show resolved Hide resolved
mnist_gan/mnist_gan.cpp Outdated Show resolved Hide resolved
mnist_gan/mnist_gan.cpp Outdated Show resolved Hide resolved
mnist_gan/mnist_gan_generate.cpp Show resolved Hide resolved
mnist_gan/mnist_gan_notebook.ipynb Outdated Show resolved Hide resolved
mnist_gan/mnist_gan_notebook.ipynb Outdated Show resolved Hide resolved
mnist_gan/.gitignore Outdated Show resolved Hide resolved
Copy link
Member

@kartikdutt18 kartikdutt18 left a comment

Choose a reason for hiding this comment

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

Some minor changes.

mnist_gan/.gitignore Outdated Show resolved Hide resolved
mnist_gan/mnist_gan.cpp Outdated Show resolved Hide resolved
mnist_gan/mnist_gan.cpp Outdated Show resolved Hide resolved
mnist_gan/mnist_gan_generate.cpp Outdated Show resolved Hide resolved
mnist_gan/mnist_gan_generate.cpp Outdated Show resolved Hide resolved
mnist_gan/mnist_gan_generate.cpp Outdated Show resolved Hide resolved
@mlpack-bot
Copy link

mlpack-bot bot commented Sep 27, 2021

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! 👍

@shrit
Copy link
Member

shrit commented Sep 27, 2021

@swaingotnochill Is there anything else required for this PR? This addition is very nice.

@swaingotnochill
Copy link
Contributor Author

@swaingotnochill Is there anything else required for this PR? This addition is very nice.

The saved model is only trained for 2 epochs...We can merge this, just that we have to train the model for this which might take some days. But, the architecture seems fine for now, so shouldn't be an issue if we add this..

@shrit
Copy link
Member

shrit commented Nov 23, 2021

@zoq do we merge this?

@shrit
Copy link
Member

shrit commented Jul 3, 2024

I will try to port the C++ example manually to the examples repo.

@shrit
Copy link
Member

shrit commented Aug 5, 2024

@geekypathak21 feel free when you have the time to port this one as well, many thanks

@geekypathak21
Copy link
Member

@shrit Sure will do 👍

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

Successfully merging this pull request may close these issues.

5 participants