foundation-model-stack / fms-hf-tuning

๐Ÿš€ Collection of tuning recipes with HuggingFace SFTTrainer and PyTorch FSDP.
Apache License 2.0
28 stars 48 forks source link

fix: Revert changes from PR#326 with trl version change #360

Closed willmj closed 1 month ago

willmj commented 1 month ago

Description of the change

Setting trl to version 0.9.6 causes the following error:

if not args.packing:
            # If we aren't skipping data preparation, then a dataset_text_field
            # or formatting_func must be provided.
            if (
                args.dataset_text_field is None
                and formatting_func is None
                and dataset_kwargs is not None
                and "skip_prepare_dataset" in dataset_kwargs
                and dataset_kwargs["skip_prepare_dataset"]
            ):
>               raise ValueError(
                    "You passed `packing=False` to the SFTTrainer/SFTConfig, but you didn't pass a `dataset_text_field` or `formatting_func` argument."
                )
E               ValueError: You passed `packing=False` to the SFTTrainer/SFTConfig, but you didn't pass a `dataset_text_field` or `formatting_func` argument.

This PR is to temporarily revert the changes in PR #326 until Fabian's qlora fix is in the latest version of trl

Related issue number

How to verify the PR

Was the PR tested

github-actions[bot] commented 1 month ago

Thanks for making a pull request! ๐Ÿ˜ƒ One of the maintainers will review and advise on the next steps.

fabianlim commented 1 month ago

@willmj what you pasted seems strange

The comments say that the raise is only when data prep is not skipped

# If we aren't skipping data preparation, then a dataset_text_field
 # or formatting_func must be provided.

But the code triggers when dataset_kwargs["skip_prepare_dataset"] is truthful, which is a contradiction because it should mean to skip the data prep

willmj commented 1 month ago

@fabianlim misunderstood your original comment in #358, closing this PR