Don't use LayoutLMv2 and LayoutLMv3 in some pipeline tests
#22774
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The documentation is not available anymore as the PR was closed or merged. |
ydshieh
changed the title
LayoutLMv2 and LayoutLMv3 in some pipeline tests
ydshieh
commented
Apr 14, 2023
| # `DocumentQuestionAnsweringPipeline` is expected to work with this model, but it combines the text and visual | ||
| # embedding along the sequence dimension (dim 1), which causes an error during post-processing as `p_mask` has | ||
| # the sequence dimension of the text embedding only. | ||
| # (see the line `embedding_output = torch.cat([embedding_output, visual_embeddings], dim=1)`) |
There was a problem hiding this comment.
cc @NielsRogge We might need to discuss this at some point.
ydshieh
commented
Apr 14, 2023
|
|
||
| return super().is_pipeline_test_to_skip( | ||
| pipeline_test_casse_name, config_class, model_architecture, tokenizer_name, processor_name | ||
| ) |
There was a problem hiding this comment.
remove all these - we just don't add these to be tested in the first place
ydshieh
commented
Apr 14, 2023
| if model_mapping is not None: | ||
| model_mapping = {config: model for config, model in model_mapping.items() if config.__name__ in _TO_SKIP} | ||
| if tf_model_mapping is not None: | ||
| tf_model_mapping = {config: model for config, model in tf_model_mapping.items() if config.__name__ in _TO_SKIP} |
There was a problem hiding this comment.
This is to avoid the changes of pipeline_model_mapping in this PR being reverted next time we update the repository with the script in add_pipeline_model_mapping_to_test.py
There was a problem hiding this comment.
This also makes things more explicit: those models are not for this pipeline (test)
sgugger
approved these changes
Apr 14, 2023
|
Merge now as this PR only touches tests. Feel free to leave a comment if any @NielsRogge |
novice03
pushed a commit
to novice03/transformers
that referenced
this pull request
Jun 23, 2023
…ngface#22774) * 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
These 2 models require different input format than those of usual text models. See the relevant code block at the end.
The offline discussion with @NielsRogge is that these 2 models are only for DocQA pipeline, despite they have implementations for different head tasks.
Therefore, this PR removes these 2 models from being tested (pipeline) in the first place, instead of skipping them at later point.
IMO, we should also remove these models being used in the pipeline classes (except DocQA) if they are not going to work. But I don't do anything on this.
LayoutLMv3withDocumentQuestionAnsweringPipeline(and the pipeline test) is still not working due to some issue. We need to discuss with @NielsRogge to see if it could be fixed, but it's out of this PR's scope.relevant code block
transformers/src/transformers/models/layoutlmv3/tokenization_layoutlmv3.py
Lines 610 to 625 in daf5324