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

[KTO] fix tokenization bugs #1418

Merged
merged 51 commits into from
Mar 14, 2024
Merged

[KTO] fix tokenization bugs #1418

merged 51 commits into from
Mar 14, 2024

Conversation

kawine
Copy link
Contributor

@kawine kawine commented Mar 12, 2024

cc @kashif

kawine and others added 30 commits February 24, 2024 18:19
add reference to paper

Co-authored-by: lewtun <[email protected]>
@kashif
Copy link
Collaborator

kashif commented Mar 12, 2024

thanks @kawine checking now

@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.

Copy link
Member

@lewtun lewtun left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the fix @kawine ! Would you mind adding a small unit test for the tokenize_row() method to ensure we don't regress on this in future 🙏 ?

@kashif
Copy link
Collaborator

kashif commented Mar 12, 2024

@kawine you can use this test:

    def test_kto_trainer_tokenize_row(self):
        with tempfile.TemporaryDirectory() as tmp_dir:
            training_args = KTOConfig(
                output_dir=tmp_dir,
                per_device_train_batch_size=2,
                max_steps=3,
                remove_unused_columns=False,
                gradient_accumulation_steps=1,
                learning_rate=9e-1,
                evaluation_strategy="steps",
                beta=0.1,
            )

            dummy_dataset = self._init_dummy_dataset()

            trainer = KTOTrainer(
                model=self.model,
                ref_model=self.ref_model,
                args=training_args,
                tokenizer=self.tokenizer,
                train_dataset=dummy_dataset,
                eval_dataset=dummy_dataset,
            )

            row = dummy_dataset[0]

            # test that the row can be tokenized
            tokenized_row = trainer.tokenize_row(row)

            # Assert bos_token_id
            assert tokenized_row["prompt_input_ids"][0] == self.tokenizer.bos_token_id
            assert tokenized_row["completion_input_ids"][0] == self.tokenizer.bos_token_id

@kawine
Copy link
Contributor Author

kawine commented Mar 12, 2024

thanks! test has been added @kashif

@kashif kashif merged commit fb6ebb1 into huggingface:main Mar 14, 2024
2 of 9 checks passed
kashif added a commit that referenced this pull request Mar 14, 2024
* add warning for imbalanced data

* update documentation

* update script commands to be same as in dpo

* use batch_size KL examples and batch_size target examples to calculate batch_size losses

* fix deepspeed issue

* speed up forward with no_grad for KL

* add some removed metrics

* Update trl/trainer/kto_trainer.py

* Update trl/trainer/kto_trainer.py

* Update trl/trainer/kto_trainer.py

add reference to paper

Co-authored-by: lewtun <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* add more detailed comments

* convert assert to ValueError

* Update kto_trainer.py

* precommit formatting

* remove nans in metrics by gathering across machines

* fix formatting

* fix choice of mismatched examples for KL term

* describe weights

* fix hanging issue in distributed training

* linting

* move metrics to cpu

* Update trl/trainer/kto_trainer.py

Co-authored-by: lewtun <[email protected]>

* Update trl/trainer/kto_trainer.py

* Update trl/trainer/kto_trainer.py

* fix tokenization error: lack of bos

* change user warning for weight hyperparams

* minor update to docs

* reshape attention mask

* reformat

* add test for bos/eos tokens

* move dependency location

* Update tests/test_kto_trainer.py

---------

Co-authored-by: Kashif Rasul <[email protected]>
Co-authored-by: lewtun <[email protected]>
kashif added a commit to fe1ixxu/trl that referenced this pull request Mar 15, 2024
* add warning for imbalanced data

* update documentation

* update script commands to be same as in dpo

* use batch_size KL examples and batch_size target examples to calculate batch_size losses

* fix deepspeed issue

* speed up forward with no_grad for KL

* add some removed metrics

* Update trl/trainer/kto_trainer.py

* Update trl/trainer/kto_trainer.py

* Update trl/trainer/kto_trainer.py

add reference to paper

Co-authored-by: lewtun <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* add more detailed comments

* convert assert to ValueError

* Update kto_trainer.py

* precommit formatting

* remove nans in metrics by gathering across machines

* fix formatting

* fix choice of mismatched examples for KL term

* describe weights

* fix hanging issue in distributed training

* linting

* move metrics to cpu

* Update trl/trainer/kto_trainer.py

Co-authored-by: lewtun <[email protected]>

* Update trl/trainer/kto_trainer.py

* Update trl/trainer/kto_trainer.py

* fix tokenization error: lack of bos

* change user warning for weight hyperparams

* minor update to docs

* reshape attention mask

* reformat

* add test for bos/eos tokens

* move dependency location

* Update tests/test_kto_trainer.py

---------

Co-authored-by: Kashif Rasul <[email protected]>
Co-authored-by: lewtun <[email protected]>
lapp0 pushed a commit to lapp0/trl that referenced this pull request May 10, 2024
* add warning for imbalanced data

* update documentation

* update script commands to be same as in dpo

* use batch_size KL examples and batch_size target examples to calculate batch_size losses

* fix deepspeed issue

* speed up forward with no_grad for KL

* add some removed metrics

* Update trl/trainer/kto_trainer.py

* Update trl/trainer/kto_trainer.py

* Update trl/trainer/kto_trainer.py

add reference to paper

Co-authored-by: lewtun <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* Update trl/trainer/kto_trainer.py

Co-authored-by: Kashif Rasul <[email protected]>

* add more detailed comments

* convert assert to ValueError

* Update kto_trainer.py

* precommit formatting

* remove nans in metrics by gathering across machines

* fix formatting

* fix choice of mismatched examples for KL term

* describe weights

* fix hanging issue in distributed training

* linting

* move metrics to cpu

* Update trl/trainer/kto_trainer.py

Co-authored-by: lewtun <[email protected]>

* Update trl/trainer/kto_trainer.py

* Update trl/trainer/kto_trainer.py

* fix tokenization error: lack of bos

* change user warning for weight hyperparams

* minor update to docs

* reshape attention mask

* reformat

* add test for bos/eos tokens

* move dependency location

* Update tests/test_kto_trainer.py

---------

Co-authored-by: Kashif Rasul <[email protected]>
Co-authored-by: lewtun <[email protected]>
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