Skip to content

Remove padding_value from CPO and use pad_token_id#4962

Open
albertvillanova wants to merge 1 commit intohuggingface:mainfrom
albertvillanova:fu-4846
Open

Remove padding_value from CPO and use pad_token_id#4962
albertvillanova wants to merge 1 commit intohuggingface:mainfrom
albertvillanova:fu-4846

Conversation

@albertvillanova
Copy link
Member

Remove padding_value from CPO and use pad_token_id.

This PR removes the padding_value parameter from the CPOConfig class and updates the trainer logic to always use the tokenizer's pad_token_id for padding. This simplifies configuration and ensures consistent padding behavior throughout the codebase.

Follow-up to:

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

self.max_length = max_length
self.generate_during_eval = args.generate_during_eval
self.padding_value = args.padding_value if args.padding_value is not None else processing_class.pad_token_id
self.pad_token_id = processing_class.pad_token_id
Copy link
Member

Choose a reason for hiding this comment

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

the issue is that, if the tokenizer doesn't have a padding token (which we should allow), it will fail. We should align with stable trainers:

if tokenizer.pad_token is None:
tokenizer.pad_token = tokenizer.eos_token
self.pad_token = tokenizer.pad_token
self.pad_token_id = tokenizer.pad_token_id

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.

3 participants