sdv-dev
Change `.fit()` call to more closely align with other `.fit()` calls like keras and fastai.
As the title states, the .fit() call is different now in that the batch_size, nr_epochs are set in the object initialization rather than the start of training. To more closely align this call with other standards like keras and fastai, these should be moved.
Hi @Baukebrenninkmeijer
Thanks for the suggestion. Indeed, epochs could be moved to the fit method, as you might want to call fit multiple times with different epoch numbers.
However, I would not move the other arguments to the fit method, as they are values inherently related to the model instance, in the sense that you don't want to change them between fit calls.
Can we restrict this issue to epochs only?
Opinions @ManuelAlvarezC @leix28 ?
Hi, I opened this from #22, where @ManuelAlvarezC asked me to create a new issue.
I agree that batch_size is not the best example, cause it changes the model. However, I do think there are more parameters than just epochs that are best suited in the fit call, take for example learning rate, batches per epoch, and whether to save and/or restore checkpoints. I think that might also be all of them. Thoughts?
Hi, I opened this from #22, where @ManuelAlvarezC asked me to create a new issue.
Yes, I saw it! It was just to comment a bit and enrich the discussion.
I agree that we could put more things, not only epochs. But the things we put should be only those that you might want to change. Perhaps we can list candidates here?
About the ones you said, the batches per epoch and the and the learning rate are things that might change the overall convergence of the model, so I don't see them as things that you change in the middle of a fitting process. You start with one value and keep it until the end.
Saving and restoring could be there, I see no harm on that, but I do not see why this would be useful either, especially restore. You tell TGAN to either start from scratch or recover from a checkpoint when you create the TGAN instance, and from there, if you call fit multiple times you should always restore and continue from the previous call. If you want to start from scratch again, you just create a new instance, as you want it to be completely unrelated to the previous one.
Hi,
Thanks for open the issue @Baukebrenninkmeijer.
About the subject of discussion:
I think that moving max_epochs and steps_per_epoch to the fit method could make sense, however, I have to agree with @csala about restore_session and save_checkpoints that even if it can be done it doesn't seem specially useful.
Any other candidate to be moved to .fit()?
Hmm, I would like to add my perspective on not including the restore_checkpoints, lr and save_checkpoints. I see your point on them, which makes sense in classic development methods, but as someone who works almost exclusively in notebooks, having the .fit call have the flexibility to experiment with different settings at run time is very useful. Most notably, the lr parameters is very useful to quickly try out multiple different learning rates.