FEAT: Add CLIs in TRL ! #1419

younesbelkada · 2024-03-12T10:49:54Z

What does the PR do?

This PR introduces a new feature in TRL - CLIs for DPO and SFTTrainer!

trl sft --config config.yaml --output_dir test-llama

trl dpo --config config.yaml --output_dir test-llama

All arguments that are supported in ModelConfig should be supported by the CLI together with arguments from TrainingArguments or transformers.

Users will need to first call accelerate config before running the CLI to use custom accelerate configs

pyproject.toml

setup.py

lvwerra · 2024-03-12T11:14:47Z

trl/__init__.py

+else:
+    import sys
+
+    sys.modules[__name__] = _LazyModule(__name__, globals()["__file__"], _import_structure, module_spec=__spec__)


Does the lazy loading have any downsides? looks like a pretty dramatic change

In any case we should test this extensively

trl/commands/sft.py

lvwerra · 2024-03-12T11:20:41Z

trl/commands/sft.py

@@ -0,0 +1,148 @@
+# flake8: noqa


So i guess this means we can't use trl/examples/scripts/sft.py directly for this?

yeah for now but I think we can move the example folders there, let me think a bit

Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>

HuggingFaceDocBuilderDev · 2024-03-12T12:17:15Z

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.

lvwerra

Looks great, mainly left a comment on where we want to handle the config parsing :)

lvwerra · 2024-03-15T16:42:34Z

example_config.yaml

@@ -0,0 +1,20 @@
+# This is an example configuration file of TRL CLI, you can use it for 


what do you think about adding it in the cli or examples folder folder?

examples/scripts/dpo.py

lvwerra · 2024-03-15T17:02:34Z

setup.cfg

