search

meta-llama / llama-recipes

Scripts for fine-tuning Meta Llama with composable FSDP & PEFT methods to cover single/multi-node GPUs. Supports default & custom datasets for applications such as summarization and Q&A. Supporting a number of candid inference solutions such as HF TGI, VLLM for local or cloud deployment. Demo apps to showcase Meta Llama for WhatsApp & Messenger.
12.07k stars 1.93k forks source link

Configs like `train_config` should be named using PascalCase #722

Open vvolhejn opened 1 day ago

vvolhejn commented 1 day ago

🚀 The feature, motivation and pitch

Config dataclasses like llama_recipes.configs.training.train_config should be named using PascalCase, in this case TrainConfig. The current naming violates a widely accepted Python convention.

It also leads to patterns like from llama_recipes.configs import train_config as TRAIN_CONFIG here, where the symbol needs to be renamed upon import to avoid conflicts with a train_config variable, which is an instance of the train_config (TRAIN_CONFIG) class.

I'm happy to do the renames but I wanted to hear from the devs first. This change does break backwards compatibility but since llama-recipes is currently on version 0.0.4 I assume the whole API is still very experimental?

Alternatives

No response

Additional context

No response

mreso commented 19 hours ago

Hi @vvolhejn thanks for bringing this up. Avoiding to break bc was what lead to this outcome. This affects all config classes and we should make these changes to all configs simultaneously. Would be happy to review a PR. I am currently working on fixing the unit tests once again so we can test the proposed changes more reliably.