Skip to content

Comments

TF: properly handle kwargs in encoder_decoder architectures#16465

Merged
gante merged 2 commits intohuggingface:mainfrom
gante:encoder_decoder_kwargs
Mar 29, 2022
Merged

TF: properly handle kwargs in encoder_decoder architectures#16465
gante merged 2 commits intohuggingface:mainfrom
gante:encoder_decoder_kwargs

Conversation

@gante
Copy link
Contributor

@gante gante commented Mar 28, 2022

What does this PR do?

Fixes #16400

Our input_processing function is very strict and raises an exception if unexpected kwargs are passed. In the issue linked above, it was found that an exception was being raised and, upon further inspection, we can see that some arguments were being passed in the wrong place.

There were no tests for it (i.e. no encoder_decoder calls with kwargs), so it slipped under the cracks. This PR also adds tests for it. The updated tests were all failing before the corresponding fix.

@gante gante requested review from Rocketknight1 and sgugger March 28, 2022 22:12
Comment on lines 575 to 577
# Add arguments to encoder from `kwargs_encoder`
for k, v in kwargs_encoder.items():
encoder_processing_inputs[k] = v
Copy link
Contributor Author

@gante gante Mar 28, 2022

Choose a reason for hiding this comment

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

You can see here that all contents in kwargs_encoder are dumped into encoder_processing_inputs, so they were effectively being passed twice.

input_processing expects kwargs_call to be empty, except under very special circumstances (when using deprecated arguments)

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Mar 28, 2022

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

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

Copy link
Member

@Rocketknight1 Rocketknight1 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@gante gante merged commit 7a9ef81 into huggingface:main Mar 29, 2022
@gante gante deleted the encoder_decoder_kwargs branch March 29, 2022 17:17
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.

TFDecoderEncoder: The following keyword arguments are not supported by this model: ['position_ids', 'token_type_ids']

4 participants