Add `obj-det` pipeline support for `LayoutLMV2` #13622

mishig25 · 2021-09-17T10:35:11Z

What does this PR do?

Note: I'm using terms document-understanding & layout-detection interchangeably and imo, term layout-detection sounds more accurate.

As we have discussed in huggingface/hub-docs#21, reusing object-detection pipeline for layout-detection architectures (specifically, `LayoutLMv2ForTokenClassification).

An important detail in reusing so is that:

LayoutLMv2ForTokenClassification needs LayoutLMv2Processor to preprocess input image
Since processor is just a combination of (tokenizer + feature_extractor), ObjectDetectionPipeline.preprocess does exactly what LayoutLMv2Processor does when the selected model has architecture ...ForTokenClassification

Fixes # (issue)

Before submitting

This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
Did you read the contributor guideline,
Pull Request section?
Was this discussed/approved via a Github issue or the forum? Please add a link
to it if that's the case.
Did you make sure to update the documentation with your changes? Here are the
documentation guidelines, and
here are tips on formatting docstrings.
Did you write any new necessary tests?

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.

@Narsil @NielsRogge

NielsRogge · 2021-09-17T10:43:26Z

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py

+if is_torch_available():
+    import torch
+    from torch import nn
+


Can you still import LayoutLMv2FeatureExtractor when you don't have torch installed?

With lazy loading, all classes are None if the various requirements are not met I think

Can you check this @mishig25?

Not sure if this answers your question. Added import torch lines because the newly added post_process method depends on torch

transformers/src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py

Lines 239 to 244 in 01509bf

def post_process(self, outputs, target_sizes, offset_mapping, bbox):

