Fix LightningCLI failing when both module and data module save hyperparameters #20221
Conversation
…arameters due to conflicting internal parameter
mauvilsa
requested review from
lantiga,
Borda,
tchaton,
awaelchli and
justusschock
as code owners
| @@ -69,6 +69,9 @@ def _log_hyperparams(trainer: "pl.Trainer") -> None: | |||
| lightning_hparams = pl_module.hparams_initial | |||
| inconsistent_keys = [] | |||
| for key in lightning_hparams.keys() & datamodule_hparams.keys(): | |||
| if key.startswith("_"): | |||
| # Prevent merging LightningCLI's internal hparams and discourage of private param names | |||
There was a problem hiding this comment.
can we throw a warning here when a private param is skipped instead of doing it silently?
There was a problem hiding this comment.
I was adding a warning, but then realized that if LightningCLI is used, then there would always be a warning. So now I changed the code so that only the LightningCLI internal params are skipped and removed from logging.
There was a problem hiding this comment.
Also only skipping _class_path because if _instantiator is skipped, then test_lightning_cli_load_from_checkpoint_dependency_injection fails. I don't understand why, but anyway _class_path is the only problem in #20182.
lantiga
added
the
waiting on author
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #20221 +/- ##
=========================================
- Coverage 88% 81% -8%
=========================================
Files 267 264 -3
Lines 23203 23152 -51
=========================================
- Hits 20526 18744 -1782
- Misses 2677 4408 +1731
|
What does this PR do?
Fixes #20182
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Reviewer checklist
📚 Documentation preview 📚: https://pytorch-lightning--20221.org.readthedocs.build/en/20221/