chore(deps): upgrade trl and transformers

[willmj]

Description of the change

requirements.txt in trl v0.13.0 no longer restricts transformers version.

Related issue number

More context

How to verify the PR

Was the PR tested

• I have added >=1 unit test(s) for every new method I have added.
• I have ensured all unit tests pass

[github-actions]

Thanks for making a pull request! 😃
One of the maintainers will review and advise on the next steps.

[dushyantbehl]

@willmj do we want to probably lock the version to higher than one which supports packing for pretokenized datasets which we enabled along with this patch.

[willmj]

@dushyantbehl I thought the latest version was 0.13.0, so this would include that. Feel free to make a suggested change if I'm misunderstanding.

[ashokponkumar]

Suggested change

	"trl>=0.9.3,<1.0",
	"trl>=0.13.0,<1.0",

@dushyantbehl is this your suggestion?

[Abhishek-TAMU]

I think if we want TRL version to be greater than >=0.12 then I guess we need to first ensure fms-hf-tuning is compatible with transformers version >=4.46 which I guess currently is not, as per this issue, because I can see TRL latest version has minimum version of transformers as 4.46 requirements.txt

[willmj]

Upgrade of transformers >=4.46 is dependent on fms-acceleration #98, also have to update SFTConfig correctly

[dushyantbehl]

I think if we want TRL version to be greater than >=0.12 then I guess we need to first ensure fms-hf-tuning is compatible with transformers version >=4.46 which I guess currently is not, as per this issue, because I can see TRL latest version has minimum version of transformers as 4.46 requirements.txt

Correct @Abhishek-TAMU that's what I wanted to highlight too thanks

[dushyantbehl]

Upgrade of transformers >=4.46 is dependent on fms-acceleration #98, also have to update SFTConfig correctly

I see....good to note both here and we can update this later post we get working our repo with the new transformers..thanks

[dushyantbehl]

@anhuong thanks for quick fix on foundation-model-stack/fms-acceleration#123

Does this mean we can go ahead and move forward with this change? cc @willmj @fabianlim

[Abhishek-TAMU]

I think if we want TRL version to be greater than >=0.12 then I guess we need to first ensure fms-hf-tuning is compatible with transformers version >=4.46 which I guess currently is not, as per this issue, because I can see TRL latest version has minimum version of transformers as 4.46 requirements.txt

Does this mean we can go ahead and move forward with this change?

As both transformers and trl version are being updated, I guess it would also be good to run 1 automated e2e test (for all models) to once confirm the successful working with the upgraded version of trl and transformers. @willmj

[Abhishek-TAMU]

@willmj

All the failed tests (41 failed tests) would be completely resolved by these 3 changes:

1- Setting gradient_accumulation_steps = 1 here in test_sft_trainer.py.

As the size of training dataset is 10 and batch_size is 4, hence due to gradient_accumulation_steps = 4 not all optimizer steps is run and hence its not running for entire 5 epochs and hence not even completing 1 epoch, hence no training_log json file is not generated (In unit tests with not found errors).

2- Same reasoning, to change gradient_accumulation_steps = 1 in test_launch_script.py

3- Use _get_checkpoint_path to get last checkpoint instead of checkpoint-5 as not every tuning has this checkpoint. Something like this:

def _get_checkpoint_path(dir_path):
    checkpoint_dirs = [
        d for d in os.listdir(dir_path)
        if os.path.isdir(os.path.join(dir_path, d)) and re.match(r'^checkpoint-\d+$', d)
    ]
    checkpoint_dirs.sort(key=lambda name: int(name.split('-')[-1]))
    return os.path.join(dir_path, checkpoint_dirs[-1])

[willmj]

New release of fms-acceleration unblocking transformer 4.46. Should we go for merging?
@anhuong @dushyantbehl @Abhishek-TAMU

[dushyantbehl]

@willmj sure lets go ahead...do you want to merge the change which enables pretokenized datasets with packing as well? or we keep it separate?
I would prefer them to be merged separately but together

[dushyantbehl]

@willmj I applied the fixes to test cases suggested by @Abhishek-TAMU above and have opened a PR on reverting the packing for pretokenized dataset restirctions #468
This is based on your PR...I added few unit tests on packing with pretokenized dataset which seem to fail...testing to fix them but @Abhishek-TAMU 's suggestions above work for this PR

[anhuong]

Running a few tests of tuning with the updated versions, but the changes look good to me. Once my tests run, I will note in this PR

[anhuong]

Full fine-tuning and LoRA tuning ran fine on minimal example. We will have to test to see how these changes affect train_runtime and quality in the next release though @Abhishek-TAMU

[anhuong]

Hmm but the changes from main broke unit tests so will investigate...

[anhuong]

New version of trl v0.15.0 was released 3 hours ago and there are not yet release docs. This upgrade broke the unit tests, setting upper limit to below 0.15.0 for now.

Noting the error here: failed in training loop when running trainer.train()

0%|          | 0/15 [00:00<?, ?it/s]ERROR:sft_trainer.py:Traceback (most recent call last):
  File "/home/runner/work/fms-hf-tuning/fms-hf-tuning/tuning/sft_trainer.py", line 676, in main
    trainer, additional_train_info = train(
                                     ^^^^^^
  File "/home/runner/work/fms-hf-tuning/fms-hf-tuning/tuning/sft_trainer.py", line 420, in train
    trainer.train(resume_from_checkpoint)
  File "/home/runner/work/fms-hf-tuning/fms-hf-tuning/.tox/coverage/lib/python3.12/site-packages/transformers/trainer.py", line 2171, in train
    return inner_training_loop(
           ^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/fms-hf-tuning/fms-hf-tuning/.tox/coverage/lib/python3.12/site-packages/transformers/trainer.py", line 2531, in _inner_training_loop
    tr_loss_step = self.training_step(model, inputs, num_items_in_batch)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/fms-hf-tuning/fms-hf-tuning/.tox/coverage/lib/python3.12/site-packages/transformers/trainer.py", line 3675, in training_step
    loss = self.compute_loss(model, inputs, num_items_in_batch=num_items_in_batch)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/fms-hf-tuning/fms-hf-tuning/.tox/coverage/lib/python3.12/site-packages/trl/trainer/sft_trainer.py", line 453, in compute_loss
    accuracy = compute_token_accuracy(shift_logits, shift_labels)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/fms-hf-tuning/fms-hf-tuning/.tox/coverage/lib/python3.12/site-packages/trl/trainer/utils.py", line 1664, in compute_token_accuracy
    correct_predictions = (predictions == labels) & mask
                           ^^^^^^^^^^^^^^^^^^^^^
RuntimeError: The size of tensor a (72) must match the size of tensor b (64) at non-singleton dimension 1

[Abhishek-TAMU]

Changes to upgrade TRL and Transformer versions LGTM! Thanks for the PR @willmj
Thanks @anhuong for testing it with minimal example.