Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support both Namespace and dict for hyperparameter saving/loading. #943

Closed
awaelchli opened this issue Feb 25, 2020 · 3 comments
Closed
Labels
feature Is an improvement or enhancement good first issue Good for newcomers help wanted Open to be worked on
Milestone

Comments

@awaelchli
Copy link
Contributor

🚀 Feature

Let the user pass a dict to LightningModule so that after model saving it can be restored using load_from_checkpoint or load_from_metrics.

Motivation

Currently, there is nothing that prevents the user from passing in hyperparameters to LightningModule via dictionary (or even somthing else). However, the model loading/saving assumes it is always a argparse.Namespace. This could potentially be an issue when load_from_checkpoint restores the module with a Namespace passed in instead of dict.

Pitch

The model saving currently converts Namespace to dict and the model loading converts it back to Namespace.

Pitch 1: Also save the type inside the checkpoint, e.g.,

checkpoint['hparams'] = dict(hparams)
checkpoint['hparams_type'] = type(hparams)

and when restoring we instantiate with the appropriate type.

Pitch 2: Dump the whole hparams object (Namespace, dict, ...) into checkpoint without converting it to dict first and let pickle take care of the rest. Most flexible option but could give problems when loading from checkpoint.

Alternatives

Somehow restrict the user to only use argparse.

Additional context

The idea was suggested by @williamFalcon in PR #919.

@awaelchli awaelchli added feature Is an improvement or enhancement help wanted Open to be worked on labels Feb 25, 2020
@williamFalcon
Copy link
Contributor

support both argparse and dict...

@awaelchli awaelchli changed the title Allow hparams in LightningModule to be dictionary Support both Namespace and dict for hyperparameter saving/loading. Feb 25, 2020
@Borda Borda added the good first issue Good for newcomers label Feb 25, 2020
@awaelchli
Copy link
Contributor Author

partially related to #525

Also, there was an attempt to pickle the hparams directly but it never got merged: #615
@neggert do you have any thoughts about this? would pickling the hparams object directly work?

@Borda Borda added this to the 0.7.0 milestone Mar 3, 2020
@awaelchli
Copy link
Contributor Author

implemented in #1029

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement good first issue Good for newcomers help wanted Open to be worked on
Projects
None yet
Development

No branches or pull requests

3 participants