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

Two bugfixes, improved err msg, and whitespace removal #63

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

Conversation

catherio
Copy link

Two bugfixes:

  • set_shape instead of _shape
  • don't squeeze away all singleton dimensions before matmul, leave batch dimension intact

Improved error message explaining how to download CIFAR-10

Whitespace removal (sorry, my editor does this automatically, and I assume you don't mind)

@igul222
Copy link
Owner

igul222 commented Mar 28, 2018

Thanks for the PR :) Re. the Inception Score changes, are those bugs also present in the canonical implementation (https://github.com/openai/improved-gan/blob/master/inception_score/model.py)? If so then I'm hesitant to change because enough people have reported results with them that they're de-facto part of the eval metric now.

@catherio
Copy link
Author

Ah, I hadn't realized this code was almost verbatim copied from the openai version. Yes, these two bugs are also there. They're not the type of bugs that cause the score to be different, though, they're stacktrace errors that mean the program just doesn't run (anymore?). I'll go file this same PR against their implementation and see what they say.

@catherio
Copy link
Author

PR on the canonical implementation is here: openai/improved-gan#31

@catherio
Copy link
Author

FYI, the changes proposed are now also in the canonical model. No numerical or semantic changes, merely compatibility. openai/improved-gan#31

@wchen342
Copy link

The o.set_shape(tf.TensorShape(new_shape)) part actually doesn't work (Tensorflow won't allow you to change the first dimension to None), and that's why you need bs=1 instead of 100. I suggest using o.shape.dims[0] = tf.Dimension(None) instead. See wchen342/SketchyGAN@f8fe961.

@farhodfm
Copy link

@catherio Thank you for your changes!

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