FluxML / model-zoo

Please do not feed the models
https://fluxml.ai/
Other
907 stars 330 forks source link

Problem with MNIST DCGANs tutorial #391

Open LucasMSpereira opened 2 years ago

LucasMSpereira commented 2 years ago

Trying to get into Flux.jl, I found this tutorial about using DCGANs with MNIST. After going through the tutorial and transferring all the code to a file, the generator didn't seem to perform very well. In the end, the generated images are just grey with some tiny fluctuations. The image includes plots of both losses. My code (inside .zip) basically only differs from the tutorial in trying to plot the histories of the losses.

TrainFluxTutorial FluxDCGANtutorial.zip

mcabbott commented 1 year ago

Comments above refer to the version on fluxml.github.io, which https://github.com/FluxML/fluxml.github.io/pull/156 has deleted. Moving the issue to the model zoo.

dmetivie commented 10 months ago

I ran into the same problem with the current official doc tutorial.

After a lot of investigation (and being unfamiliar with Flux and API changes), I figured that the code showed there had been updated to match the new API by this commit.

I think the official tutorial need update, it is very confusing for new Flux user. At least a warning should be thrown because the current code run but seems not to update the generator in training. This is probably linked to this issue so should I PR a new version of the doc with the fix of commit?

mcabbott commented 10 months ago

The top post links a version deleted from the website, the plan was to move these into the docs & update them. What you link as the "official tutorial" is the result of half this plan being done, but notice these are still hidden from the side-bar, as nobody got around to updating them (or better, arranging to have them automatically tested by CI).

As you say, the model zoo version was updated in #373. These changes could be copied back to the one in the docs. But perhaps it would be better just to delete the doc version, and have only the zoo version?

I think the version in the model zoo does run. But does it solve the issue reported here, "generated images are just grey with some tiny fluctuations"? I don't recall whether I looked into this in #373.

dmetivie commented 10 months ago

Yes it does solve the problem! I tried the code. In particular, it includes change in API like setup optimizer and update(model,...) not update(params, ...). I am not sure what exactly was happening in the version of the tutorial because no errors or warning showed.

Why not have a doc for model-zoo obtained from the scripts + literate.jl? It is more user/beginner-friendly than a GitHub Julia script. At least I liked the format of the doc tutorial with pictures and text. Of course, the current scripts would need some updates just to work with literate. Then motivated authors could add more (text, image). IMO removing tutorial format is a bit sad because there are too few in Julia for advanced examples like GAN.

ToucheSir commented 10 months ago

Literate.jl integration was attempted at one point, but people ran out of time/interest before it could be implemented. Given there's not enough time to go around to keep even a lot of the model zoo up to date, we'd need a pretty significant contribution to get that running.