Bugfix/fix gan example

[lobantseff]

• Was this discussed/approved via a Github issue? (no need for typos and docs improvements)

What does this PR do?

Fixes #1223

GAN example in repo was not working. Fixed typos and working in dp, and ddp mode.

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

[ternaus]

Is there a way to implement caching of the generated images?

Without it, the code works twice slower as we need to generate both in generator and discriminator phases.

[williamFalcon]

@armavox we moved away from hparams.
Made the changes, verify?

[ternaus]

@williamFalcon is there a way to avoid doing self(z) in the discriminator phase and reuse images generated at the generator phase?

[lobantseff]

@williamFalcon ok. Good.

[lobantseff]

@ternaus you’re right. However, the problem is that in forward(), model class, which is used by distributed training is replicated every time, then the training_step() is called for each copycat. So it seems that there is the only way to store buffered variables - to save it in the initial class before replicating, e.g. to initialize variable self.generated_images = None in __init__, and then put there value from one of the copies. But each copy tends to generate its own generated_images, hence the use of common buffer seems to be incorrect.

PyTorch developers warn about that: https://pytorch.org/docs/stable/nn.html#dataparallel-layers-multi-gpu-distributed

Maybe someone can propose a good idea of how to overcome this and make its own buffer for each replica.

Best regards, Artem.

[williamFalcon]

@ternaus yes, easiest is to use ddp and NOT dp (dp is not recommended anyhow).

However, we're working on a fix to maintain state in dp @ananyahjha93

[ananyahjha93]

@williamFalcon thanks for pointing me to this updated GAN example.

[lobantseff]

@williamFalcon @ananyahjha93 may I see the branch, where are you working on that problem?

[ananyahjha93]

@armavox This is a [wip] commit but the current look of the solution is quite different from this. So, this is not a representation of the solution we are working on right now.

#1895