"""

Converts the output of :class:`~transformers.LayoutLMv2ForTokenClassification` into the format expected by the

COCO api. Only supports PyTorch.

Args:

Should I move the import torch statement inside post_process method ?

cc'ing @LysandreJik here to check what's the best option

I would keep the import torch statement at the top level. Moving it inside the post_process function means it might crash once a program is well into progress, which can be painful. I'd rather it fail early on.

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py

src/transformers/pipelines/object_detection.py

…v2.py Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

Narsil

Some suggestions to improve, but overall seems to work !

Cheers.

src/transformers/models/auto/modeling_auto.py

Narsil · 2021-09-17T11:41:30Z

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py

+if is_torch_available():
+    import torch
+    from torch import nn
+


With lazy loading, all classes are None if the various requirements are not met I think

tests/test_pipelines_object_detection.py

src/transformers/pipelines/object_detection.py

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py

src/transformers/pipelines/object_detection.py

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py

…v2.py Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

…nto layout_detection

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py

src/transformers/models/auto/modeling_auto.py

Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

Narsil

LGTM.

I left a comment again about the last test which purpose is not entirely clear to me but I think it's OK to move forward as-is too.

tests/test_feature_extraction_layoutlmv2.py

Narsil

LGTM.

I left a comment again about the last test which purpose is not entirely clear to me but I think it's OK to move forward as-is too.

tests/test_pipelines_object_detection.py

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py

NielsRogge

LGTM! Only a minor change in the post_process method and it's OK.

…v2.py Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py

mishig25 · 2021-09-24T08:23:14Z

@Narsil since I've made this change,
ci/circleci: run_tests_pipelines_torch is failing for a reason: requires_backends(self, "detectron2")

However, I've tried adding @require_detectron2 decorator both to class ObjectDetectionPipelineTests & method run_pipeline_test and wasn't able to make the test pass.

What am I missing?

Narsil · 2021-09-24T08:30:11Z

@mishig25 It seems it's unrelated tests. Did you rebase ?

It seems like other generic tests are failing because Layout is declaring himself as ForQuestionAnswering etc... and those tests are ran without detectron2...

Feel free to ignore, I'll take a look

mishig25 · 2021-09-24T08:42:29Z

@Narsil I did rebase. It should be unrelated but the tests started failing after commit df86d51
Thanks a lot for looking at this!

Attempt huggingface#2. Limit scope of error exception to detectron2 `Lxmert` is special. Typo.

mishig25 · 2021-09-27T20:03:07Z

@LysandreJik could you please check this c93385e and defd574 commits where Nicolas is disabling some tests for a reason:

Basically Layout displays itself as a valid model for many pipelines but it isn't (like other vision models it's fine) I just disabled those tests as they're not supposed to work
(ForQuestionAnswering, ForTextClassification and so on)

LysandreJik

This looks good! The tests removed look fine to me.

Only left a comment relative to a test that would not run anymore for DETR, which is an issue

LysandreJik · 2021-09-30T19:49:37Z

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py

+if is_torch_available():
+    import torch
+    from torch import nn
+


I would keep the import torch statement at the top level. Moving it inside the post_process function means it might crash once a program is well into progress, which can be painful. I'd rather it fail early on.

LysandreJik · 2021-09-30T19:51:41Z

tests/test_pipelines_object_detection.py

+    @require_detectron2
    @require_datasets
+    @require_pytesseract
    def run_pipeline_test(self, model, tokenizer, feature_extractor):
-        object_detector = ObjectDetectionPipeline(model=model, feature_extractor=feature_extractor)
+        object_detector = ObjectDetectionPipeline(
+            model=model, tokenizer=tokenizer, feature_extractor=feature_extractor
+        )
        outputs = object_detector("./tests/fixtures/tests_samples/COCO/000000039769.png", threshold=0.0)


Adding detectron2 here means that previous object detection models (DETR) will not be run. The CircleCI run that has the detectron2 dependency only runs the LayoutLM-v2 tests.

@LysandreJik would removing require_detectron2 & require_pytesseract decorators from run_pipeline_test work? (since these decorators are being required at a class level)

transformers/tests/test_pipelines_object_detection.py

Lines 60 to 62 in e610734

@require_datasets

def run_pipeline_test(self, model, tokenizer, feature_extractor):

object_detector = ObjectDetectionPipeline(

transformers/tests/test_pipelines_object_detection.py

Lines 51 to 57 in e610734

@require_detectron2

@require_vision

@require_timm

@require_torch

@require_pytesseract

@is_pipeline_test

class ObjectDetectionPipelineTests(unittest.TestCase, metaclass=PipelineTestCaseMeta):

If not, please let me know what would be the best way to handle this problem

@LysandreJik could you comment on #13622 (comment) 👍

Narsil · 2021-10-04T07:23:00Z

@mishig25 Any blockers left for this ?

LysandreJik · 2021-10-14T13:14:38Z

~~Feel free to merge when ready @mishig25~~ @NielsRogge corrected me that some conversation still needs to happen before this is merged.

github-actions · 2021-12-04T15:02:29Z

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

Add obj-det pipe support for LayoutLMV2

302831e

mishig25 requested review from Narsil and NielsRogge September 17, 2021 10:35

Chore

2a265ef

NielsRogge reviewed Sep 17, 2021

View reviewed changes

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py Outdated Show resolved Hide resolved

NielsRogge reviewed Sep 17, 2021

View reviewed changes

src/transformers/pipelines/object_detection.py Show resolved Hide resolved

Update src/transformers/models/layoutlmv2/feature_extraction_layoutlm…

fd7d47b

…v2.py Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

Narsil reviewed Sep 17, 2021

View reviewed changes

NielsRogge reviewed Sep 17, 2021

View reviewed changes

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py Outdated Show resolved Hide resolved

NielsRogge reviewed Sep 17, 2021

View reviewed changes

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py Outdated Show resolved Hide resolved

NielsRogge reviewed Sep 17, 2021

View reviewed changes

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py Outdated Show resolved Hide resolved

NielsRogge reviewed Sep 17, 2021

View reviewed changes

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py Outdated Show resolved Hide resolved

NielsRogge reviewed Sep 17, 2021

View reviewed changes

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py Outdated Show resolved Hide resolved

mishig25 and others added 11 commits September 20, 2021 09:41

Add box normalization comments

6b44c80

Update src/transformers/models/layoutlmv2/feature_extraction_layoutlm…

0964a63

…v2.py Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

Update src/transformers/models/layoutlmv2/feature_extraction_layoutlm…

18d3805

…v2.py Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

Update src/transformers/models/layoutlmv2/feature_extraction_layoutlm…

ad7d9b2

…v2.py Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

Update src/transformers/models/layoutlmv2/feature_extraction_layoutlm…

a1267ca

…v2.py Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

Update src/transformers/models/layoutlmv2/feature_extraction_layoutlm…

af6886b

…v2.py Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

Chore

61c7df2

Merge branch 'layout_detection' of github.com:mishig25/transformers i…

85e40a4

…nto layout_detection

Add comments and make fixup

59ec807

Merge branch 'master' into layout_detection

6796027

Refacotr is_subwords

64c2df5

NielsRogge reviewed Sep 20, 2021

View reviewed changes

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py Outdated Show resolved Hide resolved

NielsRogge reviewed Sep 20, 2021

View reviewed changes

src/transformers/models/auto/modeling_auto.py Outdated Show resolved Hide resolved

Update src/transformers/models/auto/modeling_auto.py

52bcdbe

Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

Narsil mentioned this pull request Sep 21, 2021

Add Speech AutoModels #13655

Merged

5 tasks

Add post_process test and better offset naming

12d3205

Narsil approved these changes Sep 22, 2021

View reviewed changes

tests/test_feature_extraction_layoutlmv2.py Outdated Show resolved Hide resolved

Narsil reviewed Sep 22, 2021

View reviewed changes

tests/test_pipelines_object_detection.py Outdated Show resolved Hide resolved

NielsRogge reviewed Sep 23, 2021

View reviewed changes

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py Outdated Show resolved Hide resolved

NielsRogge approved these changes Sep 23, 2021

View reviewed changes

mishig25 and others added 2 commits September 23, 2021 15:48

Update src/transformers/models/layoutlmv2/feature_extraction_layoutlm…

6589fb0

…v2.py Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

Better variable names in post_process

01509bf

Narsil reviewed Sep 23, 2021

View reviewed changes

src/transformers/models/layoutlmv2/feature_extraction_layoutlmv2.py Outdated Show resolved Hide resolved

mishig25 added 5 commits September 23, 2021 22:24

Fix wrong comment

3e3d626

Extend run_pipeline_test test

df86d51

Fix tests

2cedc54

Merge branch 'master' into layout_detection

d8bdd19

Add require backends to tests

3825a41

mishig25 and others added 5 commits September 24, 2021 10:52

Update get_config to use smaller resent

5ac49f8

Rm dev change

907c4f4

Disabling tests for bimodal models on pipelines that do not support it.

c93385e

Merge branch 'master' into layout_detection

6004132

Making all tests pass attempt huggingface#1.

defd574

Attempt huggingface#2. Limit scope of error exception to detectron2 `Lxmert` is special. Typo.

mishig25 requested a review from LysandreJik September 27, 2021 19:59

LysandreJik reviewed Sep 30, 2021

View reviewed changes

Rm layout specific decorators from run_pipeline_ts

e610734

huggingface deleted a comment from github-actions bot Nov 9, 2021

github-actions bot closed this Dec 13, 2021

mishig25 mentioned this pull request Sep 23, 2021

Tracking integration for text-nearest huggingface/hub-docs#35

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `obj-det` pipeline support for `LayoutLMV2` #13622

Add `obj-det` pipeline support for `LayoutLMV2` #13622

mishig25 commented Sep 17, 2021

NielsRogge Sep 17, 2021

Narsil Sep 17, 2021

NielsRogge Sep 23, 2021

mishig25 Sep 23, 2021

NielsRogge Sep 24, 2021

LysandreJik Sep 30, 2021

Narsil left a comment

Narsil Sep 17, 2021

Narsil left a comment

Narsil left a comment

NielsRogge left a comment

mishig25 commented Sep 24, 2021

Narsil commented Sep 24, 2021

mishig25 commented Sep 24, 2021 •

edited

Loading

mishig25 commented Sep 27, 2021

LysandreJik left a comment

LysandreJik Sep 30, 2021

LysandreJik Sep 30, 2021

mishig25 Oct 4, 2021

mishig25 Oct 13, 2021

Narsil commented Oct 4, 2021

LysandreJik commented Oct 14, 2021 •

edited

Loading

github-actions bot commented Dec 4, 2021

	def post_process(self, outputs, target_sizes, offset_mapping, bbox):
	"""
	Converts the output of :class:`~transformers.LayoutLMv2ForTokenClassification` into the format expected by the
	COCO api. Only supports PyTorch.

	Args:

	@require_datasets
	def run_pipeline_test(self, model, tokenizer, feature_extractor):
	object_detector = ObjectDetectionPipeline(

	@require_detectron2
	@require_vision
	@require_timm
	@require_torch
	@require_pytesseract
	@is_pipeline_test
	class ObjectDetectionPipelineTests(unittest.TestCase, metaclass=PipelineTestCaseMeta):

Add obj-det pipeline support for LayoutLMV2 #13622

Add obj-det pipeline support for LayoutLMV2 #13622

Conversation

mishig25 commented Sep 17, 2021

What does this PR do?

Before submitting

Who can review?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Narsil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Narsil left a comment

Choose a reason for hiding this comment

Narsil left a comment

Choose a reason for hiding this comment

NielsRogge left a comment

Choose a reason for hiding this comment

mishig25 commented Sep 24, 2021

Narsil commented Sep 24, 2021

mishig25 commented Sep 24, 2021 • edited Loading

mishig25 commented Sep 27, 2021

LysandreJik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Narsil commented Oct 4, 2021

LysandreJik commented Oct 14, 2021 • edited Loading

github-actions bot commented Dec 4, 2021

Add `obj-det` pipeline support for `LayoutLMV2` #13622

Add `obj-det` pipeline support for `LayoutLMV2` #13622

mishig25 commented Sep 24, 2021 •

edited

Loading

LysandreJik commented Oct 14, 2021 •

edited

Loading