nanoGPT icon indicating copy to clipboard operation
nanoGPT copied to clipboard

Proposal for a slightly improved minimal configuration system

Open adonath opened this issue 1 year ago • 0 comments

Thanks @karpathy for this nice little project. I also really enjoyed watching the Youtube lecture that goes with it!

In my proposal I'm addressing the comment here https://github.com/karpathy/nanoGPT/blob/master/configurator.py#L12

A slight improvement over the existing system would be to rely on TOML configuration files, instead of Python files. TOML is part of the standard Python library since 3.11. The code replacement in train.py would roughly look like this:

import tomllib

a = 5
b = "beta"
c = True

config_str = """
a = 10
b = "alpha"
c = true
new = "test"
"""

config = tomllib.loads(config_str)

difference = set(config).difference(globals())

if difference:
    print(f"Not a configurable value: {difference}")

globals().update(config)

The main advantages are:

  • Remove the exec() statement, so it does not execute arbitrary Python code written in the config files.
  • Notify users of typos or values that cannot be configured.
  • The existing .py config files are basically valid TOML files. So can just be renamed.

I'd be happy to implement a solution along the lines suggested above, if welcome.

adonath avatar Jan 18 '23 15:01 adonath