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

Change assertion to warning when passing past_key_value to T5 encoder #16153

Merged
merged 2 commits into from
Mar 18, 2022
Merged

Change assertion to warning when passing past_key_value to T5 encoder #16153

merged 2 commits into from
Mar 18, 2022

Conversation

ZhaofengWu
Copy link
Contributor

What does this PR do?

Fixes # (issue)

Before submitting

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Mar 14, 2022

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

@ZhaofengWu
Copy link
Contributor Author

@patrickvonplaten

Comment on lines +650 to +651
if not self.is_decoder:
logger.warning("`past_key_values` is passed to the encoder. Please make sure this is intended.")
Copy link
Contributor

Choose a reason for hiding this comment

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

We should raise an ValueError here because past_key_values is not a valid input for the encoder and should not be passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see the discussion in #15591

@patrickvonplaten
Copy link
Contributor

I'm fine with this change

@ZhaofengWu
Copy link
Contributor Author

Thanks. I don't think I can merge -- please merge whenever makes sense.

@patrickvonplaten
Copy link
Contributor

@patil-suraj feel free to merge if ok for you

@patil-suraj patil-suraj merged commit cb2b027 into huggingface:master Mar 18, 2022
FrancescoSaverioZuppichini pushed a commit that referenced this pull request Mar 24, 2022
…#16153)

* Change assertion to warning when passing past_key_value to T5 encoder

* lint
@libratiger
Copy link

hi guys, I have one question, the T5Encoder is only used when the encoder_outputs is None, so what's the meaning for the T5Encoder.

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.

5 participants