Added strict=False for load_from_checkpoint

[shijianjian]

What does this PR do?

Enabled functionality as

 model = system.load_from_checkpoint(ckpt_path, hparams_file=vars(hparams), strict=False)

Fixes #2629

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 your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
• Did you make sure to update the documentation with your changes?
• Did you write any new necessary tests?
• Did you verify new and existing tests pass locally with your changes?
• If you made a notable change (that affects users), did you update the CHANGELOG?

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 #2819 into master will increase coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #2819   +/-   ##
======================================
  Coverage      90%     90%           
======================================
  Files          81      81           
  Lines        7553    7553           
======================================
+ Hits         6796    6797    +1     
+ Misses        757     756    -1     

[Borda]

pls rebase on master and do not add removed/deprecated API

[williamFalcon]

@shijianjian can you add a test for this?

[pep8speaks]

Hello @shijianjian! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-08-13 19:51:47 UTC

[awaelchli]

Do I understand correctly, this PR also adds unrelated to strict kwarg the possibility to have hparams_file a dict? Why do we need that? Isn't it the same as passing the dict as kwargs?

[shijianjian]

Do I understand correctly, this PR also adds unrelated to strict kwarg the possibility to have hparams_file a dict? Why do we need that? Isn't it the same as passing the dict as kwargs?

@awaelchli The only reason that I included that is because of the hparams modification by hparams['on_gpu'] = False. I will remove those code if anyone can confirm that this specific modification is not needed.

[awaelchli]

Not sure why we override the on_gpu=False, I guess it is assumed we always load the checkpoint to CPU, which is probably not necessary. But I think that requires a different solution. The reason why I am not so convinced with the hprams file as a dict is:

1. Now there are two different ways to do the same thing, which in my opinion is not a great design. Will the user know that there is no difference?
2. Two ways of doing the same thing will require also twice the amounts of tests, essentially duplicated, and twice the amount of maintenance effort.
3. The meaning of the argname "hparams_file" is now misleading if it can be a dict

[shijianjian]

@awaelchli I have no objections to remove it since it is only used for my own project, as long as you confirmed that the hparams['on_gpu'] = False is not needed here.

I will remove the corresponding code soon.

Thank you.

[shijianjian]

Should be able to be merged.

[mergify]

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

[rohitgr7]

still, some pep8 issues are there.

outdated