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

Feature Request: Remove restriction to define all configurables before parsing config file? #4

Closed
colllin opened this issue Jan 11, 2019 · 5 comments

Comments

@colllin
Copy link
Contributor

colllin commented Jan 11, 2019

I'm working in a Jupyter notebook. I would like to import gin and gin.parse_config_file('config.gin') at the top of the notebook. Then, inside the notebook, I would like to define a @gin.configurable function which is only needed inside this notebook. Currently, when I do this, I get an error at the point where I parse the config file (before defining the configurable function):

ValueError: No configurable matching 'my_function'.
  In file "config.gin", line 3
    my_function.my_arg = 'my_string'

Is this behavior intentional? If so, why? (I'm fairly new to python, so it could be that there's a great reason for this, but it's not obvious to me.)

I think this feature request would make gin more flexible and easier to use in this setting, and it seems like a reasonable behavior given that gin already requires configurable names to be unique. It seems like one of the benefits of gin is providing deep configuration of your python functions and classes, meaning that I can, for example, configure the constructor of a particular class without changing any of the code which instantiates that class to know about the configuration. So it seems counter-intuitive, then, that the same library would require me to define all of my configurable functions and classes in advance of loading my config file, which essentially means that I need an executable script which is completely independent of any (configurable) function or class definitions.

Without knowing the technical complexity of the change, it seems like the only trade-off in supporting this feature request is that we would lose the ability to give this kind of error message (shown above) upon parsing a config file, because we wouldn't know in advance whether a particular configurable will ever be defined.

@colllin
Copy link
Contributor Author

colllin commented Jan 15, 2019

This might be another reason to support this: I'm trying to re-use a config file across scripts. For example, I have a test/debug script inside my "data" module, and my "data" module has its own config.gin file. I want to import some of the functions from this module (and their configuration) into my debug script, but if I don't import every configurable mentioned in config.gin, I get an exception:

ValueError: No configurable matching 'some_function_i_did_not_import'

I'm looking for some guidance here — it seems that I either need to create & maintain a separate config file for each script (and stop thinking of gin as configuration for my module), or maybe create some kind of configuration module that imports/defines all necessary configurables before parsing the configuration file, assuming python/gin will keep those configurations when I import those configurables into my main script.

@colllin
Copy link
Contributor Author

colllin commented Jan 16, 2019

I believe I've found a way around this for now. In my __init__.py, I've added:

import gin

from .some_python_file_containing_gin_configurables import *
from .another_python_file_containing_gin_configurables import *

import torch
DataLoader = gin.external_configurable(torch.utils.data.DataLoader)

gin.parse_config_file('config.gin')

This seems to have the added advantage that no one outside of the module needs to know about gin-config, nor do they need to import the functions/classes they need in any special way based on these __init__.py changes.

@sguada
Copy link
Collaborator

sguada commented Jan 17, 2019

What I recommend to do is to include the imports of the modules you use in the gin file.

I also recommend to use the module name when configuring a function or Class.

Ex:

# Gin file configuring module1 and module2
import module1
from folder import module2

module1.function.param = value
module2.function.paran = value

@sguada
Copy link
Collaborator

sguada commented Jan 17, 2019

Removing the restrictions about defining the functions before been able to configure them is quite problematic, since it would prevent error checking.

Just move the gin.parse_config() later in the colab.

@colllin
Copy link
Contributor Author

colllin commented Jan 24, 2019

Thanks @sguada — I had no idea you could import directly into config.gin.

@colllin colllin closed this as completed Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants