Skip to content

Fix FashionMNIST loading MNIST - #640

Merged
soumith merged 1 commit into
pytorch:masterfrom
ls1xt:fix-fashion-mnist
Oct 26, 2018
Merged

Fix FashionMNIST loading MNIST#640
soumith merged 1 commit into
pytorch:masterfrom
ls1xt:fix-fashion-mnist

Conversation

@ls1xt

@ls1xt ls1xt commented Oct 26, 2018

Copy link
Copy Markdown
Contributor

Before this would lead FashionMNIST to contain mnist data:

MNIST(root, download=True)
FashionMNIST(root, download=True)

As FashionMNIST inherits almost everything from MNIST, the processed
outputs actual ended up to be the same files. This commit now
stores them at different files and also stores the class name when
saving them. I also added md5 sums.

@fmassa fmassa left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is awesome, thanks a lot for the PR!

There is only a small lint issue, but after that this is good to merge!

'train-labels-idx1-ubyte.gz': '25c81989df183df01b3e8a0aad5dffbe',
}


This comment was marked as off-topic.

Before this would lead FashionMNIST to contain mnist data:

```
MNIST(root, download=True)
FashionMNIST(root, download=True)
```

As MNIST and FashionMNIST are the same classes, the processed
outputs actual ended up to be the same files. This commit now
stores them at different files and also stores the class name when
saving them. I also added md5 sums.
@ls1xt
ls1xt force-pushed the fix-fashion-mnist branch from 1e23d10 to 436cb4d Compare October 26, 2018 17:47
@ls1xt

ls1xt commented Oct 26, 2018

Copy link
Copy Markdown
Contributor Author

I fixed the lint problem. Thank you!

@soumith
soumith merged commit 3621569 into pytorch:master Oct 26, 2018
@soumith

soumith commented Oct 26, 2018

Copy link
Copy Markdown
Contributor

thanks @berleon

@dizcza

dizcza commented Oct 30, 2018

Copy link
Copy Markdown
Contributor

Are you kidding? I've already fixed this problem in #601 ... omg.
You've broken so many things in this commit, I don't want to even mention them.

@fmassa

fmassa commented Oct 30, 2018

Copy link
Copy Markdown
Member

There seems to be indeed two PRs that conflicted with each other. I'm reverting this one, sorry for the trouble!

fmassa added a commit that referenced this pull request Oct 30, 2018
fmassa added a commit that referenced this pull request Oct 30, 2018
* Revert "move area calculation out of loop (#641)"

This reverts commit 62cbf0b.

* Revert "Fix FashionMNIST loading MNIST (#640)"

This reverts commit 3621569.
@fmassa

fmassa commented Oct 30, 2018

Copy link
Copy Markdown
Member

@berleon there were some nice functionalities from your PR that were reverted, like the tests and the md5 checks.
Could you send another PR with those improvements?
Thanks and sorry for the trouble!

rajveerb pushed a commit to rajveerb/vision that referenced this pull request Nov 30, 2023
* Update README.md

* Update README.md

Add Data download info

* [GPT3] Clean up README so it is easier for others to follow instructions

---------

Co-authored-by: Shriya Palsamudram <spalsamudram@nvidia.com>
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.

4 participants