new LightningModule hook "configure_callbacks"

[awaelchli]

What does this PR do?

Fixes #5615

Introduces a configure_callbacks hook in LightningModule
Possible implementations:

Option 1: "appending" (CURRENT IMPLEMENTATION ON THIS PR)
Simply appens the returned callbacks. Existing callbacks cannot be modified directly.

def configure_callbacks(self):
    my_callback = MyCallback()
    return [my_callback]

# this is what the trainer would do:
trainer.callbacks.extend(model.configure_callbacks())

Option 2: "replacement"
Allows to define the order and to modify existing callbacks.

def configure_callbacks(self, callbacks):
    my_callback = MyCallback()
    return [callbacks] + callbacks

# this is what the trainer would do:
trainer.callbacks = model.configure_callbacks()

Option 3: "inplace modification"
Allows to define the order and to modify existing callbacks.

def configure_callbacks(self, callbacks):
    my_callback = MyCallback()
    callbacks.append(my_callback)
    # no return

# this is what the trainer would do:
model.configure_callbacks(trainer.callbacks)

Options 2 and 3 give the user more control over how callbacks are used and in which order they are executed.
However, if the LightningModule modifies the the callbacks passed to the Trainer, it is less transparent what code actually gets executed. On the other hand, a user can mutate the callbacks list in the trainer anyway through Trainer.callbacks, if they want to.

TODOs:

• decide on the interface, discussion needed (see proposed options below)
• update docs
• find the best place to invoke the hook
• add tests
• add changelog
• Merge Force ModelCheckpoint callback to run last #5731, ensures that model checkpoints will run last

Poll options

1. • ❤️
2. • 🎉
3. • 🚀

[akihironitta]

+1 for Option 1: "appending" since it is simple and similar to configure_optimizers() so that users don't get confused. Also, I'm curious about the motivation for adding this feature...

[awaelchli]

I'm curious about the motivation for adding this feature...

This was requested internally, see grid chat. The motivation is to have model-specific callbacks defined in the model, making it more portable.

[ananthsub]

@awaelchli what are the implications for checkpoint loading/saving? how are callback states treated here vs callbacks attached to the trainer? Asking since it came up in #5542

[awaelchli]

@ananthsub The callbacks attached through this hook here will still go into the trainer as if they were added directly to the Trainer.
Therefore, the same rules and limitations apply as for the Trainer.
As a general rule of thumb, the callbacks that do have state should not change their class name between saving and loading (as this is the key by which they are saved to the checkpoint and retrieved) and there can only be one callback of each type.
Callbacks that don't save state do not have these limitations, since they don't get saved.

[awaelchli]

@PyTorchLightning/core-contributors would you find such a hook to configure model-specific callbacks directly in the LightningModule useful and if so which API would you prefer (3 choices above)?

[rohitgr7]

option 1 looks better to me since trainer callbacks can be configured/modified using self.trainer.callbacks if required.

This was requested internally, see grid chat.

what's grid chat??

[carmocca]

I agree with @akihironitta

[s-rog]

While I would personally prefer 2 for maximum flexibility, I agree with Akihiro that it could be unintuitive for users.

[teddykoker]

I think we can start with 1, and we can always add 2 later if it becomes heavily requested

[Borda]

@akihironitta @teddykoker @ananthsub @rohitgr7 @s-rog
I have added a small poll ❤️ 🎉 🚀 on the description, so please check it there 🐰

[Borda]

could it be a mix of the first two such that there will be an extra argument that would specify if the model CB is added as first or as last? ideally you would like to define some kind of the insert but it is maybe overkilled atm 🔢

[SeanNaren]

I'd agree with option 1, with 2 being the future requirement for callbacks that interfere with each other!

[awaelchli]

@akihironitta @Borda why does yapf format it like this?
is it a bug?

[Borda]

that looks strange... :/

[awaelchli]

same here, for some reason there are two elements on one line and the rest on the others. I thought it is supposed to give consistent formatting

[Borda]

yes, this looks strange...

[tchaton]

LGTM !