Lightning-AI / pytorch-lightning

Pretrain, finetune and deploy AI models on multiple GPUs, TPUs with zero code changes.
https://lightning.ai
Apache License 2.0
27.93k stars 3.34k forks source link

Deprecate summarize() off LightningModule #8478

Closed ananthsub closed 3 years ago

ananthsub commented 3 years ago

🚀 Feature

Background We are auditing the Lightning components and APIs to assess opportunities for improvements:

One item that came up was summarize() defined on the LightningModule. This does not need to be on the core API.

Objective Instead, we'd like to abstract this out into a separate utility function which accepts a LightningModule and prints out the model summary

Motivation

Pitch

Extensions

Define a ModelSummary Callback which calls this utility function. A callback in Lightning naturally fits this extension purpose. It generalizes well across lightning modules, has great flexibility for when it can be called, and allows users to customize the summarization logic (e.g. integrate other libraries like torchsummary more easily).

Why do we want to remove this from the core trainer logic?

Offering this as a callback allows us to deprecate weights_summary from the Trainer constructor. This benefits the project by reducing the number of custom constructor args the trainer accepts in favor of a generic one (via the callbacks argument)

Another extension:

Alternatives

Keep as is

Additional context

awaelchli commented 3 years ago

Hey @ananthsub thanks for setting this in motion.

Offering this as a callback allows us to deprecate weights_summary from the Trainer constructor. This benefits the project by reducing the number of custom constructor args the trainer accepts in favor of a generic one (via the callbacks argument)

I am not convinced about it. If we remove it then there won't be any model summary by default and a user has to explicitly add the callback (opt-in). By this reasoning, we would then also remove the progress bar etc. Isn't it a good thing that Lightning provides reasonable defaults that benefits most users? By turning these features off we must be confident that most users actually want them off and are actively turning them off. Do we have such evidence?

ananthsub commented 3 years ago

By turning these features off we must be confident that most users actually want them off and are actively turning them off. Do we have such evidence?

@edenafek - how could we get this evidence from users to confirm what the preferred experience should be? A poll on GitHub / slack?

I think the options are:

The desire is that customizations and extensions (as evidenced by the recent weights_summary deprecation in favor of max_depth) should happen at the per-component level instead of adding more args to the Trainer. As these extensions get more complex, relying on the Trainer to supply all reasonable defaults becomes harder to do. Additionally, the framework must maintain the documentation for those defaults and when they're applied, which still requires work for users to follow.

ananthsub commented 3 years ago

Followed #9043 to discuss the extensions here