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

Clean configs documentation #1944

Merged
merged 81 commits into from
Sep 4, 2024
Merged

Clean configs documentation #1944

merged 81 commits into from
Sep 4, 2024

Conversation

qgallouedec
Copy link
Member

@qgallouedec qgallouedec commented Aug 18, 2024

This PR focuses on standardizing and enhancing the configuration documentation across the codebase. Consistency and uniformity make maintenance easier and improve readability. Future PRs will extend these improvements to other components, particularly the trainers.

Key Changes

  • Line Wrapping: Applied a consistent line wrap at column 120 to improve readability.
  • Definite Articles: Removed definite articles where possible to streamline language.
  • Type Annotations:
    • Always include type definitions, indicating if a parameter is optional and specifying the default value.
    • Note that Optional means that the value can be None, and *optional* means that it is not required for the user to pass a value. Eg: For values that can be None:
      foo (`Optional[int]`, *optional*, defaults to `None`):
    • For values that can't be None.
      foo (`int`, *optional*, defaults to `4`):
  • String Defaults:
    • Ensured that default string values are wrapped in double quotes:
      defaults to `"foo"`
  • Dictionary Typing:
    • Replaced generic Dict type hints with more explicit Dict[str, Any] to clarify expected key-value pairs.
  • Default Value Formatting:
    • Consistently surrounded default values with backticks for improved formatting:
      defaults to `4`
  • Consistency Across Configurations: Ensured that similar arguments across different configurations have consistent descriptions
  • Consistent main docstring:

Overall, this is the template:

@dataclass
class FOOConfig(TrainingArguments):
    r"""
    Configuration class for the [`FOOTrainer`].

    Using [`~transformers.HfArgumentParser`] we can turn this class into
    [argparse](https://docs.python.org/3/library/argparse#module-argparse) arguments that can be specified on the
    command line.

    Args:
        foo (`Optional[str]`, *optional*, defaults to `None`):
            Description of foo.
        bar (str, *optional*, defaults to `"barbar"`):
            Description of the bar. This description can be long, but make sure you break the line so that the maximum
            length is 220.
        baz (`Optional[Dict[str, Any]]`, *optional*, defaults to `None`):
            Description of the baz.
    """
    foo: Optional[str] = None
    bar: str = "barbar"
    baz: Optional[Dict[str, Any]] = None

Progress

  • alignprop_config.py
  • bco_config.py
  • cpo_config.py
  • ddpo_config.py
  • dpo_config.py
  • kto_config.py
  • model_config.py
  • online_dpo_config.py
  • orpo_config.py
  • ppo_config.py
  • ppov2_config.py
  • reward_config.py
  • rloo_config.py
  • sft_config.py
  • OnPolicyConfig

Possibly breaking changes delayed for another PR

DDPOConfig and AlignProbConfignow inherits from TrainingArguments. It implies:

  • To avoid conflict with the super class, I've removed train_batch_size in favour of per_device_train_batch_size of the super class.
  • To avoid conflict with the super class, I've removed run_name which is already available in the super class.
  • output_dir is now a required argument

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@qgallouedec qgallouedec marked this pull request as ready for review September 3, 2024 22:04
trl/trainer/dpo_config.py Outdated Show resolved Hide resolved
@qgallouedec qgallouedec changed the title [WIP] Clean configs documentation Clean configs documentation Sep 3, 2024
@@ -125,14 +150,14 @@ class DPOConfig(TrainingArguments):
truncation_mode: str = "keep_end"
max_length: Optional[int] = None
max_prompt_length: Optional[int] = None
max_target_length: Optional[int] = None
max_completion_length: Optional[int] = None
Copy link
Member Author

Choose a reason for hiding this comment

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

I realized this was not consistent with other trainers. Changing it shouldn't be breaking, right? Or maybe add a post-init warning?
@kashif @edbeeching @lewtun

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add a post-init warning.

@qgallouedec qgallouedec merged commit fc20db8 into main Sep 4, 2024
10 checks passed
@qgallouedec qgallouedec deleted the clean-config branch September 4, 2024 08:07
@qgallouedec qgallouedec mentioned this pull request Nov 15, 2024
5 tasks
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

Successfully merging this pull request may close these issues.

4 participants