larq / zookeeper

A small library for managing deep learning models, hyperparameters and datasets
Apache License 2.0
23 stars 10 forks source link

Dictionary fields #289

Open jneeven opened 3 years ago

jneeven commented 3 years ago

Feature motivation

If you have multiple variants of some kind of component, e.g. multiple preprocessing functions, it would be very nice to store them all in a dictionary rather than having to define separate fields (which is error-prone especially as component classes get bigger, subclass other components etc)

(CC @CNugteren)

Feature description

Would be nice to be able to do something like this:

preprocessing: Dict[str, Preprocessing] = {
    "default": ComponentField(),
    "custom": ComponentField(),
}

Of course the fields would still have to be CLI overridable, e.g. MyComponent.preprocessing.default.some_attribute="value"

Feature implementation

I don't think this will necessarily be easy, but I also don't think it should be too difficult. We'll mainly have to make sure the config "paths" are handled correctly and the inheritance works properly. I'd hope this could be done in a day or so.

AdamHillier commented 3 years ago

I'm not quite sure I understand the motivation exactly — to be clear, you don't want the dict itself to be modifyable (i.e. don't want to be able to add new keys to the dict dynamically)?

jneeven commented 3 years ago

to be clear, you don't want the dict itself to be modifyable (i.e. don't want to be able to add new keys to the dict dynamically)?

Being able to add keys would be great, but I'd already be very happy just to be able to CLI-modify the key-value pairs that are already provided in the code (for tuning purposes). With the example above, the current solution would be resort to something like

default_preprocessing = ...
custom_preprocessing = ...

Which is very much suboptimal and also doesn't allow specifying new "keys" through the CLI

AdamHillier commented 3 years ago

I guess that's my question then: assuming that we don't allow key insertion, why would the current approach be suboptimal?

Another question I have is how subclassing would work with overriding field defaults: if B subclasses A but only wants to change the default for preprocessing["custom"], how would that work?

jneeven commented 3 years ago

Another question I have is how subclassing would work with overriding field defaults: if B subclasses A but only wants to change the default for preprocessing["custom"], how would that work?

I'd say it wouldn't, if you change anything you'd need to change the entire thing. Of course being able to just add and change fields would be convenient, but it'd become a mess very quickly. I don't think we could support the case you describe without simultaneously removing support for changing the entire field (e.g. deleting keys), which seems a more important feature to have