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

Switch mnist dataset mirror to a more reliable one #1201

Merged
merged 2 commits into from
Mar 17, 2021

Conversation

tejuafonja
Copy link
Collaborator

The current mnist dataset mirror used in the cleverhans_v3 sometimes sporadically return 503. This issue was first noted in 2018 (#496) and considered resolved since everything seem to be working fine since then.
Yesterday, @orlp reported this issue again. I looked into it and it's quite sporadic, returning a 503 every once in a while. I thought it best to switch the url once and for all.
Since we're using the "https://storage.googleapis.com/cvdf-datasets/mnist/" in our jax tutorial, I wanted to stay consistent with that and changed all referenced " http://yann.lecun.com/exdb/mnist/" to "https://storage.googleapis.com/cvdf-datasets/mnist/". This is the same url referenced in the tf datasets class.

The current mnist dataset mirror used in the cleverhans_v3 sometimes sporadically return 503. This issue was first noted in 2018 (cleverhans-lab#496) and considered resolved since everything seem to be working fine since then.
Yesterday, @orlp reported this issue again. I looked into it and it's quite sporadic, returning a 503 every once in a while. I thought it best to switch the url once and for all.
Since we're using the "https://storage.googleapis.com/cvdf-datasets/mnist/" in our jax tutorial, I wanted to stay consistent with that and changed all referenced " http://yann.lecun.com/exdb/mnist/" to "https://storage.googleapis.com/cvdf-datasets/mnist/". This is the same url referenced in the tf datasets class.
Copy link
Member

@npapernot npapernot left a comment

Choose a reason for hiding this comment

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

Thanks Teju!

@npapernot
Copy link
Member

You will need to resolve conflicts: are you working from the last commit? Could be easier to open a new PR if that’s not the case since this is 2 line diff

@tejuafonja
Copy link
Collaborator Author

Thanks, Nicolas - now resolved.

You will need to resolve conflicts: are you working from the last commit? Could be easier to open a new PR if that’s not the case since this is 2 line diff

Copy link
Collaborator

@jonasguan jonasguan left a comment

Choose a reason for hiding this comment

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

Looks great!

@tejuafonja tejuafonja merged commit 42fc431 into cleverhans-lab:master Mar 17, 2021
@tejuafonja tejuafonja deleted the switch_mnist_mirror branch March 17, 2021 13:06
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