mosaicml / composer

Supercharge Your Model Training
http://docs.mosaicml.com
Apache License 2.0
5.14k stars 416 forks source link

Remove `Event.INIT`; move `Event.TRAINING_START` to before the DDP #157

Closed ravi-mosaicml closed 2 years ago

ravi-mosaicml commented 2 years ago

Once #43 is merged in, then Event.INIT can be removed.

Event.TRAINING_START can be moved up to the line right before ddp.prepare_module(). All existing algorithms that currently run on training start would be compatible with this change.

TODO:

jbloxham commented 2 years ago

I'm not sure how I feel about removing INIT. I definitely agree it's hacky how Event.INIT currently needs to exist between model initialization and optimizer initialization to accommodate surgery, but #43 will allow us to fix those issues without just removing the entire event. I think INIT should be called at the end of Trainer initialization, and TRAINING_START should be called after Trainer.fit() is called. Note also that prepare_ddp_module() is now called within Trainer.__init__(), not Trainer.fit()

ravi-mosaicml commented 2 years ago

Surgery is fixed in #249, so we can now address this issue.

ravi-mosaicml commented 2 years ago

Addressed in #263. Kept Event.INIT; renamed Event.TRAINING_START to Event.FIT_START