Skip to content

Added Fashion MNIST jupyter notebook#549

Merged
vfdev-5 merged 8 commits intopytorch:masterfrom
ANUBHAVNATANI:master
Jun 29, 2019
Merged

Added Fashion MNIST jupyter notebook#549
vfdev-5 merged 8 commits intopytorch:masterfrom
ANUBHAVNATANI:master

Conversation

@ANUBHAVNATANI
Copy link
Contributor

@ANUBHAVNATANI ANUBHAVNATANI commented Jun 26, 2019

Description:
Added Fashion MNIST jupyter notebook

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Jun 26, 2019

@ANUBHAVNATANI looks good, thanks, but maybe we can improve some points:

  • typos : Infrencing the model, #moving model to cpu for infrencing
  • nits: I would prefer the comments with a space between # symbol and the following text:
#transform to normalize the data
->
# transform to normalize the data
  • in the text all variables, modules etc it would be better to put into the ticks. For example,

We import torch, nn and functional modules to create our models.

-> We import `torch`, `nn` and `functional` modules to create our models.
  • Training curves looks a bit strange with accuracy (train/val) going down. Can we improve this ?
  • I'd like to see also the usage of ConfusionMatrix how well classifier is doing, can you please implement this ?

@ANUBHAVNATANI
Copy link
Contributor Author

@vfdev-5 I had made all the changes that you had mentioned above.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Jun 26, 2019

@ANUBHAVNATANI thanks, that looks better. However, concerning the confusion matrix I wasn't enough clear. In ignite we provide ConfusionMatrix metric, so I was thinking to compute it as attached metric.

About the training, so now we are clearly overfitting, any ideas how to get val accuracy ~> 93-95 % (it's a MNIST-like dataset anyway) ?

@ANUBHAVNATANI
Copy link
Contributor Author

@vfdev-5 I think by changing the model architecture or doing some hyper parameter tuning we can achieve better accuracy.
For the confusion matrix I was not able to fully understand the Confusion Matrix of ignite library and it was not working on my laptop I was not able to import it that's why I used sklearn implementation.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Jun 27, 2019

I think by changing the model architecture or doing some hyper parameter tuning we can achieve better accuracy.

Yes that would be nice to see in the notebook.

For the confusion matrix I was not able to fully understand the Confusion Matrix of ignite library and it was not working on my laptop I was not able to import it that's why I used sklearn implementation.

Can post your code which didn't work. Confusion matrix should normally work as any other metric by attaching to an evaluator...

@ANUBHAVNATANI
Copy link
Contributor Author

@vfdev-5 I had made further changes to the notebook to get high accuracy and encompassing confusion matrix of ignite library.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Jun 28, 2019

@ANUBHAVNATANI nice! Result is better.
just some nits:

  1. Missing spaces after comma
from torchvision import datasets,transforms ->
from torchvision import datasets, transforms

and

from ignite.metrics import Accuracy,Loss,RunningAverage,ConfusionMatrix
from ignite.handlers import ModelCheckpoint,EarlyStopping

and maybe others

  1. In "CNN Model", "Explanation of Model Architecture", I'm not sure about whether we need to provide the following:
- The output of Convolutional layer is determined by W'(new dimension) = (W-F+2P)/S + 1 here W is dimension 28(28 x 28) and F is the number of filters and P is the amount of padding and S is the amount of stride.
- After second convolutional layer and max pooling we get the output as 64x6x6
- We convert this 64x6x6 output to the flat tensor by using pytorch tensor view method
- Then we pass this flat tensor to the fully connected layers

Maybe we could better explain ignite part rather than NN stuff. What do you think ?

  1. Can you please make another pass to check that the functions are between ticks...
    For example, below "EarlyStopping - Tracking Validation Loss" is not the case. Please, fix also the cases like
Now we will setup a `Early Stopping` handler for this training process. 

to

Now we will setup a `EarlyStopping` handler for this training process. 
  1. Small code refactoring
evaluator = create_supervised_evaluator(model,metrics={'accuracy':Accuracy(),'nll':Loss(criterion),'cm':ConfusionMatrix(num_classes=10)},device=device)

to

metrics = {
# ...
}
evaluator = create_supervised_evaluator(model, metrics=metrics, device=device)

and explain how and which we setup the metrics, please.

Thanks !

@ANUBHAVNATANI
Copy link
Contributor Author

@vfdev-5 Thanks for the suggestion I had made all the changes to the notebook as you have mentioned before.

Markdown Update 3
Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @ANUBHAVNATANI

@vfdev-5 vfdev-5 merged commit 4432bb9 into pytorch:master Jun 29, 2019
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