Improve collision check on hparams between LightningModule and DataModule

[ananthsub]

🚀 Feature

Motivation

With the recent ability to log hyperparameters on the datamodule, this exception was introduced in case the keys had overlaps between the lightning module and datamodule
https://github.com/PyTorchLightning/pytorch-lightning/blob/ec828b826717cd3b5beabcb6d0cacf41b2320a98/pytorch_lightning/trainer/trainer.py#L1043-L1053

However, this check can be overly strict: if the same hparams are shared across the LightningModule and DataModule, this will result in an error.

Pitch

Only raise an exception if there are overlapping keys which have different values across lightning module and datamodule

datamodule_hparams = self.datamodule.hparams_initial
lightning_hparams = self.lightning_module.hparams_initial
colliding_keys = lightning_hparams.keys() & datamodule_hparams.keys()
if colliding_keys:
    inconsistent_keys = []
    for key in colliding_keys:
        if lightning_hparams[key] != datamodule_hparams[key]:
            inonsistent_keys.append(key)
    if len(inconsistent_keys) > 0:
        raise MisconfigurationException(
            f"Error while merging hparams: the keys {inconsistent_keys} are present "
            "in both the LightningModule's and LightningDataModule's hparams and have different values."
        )
hparams_initial = {**lightning_hparams, **datamodule_hparams}

Alternatives

Additional context

---

If you enjoy Lightning, check out our other projects! ⚡

• Metrics: Machine learning metrics for distributed, scalable PyTorch applications.

• Flash: The fastest way to get a Lightning baseline! A collection of tasks for fast prototyping, baselining, finetuning and solving problems with deep learning

• Bolts: Pretrained SOTA Deep Learning models, callbacks and more for research and production with PyTorch Lightning and PyTorch

• Lightning Transformers: Flexible interface for high performance research using SOTA Transformers leveraging Pytorch Lightning, Transformers, and Hydra.

[tchaton]

Good catch ! Sounds like a good idea.

[ananthsub]

The workaround right now is, in one of the components, to call self.save_hyperparameters(..., logger=False) to avoid the collision