Pytorch-lightning: callbacks: some callback hooks can be cleaned up

[ydcjeff]

🚀 Feature

Motivation

There are some duplicate callback hooks which can be removed.

ON TRAIN START
ON EPOCH START          <-- same 1
ON TRAIN EPOCH START    <-- same 1
ON BATCH START          <-- same 2 
ON TRAIN BATCH START    <-- same 2
ON BATCH END
ON TRAIN BATCH END
ON BATCH START
ON TRAIN BATCH START
ON BATCH END
ON TRAIN BATCH END
ON VALIDATION START
ON VALIDATION EPOCH START
ON VALIDATION BATCH START
ON VALIDATION BATCH END
ON VALIDATION EPOCH END
ON VALIDATION END
ON EPOCH END
ON TRAIN EPOCH END
ON TRAIN END
ON FIT END
TEARDOWN
SETUP
ON FIT START
ON PRETRAIN ROUTINE START
ON PRETRAIN ROUTINE END
ON TEST START
ON TEST EPOCH START
ON TEST BATCH START
ON TEST BATCH END
ON TEST EPOCH END
ON TEST END
ON FIT END
TEARDOWN
TEARDOWN

Pitch

ON EPOCH START
ON TRAIN START
ON TRAIN EPOCH START
ON TRAIN BATCH START
ON TRAIN BATCH END
ON TRAIN EPOCH END
ON TRAIN END

ON VALIDATION START
ON VALIDATION EPOCH START
ON VALIDATION BATCH START
ON VALIDATION BATCH END
ON VALIDATION EPOCH END
ON VALIDATION END

ON EPOCH END
ON FIT END
TEARDOWN

SETUP
ON FIT START
ON PRETRAIN ROUTINE START
ON PRETRAIN ROUTINE END
ON TEST START
ON TEST EPOCH START
ON TEST BATCH START
ON TEST BATCH END
ON TEST EPOCH END
ON TEST END
ON FIT END
TEARDOWN
TEARDOWN

Alternatives

Due to various researches, the above hooks calling style can't be fit every needs.
But it would be intuitive to see like
(one thing is the user can think TRAIN / VALIDATION can be in a separate epoch)

ON TRAIN START
ON TRAIN EPOCH START
ON TRAIN BATCH START
ON TRAIN BATCH END
ON TRAIN EPOCH END
ON TRAIN END

ON VALIDATION START
ON VALIDATION EPOCH START
ON VALIDATION BATCH START
ON VALIDATION BATCH END
ON VALIDATION EPOCH END
ON VALIDATION END

ON FIT END
TEARDOWN

SETUP
ON FIT START
ON PRETRAIN ROUTINE START
ON PRETRAIN ROUTINE END
ON TEST START
ON TEST EPOCH START
ON TEST BATCH START
ON TEST BATCH END
ON TEST EPOCH END
ON TEST END
ON FIT END
TEARDOWN
TEARDOWN

Additional context

https://colab.research.google.com/drive/1xVpEVaMKsbnLL4cwLB60lhz8zL3FcWKo?usp=sharing

[tchaton]

Hey @ydcjeff,

I think we need them 👍 For me, I understand them this way (TRAIN, TEST, VAL, ALL). So we need the 4 of each.
But you are right, there is some inconsistency in the code where bach_start and batch_train_start are called together, but not for test and val.

Best,
T.C

[ydcjeff]

Yes, train hooks needs to be consistent with val and test hooks

[rohitgr7]

@awaelchli had some suggestions here. Same thing we can do for on_batch_start/on_batch_end if we redefine them to be called before and after every batch regardless of train/val/test. WDYT?

[ydcjeff]

I see! Yep they can be useful if the users want to do same functionality before and after of the batch. So, it would be like this?

ON EPOCH START
ON TRAIN START
ON TRAIN EPOCH START
ON BATCH START
ON TRAIN BATCH START
ON TRAIN BATCH END
ON BATCH END
ON TRAIN EPOCH END
ON TRAIN END

ON VALIDATION START
ON VALIDATION EPOCH START
ON BATCH START
ON VALIDATION BATCH START
ON VALIDATION BATCH END
ON BATCH END
ON VALIDATION EPOCH END
ON VALIDATION END

ON EPOCH END
ON FIT END
TEARDOWN

SETUP
ON FIT START
ON PRETRAIN ROUTINE START
ON PRETRAIN ROUTINE END
ON TEST START
ON TEST EPOCH START
ON BATCH START
ON TEST BATCH START
ON TEST BATCH END
ON BATCH END
ON TEST EPOCH END
ON TEST END
ON FIT END
TEARDOWN
TEARDOWN

But, I don't clearly get on_epoch_start/on_epoch_end. Would they be called before and after of epoch which contains train + val?

Or rather would they be called before and after of train/val/test epoch?
if that so, we should clearly mention in the docs that train + val is happening in the same epoch and on_epoch_start/on_epoch_end are being called before and after of train/val/test epoch?

ON TRAIN START
ON EPOCH START
ON TRAIN EPOCH START
ON BATCH START
ON TRAIN BATCH START
ON TRAIN BATCH END
ON BATCH END
ON TRAIN EPOCH END
ON EPOCH END
ON TRAIN END

ON VALIDATION START
ON EPOCH START
ON VALIDATION EPOCH START
ON BATCH START
ON VALIDATION BATCH START
ON VALIDATION BATCH END
ON BATCH END
ON VALIDATION EPOCH END
ON EPOCH END
ON VALIDATION END

ON FIT END
TEARDOWN

SETUP
ON FIT START
ON PRETRAIN ROUTINE START
ON PRETRAIN ROUTINE END
ON TEST START
ON EPOCH START
ON TEST EPOCH START
ON BATCH START
ON TEST BATCH START
ON TEST BATCH END
ON BATCH END
ON TEST EPOCH END
ON EPOCH END
ON TEST END
ON FIT END
TEARDOWN
TEARDOWN

[awaelchli]

I think it should be on_train_epoch_start, then on_epoch_start so that you can "chain" them and use both if needed :)
Unfortunately, changing the behaviour of on_epoch_start to be not only training epoch, but general, is a "silent" breaking change and I don't know how many users will be affected. Personally, I wouldn't mind.