fix tokenize_row in xPOTrainer

[AIR-hl]

• Remove tokenize_row and build_tokenized_answer from DPOTrainer.py, ORPOTrainer.py and CPOTrainer.py;
• Add tokenize_row and build_tokenized_answer to trainer/utils.py;
• Modify the codes related to the function in the corresponding files;
• Modify the parameter max_completion_length to max_target_length in ORPO in order to be consistent with DPO and CPO

Sorry for my multiple pr, I've never done it before, this is my first time contributing to a project. :(

[vwxyzjn]

Very nice refactoring. Thanks. CC @kashif for a check.

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

[vwxyzjn]

Tests failing. @AIR-hl could you check?

[AIR-hl]

Tests failing. @AIR-hl could you check?

I guess it’s because the default max_length in config is None. Normally, it would be initiated if it is None, but now tokenizer_now is separated, the max_length in args doesn't be initiated, I will try it fix

[kashif]

btw the TRL maintainers might have an issue with arg being renamed due to backward compatibility... there is a mechanism i believe for deprecating things

[AIR-hl]

btw the TRL maintainers might have an issue with arg being renamed due to backward compatibility... there is a mechanism i believe for deprecating things

btw the TRL maintainers might have an issue with arg being renamed due to backward compatibility... there is a mechanism i believe for deprecating things

Do I have any good idea? I am just a student who lacks practical experience

[kashif]

for now perhaps lets not rename the arguments?

[AIR-hl]

for now perhaps lets not rename the arguments?

max_completion_length in ORPO and max_target_length in DPO and CPO are equivalent in terms of function, is this due to irrational naming by the trl developers?

Maybe I can add a judgement in tokenize_row , such as:

if hasattr(args, "max_target_length"):
    max_length = object.max_target_length
elif hasattr(args, "max_completion_length"):
    max_length = object.max_completion_length
else:
    raise ValueError(f"You should set 'max_target_length' or 'max_completion_length' when using encode_decoder")

How do you think that?

[winglian]

I'm not sure this is a great change. This makes it near impossible to extend the functionality of tokenize_row. To modify the tokenize_row, a developer has no hooks to do so cleanly with this change, whereas at least a class method allows for this cleanly.