Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ In configuronic, configurations are **closures** - callables that store both the
import configuronic as cfn

# Create a configuration
@cfn.config(batch_size=32, lr=0.001)
def create_optimizer(batch_size: int, lr: float):
return torch.optim.Adam(lr=lr)
@cfn.config(lr=1e-3, weight_decay=0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to expand the other sections (particularly resolution and relative resolution) to document this new functionality

def create_optimizer(lr: float, weight_decay: float):
return torch.optim.Adam(lr=lr, weight_decay=weight_decay)

# Override and create variants
fast_optimizer = create_optimizer.override(lr=0.01)
large_batch_optimizer = create_optimizer.override(batch_size=128)
fast_optimizer = create_optimizer.override(lr=1e-2)
regularaized_optimizer = create_optimizer.override(weight_decay=1e-3)

# Instantiate when needed
optimizer = fast_optimizer.instantiate()
Expand Down
7 changes: 6 additions & 1 deletion configuronic/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,11 @@ def _get_creator_module() -> ModuleType | None:
return module


def _get_default_argument_values(target: Callable) -> dict[str, Any]:
sig = inspect.signature(target)
return {name: param.default for name, param in sig.parameters.items() if param.default != inspect.Parameter.empty}


class Config:
def __init__(self, target, *args, **kwargs):
"""
Expand Down Expand Up @@ -231,7 +236,7 @@ def __init__(self, target, *args, **kwargs):
assert callable(target), f'Target must be callable, got object of type {type(target)}.'
self.target = target
self.args = [_resolve_value(arg) for arg in args] # TODO: cover argument override with tests
self.kwargs = {}
self.kwargs = _get_default_argument_values(target)
self._override_inplace(**kwargs)

self._creator_module = _get_creator_module()
Expand Down
12 changes: 12 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,15 @@ def identity(a, b):
cfn.cli(identity)
out, err = capfd.readouterr()
assert "This is a test function." in out


def test_cli_help_contains_function_default_values(capfd):
@cfn.config()
def identity(a=1, b=2):
print(a, b)

with patch('sys.argv', ['script.py', '--help']):
cfn.cli(identity)
out, err = capfd.readouterr()
assert "a: 1" in out
assert "b: 2" in out
62 changes: 62 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -913,5 +913,67 @@ def func(x):
cfn.Config(func, '.return1')


def test_config_config_default_values_are_resolved_properly():

@cfn.config()
def return1():
return 1

@cfn.config()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this PR mean that there's no need to have cfn.config decorator as a function (aka decorator factory) and we can just make it a usual decorator?

def add1(a=return1):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we imagine the non-config-decorated functions that take configs as arguments?

What I want to say is that I don't like passing configs as defaults to functions. I would propose to pass configs as defaults only to config decorators. Otherwise, the reader can really be confused, as the real default is not the config, but its instantiation.

return a + 1

assert add1.instantiate() == 2


def test_config_config_multiple_default_values_are_resolved_properly():

@cfn.config()
def returnx(x=1):
return x

return1 = returnx.override(x=1)
return2 = returnx.override(x=2)

@cfn.config()
def add1(a, b=return1, c=return2):
return f"{a} + {b} + {c}"

assert add1(a=0) == "0 + 1 + 2"


def test_config_default_function_kwargs_are_overriden_by_config_kwargs():
@cfn.config()
def return1():
return 1

@cfn.config()
def return2():
return 2

@cfn.config(a=return1)
def add1(a=return2):
return a + 1

assert add1.instantiate() == 2


def test_config_args_and_kwargs_kwargs_are_overriden_by_config_kwargs():
@cfn.config()
def return1():
return 1

@cfn.config()
def return2():
return 2

def func(*args, b=return2):
return f"{args} + {b}"

func = cfn.Config(func, 1, 2, b=return1)

assert func.instantiate() == "(1, 2) + 1"


if __name__ == '__main__':
pytest.main()
Loading