+[options.packages.find]
+include = examples/scripts/*.py


can also be removed, no?

lvwerra · 2024-03-15T17:14:40Z

trl/commands/cli.py

+        parser = HfArgumentParser((SftScriptArguments, TrainingArguments, ModelConfig))
+
+        (args, training_args, model_config, _) = parser.parse_args_into_dataclasses(return_remaining_strings=True)
+
+        if command_name not in SUPPORTED_COMMANDS:
+            raise ValueError(
+                f"Please use one of the supported commands, got {command_name} - supported commands are {SUPPORTED_COMMANDS}"
+            )
+
+        # Get the required args
+        config = args.config
+
+        # if the configuration is None, create a new `output_dir` variable
+        config_parser = YamlConfigParser(config, [args, training_args, model_config])
+        trl_examples_dir = os.path.dirname(__file__)
+
+        model_name = model_config.model_name_or_path


I wonder if it would be cleaner to just pass all the args as they are to the downstream script rather than parsing them here and then passing them as a string. We could add the logic to update the config with passed args inside the dpo.py and sft.py so they would also immediately profit from being able to be called with a config. wdyt?

Indeed the approach you suggested is much cleaner!

lvwerra · 2024-03-18T09:05:44Z

trl/commands/cli_utils.py

+class TrlParser(HfArgumentParser):
+    def __init__(self, args, training_args, model_config):
+        super().__init__((args, training_args, model_config))
+
+    def parse_args_and_config(self):
+        parsed_args, parsed_training_args, parsed_model_config, _ = self.parse_args_into_dataclasses(
+            return_remaining_strings=True
+        )
+
+        self.config_parser = YamlConfigParser(parsed_args.config)
+        args, training_args, model_config = self.config_parser.merge_dataclasses(
+            ((parsed_args, parsed_training_args, parsed_model_config))
+        )
+
+        training_args.gradient_checkpointing_kwargs = dict(use_reentrant=args.gradient_checkpointing_use_reentrant)
+        return args, training_args, model_config


can we make it a bit more agnostic such that we don't hardcode the order and the kind of args? e.g. like the HfArgumentParser does it. the chat interface for example will only have one dataclass to pass and maybe a future method might require 3 or 4.

lvwerra · 2024-03-18T09:06:42Z

trl/commands/config_parser.py

+import yaml
+
+
+class YamlConfigParser:


I would move this into cli_utils.py

lvwerra · 2024-03-18T10:31:12Z

trl/commands/cli_utils.py

+        for parser_dataclass in dataclasses:
+            if hasattr(parser_dataclass, "config"):
+                self.config_parser = YamlConfigParser(parser_dataclass.config)


we should check if we already parsed a config once and throw a warning/error if there are more than one dataclass with a config - otherwise there will weird behaviour (e.g. only the last config is applicable)

Makes totally sense - fixed it !

@lvwerra

* CLI V1 * v1 CLI * add rich enhancmeents * revert unindented change * some comments * cleaner CLI * fix * fix * remove print callback * move to cli instead of trl_cli * revert unneeded changes * fix test * Update trl/commands/sft.py Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com> * remove redundant strings * fix import issue * fix other issues * add packing * add config parser * some refactor * cleaner * add example config yaml file * small refactor * change a bit the logic * fix issues here and there * add CLI in docs * move to examples/sft * remove redundant licenses * make it work on dpo * set to None * switch to accelerate and fix many things * add docs * more docs * added tests * doc clarification * more docs * fix CI for windows and python 3.8 * fix * attempt to fix CI * fix? * test * fix * tweak? * fix * test * another test * fix * test * fix * fix * fix * skip tests for windows * test @lvwerra approach * make dev * revert unneeded changes * fix sft dpo * optimize a bit * address final comments * update docs * final comment --------- Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>

younesbelkada added 7 commits March 11, 2024 17:25

CLI V1

f0d29ce

v1 CLI

3a2283a

add rich enhancmeents

15d166e

revert unindented change

16463b8

some comments

d20167f

cleaner CLI

38ee375

fix

f83882c

lvwerra reviewed Mar 12, 2024

View reviewed changes

younesbelkada and others added 10 commits March 12, 2024 11:21

fix

14911d2

remove print callback

b7f96bc

move to cli instead of trl_cli

a328d9b

revert unneeded changes

4ee1c8e

fix test

55659ce

Update trl/commands/sft.py

459c3eb

Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>

remove redundant strings

171fd94

Merge branch 'add-cli' of https://github.com/lvwerra/trl into add-cli

54662da

fix import issue

fbec5ca

fix other issues

616ee60

younesbelkada added 11 commits March 12, 2024 12:26

add packing

553f898

add config parser

d098262

some refactor

6110423

cleaner

e9d4f91

add example config yaml file

265b488

small refactor

355e57c

change a bit the logic

8df993a

Merge remote-tracking branch 'origin/main' into add-cli

8cf0b05

fix issues here and there

f64618f

add CLI in docs

c33dead

move to examples/sft

0e45168

younesbelkada added 14 commits March 15, 2024 13:50

Merge branch 'add-cli' of https://github.com/lvwerra/trl into add-cli

d93a8e1

fix

d5ab9d6

tweak?

026ceef

fix

be1ec61

test

8600269

another test

ac99f35

fix

7252ad0

test

55eda92

fix

76dbe94

fix

e6678f3

fix

45424b8

skip tests for windows

2184b05

test @lvwerra approach

79a4074

make dev

c92dd3b

lvwerra reviewed Mar 15, 2024

View reviewed changes

younesbelkada added 3 commits March 15, 2024 17:24

revert unneeded changes

a477236

fix sft dpo

a1d228f

optimize a bit

ef144d0

lvwerra reviewed Mar 18, 2024

View reviewed changes

younesbelkada added 2 commits March 18, 2024 09:50

address final comments

c85b8e4

update docs

91a55ca

younesbelkada requested a review from lvwerra March 18, 2024 09:52

lvwerra reviewed Mar 18, 2024

View reviewed changes

final comment

7754760

younesbelkada requested a review from lvwerra March 18, 2024 11:07

lvwerra approved these changes Mar 18, 2024

View reviewed changes

younesbelkada merged commit a2aa0f0 into main Mar 18, 2024
9 checks passed

younesbelkada deleted the add-cli branch March 18, 2024 11:20

qgallouedec mentioned this pull request Nov 22, 2024

🕹️ CLI refactor #2380

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEAT: Add CLIs in TRL ! #1419

FEAT: Add CLIs in TRL ! #1419

younesbelkada commented Mar 12, 2024 •

edited

Loading

lvwerra Mar 12, 2024

lvwerra Mar 12, 2024

lvwerra Mar 12, 2024

younesbelkada Mar 12, 2024

HuggingFaceDocBuilderDev commented Mar 12, 2024

lvwerra left a comment

lvwerra Mar 15, 2024

lvwerra Mar 15, 2024

lvwerra Mar 15, 2024

younesbelkada Mar 15, 2024

lvwerra Mar 18, 2024

lvwerra Mar 18, 2024

lvwerra Mar 18, 2024

younesbelkada Mar 18, 2024

		@@ -0,0 +1,20 @@
		# This is an example configuration file of TRL CLI, you can use it for

		[options.packages.find]
		include = examples/scripts/*.py

FEAT: Add CLIs in TRL ! #1419

FEAT: Add CLIs in TRL ! #1419

Conversation

younesbelkada commented Mar 12, 2024 • edited Loading

What does the PR do?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuggingFaceDocBuilderDev commented Mar 12, 2024

lvwerra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

younesbelkada commented Mar 12, 2024 •

edited

Loading