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

Fix pad_token check condition #25685

Merged
merged 1 commit into from
Aug 23, 2023
Merged

Fix pad_token check condition #25685

merged 1 commit into from
Aug 23, 2023

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Aug 23, 2023

What does this PR do?

Fix #25625

@ydshieh ydshieh changed the title Fix pad_token check condition Fix pad_token check condition Aug 23, 2023
@@ -2503,7 +2503,7 @@ def _get_padding_truncation_strategies(
max_length = self.model_max_length

# Test if we have a padding token
if padding_strategy != PaddingStrategy.DO_NOT_PAD and (not self.pad_token or self.pad_token_id < 0):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This line exist for 3 years but we don't have problem. Guess it's because we never has pad_token=0 (not id)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or rather ('n', 'no', 'f', 'false', 'off', '0') 🤣

@ydshieh ydshieh requested a review from ArthurZucker August 23, 2023 14:16
Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -2503,7 +2503,7 @@ def _get_padding_truncation_strategies(
max_length = self.model_max_length

# Test if we have a padding token
if padding_strategy != PaddingStrategy.DO_NOT_PAD and (not self.pad_token or self.pad_token_id < 0):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Or rather ('n', 'no', 'f', 'false', 'off', '0') 🤣

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 23, 2023

The documentation is not available anymore as the PR was closed or merged.

@ydshieh ydshieh merged commit 2189a7f into main Aug 23, 2023
@ydshieh ydshieh deleted the fix_pad_check branch August 23, 2023 14:39
parambharat pushed a commit to parambharat/transformers that referenced this pull request Sep 26, 2023
fix

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
blbadger pushed a commit to blbadger/transformers that referenced this pull request Nov 8, 2023
fix

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
EduardoPach pushed a commit to EduardoPach/transformers that referenced this pull request Nov 18, 2023
fix

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
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.

self.pad_token detect should be is None rather than "not self.pad_token"
3 participants