Deprecate the Trainer

[avik-pal]

Background

TorchGAN started out with a primary focus on GANs. But recent literature has suggested that GANs are being used in time series modeling, NLP, etc. Hence the name Trainer sends the wrong message.

Proposal

1. Deprecate Trainer and ParallelTrainer in v0.0.4 and completely remove them in v0.0.5+
2. Introduce an ImageTrainer and ParallelImageTrainer which are exact copies of the current version of Trainer and ParallelTrainer.
3. Introduce application-specific Trainers to broaden the scope of TorchGAN. RGAN and RCGAN implementation #110 discusses this.

[dsevero]

I can start on this in about 2 weeks if nobody else gets on it :)

[dsevero]

What do you guys think of using pytorch-lightning to refactor the trainer?

[avik-pal]

Yes, I looked at it a couple of days back. It provides a really good baseline to redesign our Trainer. It also helps remove all the boilerplate logging code and also benefit from new additions to the training techniques there.

Having said that, I am still not comfortable with completely dropping the vanilla pytorch based trainer. Maybe we can shift to a pytorch-lightning based Trainer and move the entire current Training + Logging code to a legacy module.

[dsevero]

I vote for fully dropping the current Trainer only on a new major release. How about the following:

Currently we have:

• BaseTrainer()
• Trainer(BaseTrainer)
• ParallelTrainer(BaseTrainer)

We should move towards:

• BaseTrainer() -> LegacyBaseTrainer()
• Trainer(BaseTrainer) -> LegacyTrainer(LegacyBaseTrainer)
• ParallelTrainer(BaseTrainer) -> LegacyParallelTrainer(LegacyBaseTrainer)

New stuff

• PyLightningTrainer()
• SequentialTrainer(PyLightningTrainer)
• ImageTrainer(PyLightningTrainer)

Scaling training is handled by pytorch-lightning.

Do we move the logging module into legacy also?

[avik-pal]

LGTM only I think instead of doing BaseTrainer() -> LegacyBaseTrainer() and the other 2, we should do trainer.BaseTrainer() -> legacy.trainer.BaseTrainer(). Its easier to modify the code IMO.

The logger can then be moved to legacy.logging module as we don't need the logger for pylightning as far as I have seen their code.