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

Make it possible to specify multiple config files #54

Merged
merged 1 commit into from
Feb 18, 2025
Merged

Make it possible to specify multiple config files #54

merged 1 commit into from
Feb 18, 2025

Conversation

EntilZha
Copy link
Contributor

@EntilZha EntilZha commented Feb 12, 2025

Summary:

Make it possible to specify multiple config files.
Parsing CLI is not a special case anymore, just uses the same config inheritance method.

Test Plan:

Test that this iterpolates in the right order via unit tests

Sample usage, loads the internal config, which references bytelatent/configs/entropy_model.yaml. The precendence order is:

  • Default pydantic args
  • Included configs, eg config
  • CLI args
python -m bytelatent.print_config config=internal/configs/entropy_model.yaml eval=null

Summary:

Test Plan:

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Feb 12, 2025
@sriniiyer sriniiyer changed the base branch from main to pr53 February 13, 2025 02:02
file_cfgs.append(file_cfg)

if "configs" in cli_args:
for c in cli_args["configs"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

does this work recursively? Can you make it work recursively?

@EntilZha EntilZha changed the base branch from pr53 to main February 14, 2025 21:03
@EntilZha EntilZha force-pushed the pr54 branch 4 times, most recently from aa78c96 to f94babc Compare February 14, 2025 22:51
This was referenced Feb 14, 2025
Copy link
Contributor

@sriniiyer sriniiyer left a comment

Choose a reason for hiding this comment

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

lgtm

Summary:

Make it possible to specify multiple config files.
Parsing CLI is not a special case anymore, just uses the same config inheritance method.

Test Plan:

Test that this iterpolates in the right order via unit tests

Sample usage, loads the internal config, which references bytelatent/configs/entropy_model.yaml. The precendence order is:

- Default pydantic args
- Included configs, eg `config`
- CLI args

```
python -m bytelatent.print_config config=internal/configs/entropy_model.yaml eval=null

```


Summary:

Test Plan:
@EntilZha EntilZha merged commit 82ab593 into main Feb 18, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants