Raise a warning when nn.Module instance is saved with save_hyperparameters()

[rohitgr7]

What does this PR do?

Addresses: #11834 (comment)

nn.Modules are not hparams and when someone uses LM as systems, they might forget to ignore the arguments and they will be saved inside hparams thus will increase the size of the checkpoint.

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

• Was this discussed/approved via a GitHub issue? (not for typos and docs)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure your PR does only one thing, instead of bundling different changes together?
• Did you make sure to update the documentation with your changes? (if necessary)
• Did you write any new necessary tests? (not for typos and docs)
• Did you verify new and existing tests pass locally with your changes?
• Did you list all the breaking changes introduced by this pull request?
• Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

• Is this pull request ready for review? (if not, please submit in draft mode)
• Check that all items from Before submitting are resolved
• Make sure the title is self-explanatory and the description concisely explains the PR
• Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

cc @Borda

[awaelchli]

It's probably ok to do this, but we should stop here. There are many other parameter type that may or may not be desired to serialize, however we can't do the guess work for everything.

Note: If we go with this PR, there is no way for the user to force what is being ignored here. I hesitate to accept here, because normally we shouldn't make such assumptions for the user (the framework should not be limiting them).

[rohitgr7]

It's probably ok to do this, but we should stop here. There are many other parameter type that may or may not be desired to serialize, however we can't do the guess work for everything.

Note: If we go with this PR, there is no way for the user to force what is being ignored here. I hesitate to accept here, because normally we shouldn't make such assumptions for the user (the framework should not be limiting them).

I agree, my idea here was to avoid only nn.Modules because that's already been captured when we save state_dict inside the checkpoint. Other different types of arguments will not be captured automatically.

I was running t5-small, and it got stuck due to this, with no errors at all, so it was hard to figure out what went wrong here until I did some debugging with @krshrimali .

We can mention this inside docstrings if required.

[carmocca]

I'm also hesitant about this.

I was running t5-small, and it got stuck due to this, with no errors at all, so it was hard to figure out what went wrong here until I did some debugging with @krshrimali .

What if we focus instead on notifying the user about the associated problems? We can suggest excluding the keys with save_hyperaprameters

[rohitgr7]

I'm also hesitant about this.

I was running t5-small, and it got stuck due to this, with no errors at all, so it was hard to figure out what went wrong here until I did some debugging with @krshrimali .

What if we focus instead on notifying the user about the associated problems? We can suggest excluding the keys with save_hyperaprameters

well, that's a good suggestion. works for me since it will happen during init. If everyone agrees on this, I'll update the PR :)

[krshrimali]

LGTM, thanks Rohit!