Fix Mixing hparams and arguments in LightningModule

[HenryJia]

Before submitting

• Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure to update the docs?
• Did you write any new necessary tests?
• If you made a notable change (that affects users), did you update the CHANGELOG?

What does this PR do?

Fixes #1468

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

[codecov]

Codecov Report

Merging #1505 into master will decrease coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #1505   +/-   ##
======================================
- Coverage      91%     91%   -0%     
======================================
  Files          67      67           
  Lines        3784    3782    -2     
======================================
- Hits         3439    3437    -2     
  Misses        345     345           

[Borda]

LGTM 🚀
just thinking that kwargs shall be also filtered if the parameter has such attribute

[HenryJia]

As in allow things in kwargs to override model_args?

[williamFalcon]

i say keep it simple as this PR modifies

[williamFalcon]

@HenryJia can you update the docs to show this?

[HenryJia]

@williamFalcon Yep, done

[Borda]

add note to changelog and think about adding test

[mergify]

This pull request is now in conflict... :(

[HenryJia]

@Borda Added note to changelog, looking into clean ways of adding a test for this...

[awaelchli]

The note you added in the documentation could be confusing. I wouldn't call these extra arguments hyperparameters. Because to me it reads like you can do load_from_checkpoint(file, learning_rate=2) and then magically somewhere in Lightningmodule access it with hparams like self.hparams.learning_rate. But clearly this is not what you try to achieve. You simply want to pass in some extra args, in addition to hparams dict. Apart from that, I don't see why this implementation prevents anyone passing in arguments that ARE included in the hparams dict.

A reformulation and an example in the docs would make it clear (at least for me).

[HenryJia]

@awaelchli Sounds good I'll change it slightly then. Regarding the example, there already is one in the docs. It's the last example of that section.

[awaelchli]

Awesome, it's cristal clear now. Thanks for the fix.