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

Lightweight Hyperparameter Datastructure #1871

Closed
awaelchli opened this issue May 18, 2020 · 10 comments
Closed

Lightweight Hyperparameter Datastructure #1871

awaelchli opened this issue May 18, 2020 · 10 comments
Labels
feature Is an improvement or enhancement help wanted Open to be worked on

Comments

@awaelchli
Copy link
Contributor

awaelchli commented May 18, 2020

🚀 Feature

A simple and flexible way to store hyperparameters in a dict/Namespace-like object.

Motivation

Pitch

An object that behaves like this:

# just like Namespace
hparams = Hyperparameters(x=1, y=2)  
# or from a dict
hparams = Hyperparameters({"x": 1, "y": 2})  
# it could support nesting
hparams = Hyperparameters({"x": 1, "y": {"a": 3}}) 

# Namespace-like look up
x = hparams.x
a = hparams.y.a
# or like a dict
x = hparams["x"]
a = hparams["y"]["a"]

# we will have to check for invalid keys
hparams["batch_size"]  # ok
hparams["batch-size"]  # error
hparams["batch size"]  # error

Optional features:

# could support reading from files
# useful for checkpoint loading
hparams = Hyperparameters.from_yaml("file.yml")

# could support flattening/sanitizing nested structure
# useful for logging to TensorBoard
# or to make it pickleable
clean = hparams.flatten().sanitize()

# note that these internal methods will prevent these keys:
hparams.flatten = True  # Problem, because not hyperparameter!

Pro:

  • Behaves like dict and Namespace in one, so it will work out of the box with PL since internally we treat it as dict anyway (saving checkpoint).

Contra:

  • Any convenience methods/attributes we add to the class will collide with hyperparameters names. See example above.
  • Need to spend extra effort making sure user does not pick bad dict keys

Considerations

  • "Hyperparameters" may be a too specific name for such a general datastructure.

Additional context

Discussed on slack and idea has popped up in other isses as well.

Related to #1841, #1735 and would solve #1737

@awaelchli awaelchli added feature Is an improvement or enhancement help wanted Open to be worked on labels May 18, 2020
@awaelchli
Copy link
Contributor Author

Also post more ideas here.

@Borda
Copy link
Member

Borda commented May 18, 2020

About the name we can have inside auto replacement of any separator ( , ,) by _

@yukw777
Copy link
Contributor

yukw777 commented May 18, 2020

Should we just use OmegaConf for this? It might make it easier to integrate Hydra down the line too.

@miriaford
Copy link

Completely agree with @yukw777 . OmegaConf does all of the above and better, PL shouldn't reinvent the wheel.

@awaelchli
Copy link
Contributor Author

I guess the idea was to provide a basic datastructure that works without dependency on another library. A simple wheel that rolls and does nothing more, created with tools that we already have :)

@yukw777
Copy link
Contributor

yukw777 commented May 18, 2020

Maybe we could use ConfigParser? It's part of the standard library, so no external dependency. Its interface is not as simple as what @awaelchli has envisioned though.

@Borda
Copy link
Member

Borda commented May 18, 2020

@omry
Copy link
Contributor

omry commented May 24, 2020

We need to do our arg casting so it would be nice to wrap it in one object

https://github.com/PyTorchLightning/pytorch-lightning/blob/981169cacc5da17af8d796d94b747c17566ef0bc/pytorch_lightning/trainer/trainer.py#L723

What do you get from the Namespace object that you are not getting from OmegaConf DictConfig?

In other words, DictConfig is most likely a drop-in replacement to Namespace. (with the exception of explicit type checks).

@awaelchli
Copy link
Contributor Author

awaelchli commented May 24, 2020

I didn't know OmegaConf when I opened the issue.
Anyway, the PL team decided to simplify the way hyperparameters are passed into the model so there is no need for an object like this anymore. Closing this. Thanks for the discussion.

@drozzy
Copy link

drozzy commented Jun 1, 2020

Maybe pydantic? (just my 2c).

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 help wanted Open to be worked on
Projects
None yet
Development

No branches or pull requests

6 participants