Add BROS #23190

jinhopark8345 · 2023-05-07T10:27:50Z

What does this PR do?

Add BROS(BERT Relying On Spatiality) to 🤗 Transformers

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.

@NielsRogge

amyeroberts · 2023-05-10T14:07:17Z

@jinhopark8345 Awesome work - looking forward to having this model added! Feel free to ping us when the PR is ready for review or you have any implementation questions in the meantime.

jinhopark8345 · 2023-05-14T16:18:48Z

@amyeroberts

I am confused about what needs to be done.

According to the How to add a new model guideline, a big part of it is porting pretrained models (from the original repo) into Huggingface transformers and making sure they are correctly ported by checking the outputs of each layer's forward step.

However, it seems like the authors of the Bros model used transformers-cli to create the boilerplate code, and I don't think there is much to change from the original code.

Do I need to write a conversion script? Or can I skip this step and move to the step where I add model test codes?

Thanks for the help in advance!

amyeroberts · 2023-05-15T10:44:59Z

@jinhopark8345 Interesting - that will definitely make things easier! In this case, if the files are already on the hub and in the correct format, there's no need for the conversion script. It's possible there might be additional arguments required in the config files or additional files needed in the hub repo, in which case, I'd suggest writing a script to add these. You probably won't be able to write directly to the org's repo, but can open a PR with any necessary changes.

Bros' positional embedding)

jinhopark8345 · 2023-09-14T01:48:30Z

@amyeroberts I added convert_bros_to_pytorch.py script because bbox_projection (linear layer) moved from BrosTextEmbeddings to newly added BrosBboxEmbedding.

amyeroberts · 2023-09-14T11:41:53Z

README.md

+1. **[Bros](https://huggingface.co/docs/transformers/model_doc/bros)** (from NAVER CLOVA) released with the paper [BROS: A Pre-trained Language Model Focusing on Text and Layout for Better Key Information Extraction from Documents](https://arxiv.org/abs/2108.04539) by Teakgyu Hong, Donghyun Kim, Mingi Ji, Wonseok Hwang, Daehyun Nam, Sungrae Park.
+1. **[BROS](https://huggingface.co/docs/transformers/main/model_doc/bros)** (from <FILL INSTITUTION>) released with the paper [<FILL PAPER TITLE>](<FILL ARKIV LINK>) by <FILL AUTHORS>.


This needs to be fixed for the documentation to build. Once resolved we can merge 🤗

Applied the fix!

amyeroberts · 2023-09-14T14:31:17Z

@jinhopark8345 Great - thank you! Could you rebase on main to resolve the conflicts? We should be good to go after that :)

amyeroberts · 2023-09-14T17:03:29Z

@jinhopark8345 Thanks for contributing this model! Make sure to share about it's addition to the library on twitter/linkedin/your medium of choice 🤗

ydshieh · 2023-09-19T07:50:35Z

@jinhopark8345 Thank you for adding this model into transformers.

Regarding the test BrosModelIntegrationTest.test_inference_no_head, it fails on our T4 GPU VM as the expected and actual output differ too much, as you can see.

Could you double check on your machines, please? And what's your machine (GPU) type?

Thank you in advance.

(Pdb) outputs.last_hidden_state[0, :3, :3]
tensor([[-0.3165,  0.0830, -0.1203],
        [-0.0089,  0.0031,  0.0736],
        [-0.0461,  0.0146,  0.0880]], device='cuda:0')
(Pdb) expected_slice
tensor([[-0.4027,  0.0756, -0.0647],
        [-0.0192, -0.0065,  0.1042],
        [-0.0671,  0.0214,  0.0960]], device='cuda:0')

You can run the test with

TF_FORCE_GPU_ALLOW_GROWTH=true RUN_SLOW=1 python3 -m pytest -v tests/models/bros/test_modeling_bros.py::BrosModelIntegrationTest::test_inference_no_head

jinhopark8345 · 2023-09-19T11:13:36Z

@jinhopark8345 Thank you for adding this model into transformers.

Regarding the test BrosModelIntegrationTest.test_inference_no_head, it fails on our T4 GPU VM as the expected and actual output differ too much, as you can see.

Could you double check on your machines, please? And what's your machine (GPU) type?

Thank you in advance.
(Pdb) outputs.last_hidden_state[0, :3, :3]
tensor([[-0.3165,  0.0830, -0.1203],
        [-0.0089,  0.0031,  0.0736],
        [-0.0461,  0.0146,  0.0880]], device='cuda:0')
(Pdb) expected_slice
tensor([[-0.4027,  0.0756, -0.0647],
        [-0.0192, -0.0065,  0.1042],
        [-0.0671,  0.0214,  0.0960]], device='cuda:0')
You can run the test with
TF_FORCE_GPU_ALLOW_GROWTH=true RUN_SLOW=1 python3 -m pytest -v tests/models/bros/test_modeling_bros.py::BrosModelIntegrationTest::test_inference_no_head

@ydshieh Thank you for providing the test command!

I was able to reproduce the issue, but the outputs.last_hidden_state[0, :3, :3] value I obtained was different.
Interestingly, not only did the output was different from the expected_slice but the value of outputs.last_hidden_state[0, :3, :3] changed every time I ran the command you provided. For testing, I am using an RTX3090.

After some testing, I found that some weights weren't being initialized properly.
This issue came from that bbox_projection layer (a linear layer) was moved from BrosEmbeddings to under BrosBboxEmbeddings (BrosBboxEmbeddings is newly added class)

By changing:

model = BrosModel.from_pretrained("naver-clova-ocr/bros-base-uncased").to(torch_device)

to:

model = BrosModel.from_pretrained("jinho8345/bros-base-uncased").to(torch_device)

I was able to get consistent outputs. (conversion script : transformers/models/bros/convert_bros_to_pytorch.py)

I suspect this issue wasn't detected earlier because when running:

python3 -m pytest -v tests/models/bros/test_modeling_bros.py

torch cuda seed is manually set to certain value, perhaps due to other tests or other reasons.

The update is here but I am not sure how I should apply this patch to Transformers library.

ydshieh · 2023-09-19T12:53:36Z

Hi @jinhopark8345 Thanks a lot for looking into this!

You can open a PR to update the checkpoint repo used in the test, or we can do it on our own side.

But is it expected that naver-clova-ocr/bros-base-uncased doesn't have all the weights? What are the difference between these 2 checkpoints?

jinhopark8345 · 2023-09-19T14:37:35Z

Hi @jinhopark8345 Thanks a lot for looking into this!

You can open a PR to update the checkpoint repo used in the test, or we can do it on our own side.

But is it expected that naver-clova-ocr/bros-base-uncased doesn't have all the weights? What are the difference between these 2 checkpoints?

The naver-clova-ocr/bros-base-uncased has all the weights. But some weights have been renamed. So if we load BrosModel with naver-clova-ocr/bros-base-uncased checkpoint (original checkpoint), the renamed weights won't be initialized correctly with pretrained weights.

these are the renamed weights!

def rename_key(name):
   if name == "embeddings.bbox_projection.weight":
       name = "bbox_embeddings.bbox_projection.weight"

   if name == "embeddings.bbox_sinusoid_emb.x_pos_emb.inv_freq":
       name = "bbox_embeddings.bbox_sinusoid_emb.x_pos_emb.inv_freq"

   if name == "embeddings.bbox_sinusoid_emb.y_pos_emb.inv_freq":
       name = "bbox_embeddings.bbox_sinusoid_emb.y_pos_emb.inv_freq"

   return name

If you confirm updating the checkpoint is okay, I would like to open PR!

ydshieh · 2023-09-19T15:22:43Z

Sure, go for it. BTW, I see a lot of naver-clova-ocr/bros-base-uncased used, in particular in the examples. So just to be sure, is the user expected to use naver-clova-ocr/bros-base-uncased or the renamed one jinho8345/bros-base-uncased?

From your description, I think it is jinho8345/bros-base-uncased. If this is the case, could you update all occurrence (not just in the tests). Thank you!

ydshieh · 2023-09-20T12:29:18Z

Hello @jinhopark8345 Thank you again for fixing the checkpoint. I have yet another question needs your help.

For BrosModel, the bbox_position_embeddings could be None before calling self.encoder (if bbox is None)

transformers/src/transformers/models/bros/modeling_bros.py

Lines 927 to 933 in 37c205e

    
           bbox_position_embeddings = None 
        
           if bbox is not None: 
        
               # if bbox has 2 points (4 float tensors) per token, convert it to 4 points (8 float tensors) per token 
        
               if bbox.shape[-1] == 4: 
        
                   bbox = bbox[:, :, [0, 1, 2, 1, 2, 3, 0, 3]] 
        
               scaled_bbox = bbox * self.config.bbox_scale 
        
               bbox_position_embeddings = self.bbox_embeddings(scaled_bbox)

but eventually, BrosSelfAttention will fail if it receives None for bbox_pos_emb

transformers/src/transformers/models/bros/modeling_bros.py

Line 391 in 37c205e

bbox_pos_emb = bbox_pos_emb.view(seq_length, seq_length, batch_size, d_head)

Could you double check if BrosModel will only work if bbox is not None in the original implementation? If this is not the case, how is bbox_pos_emb being created if bbox is None etc.

Thank you in advance, again!

jinhopark8345 · 2023-09-20T13:27:11Z

Hello @ydshieh Thank you for asking!

Below code is the original implementation

        scaled_bbox = bbox * self.config.bbox_scale
        bbox_pos_emb = self.embeddings.calc_bbox_pos_emb(
            scaled_bbox, self.config.pe_type
        )

In original implementation, BrosModel will only work if bbox is not None.

Would it be more helpful to users if we remove

transformers/src/transformers/models/bros/modeling_bros.py

Line 928 in 2d71307

if bbox is not None:

so that BrosModel fails earlier? or do you suggest different solutions?

ydshieh · 2023-09-20T14:01:12Z

Hi! In this case, you can add a try: except at the beginning of BrosModel.forward method as the input validation.

(we might need a few more fixes if CI fails due to this)

Thank you !

NielsRogge · 2023-09-20T17:07:45Z

Hi @jinhopark8345 , congrats on this amazing contribution.

Feel free to share about it on Twitter/LinkedIn and we'll amplify.

* add Bros boilerplate * copy and pasted modeling_bros.py from official Bros repo * update copyright of bros files * copy tokenization_bros.py from official repo and update import path * copy tokenization_bros_fast.py from official repo and update import path * copy configuration_bros.py from official repo and update import path * remove trailing period in copyright line * copy and paste bros/__init__.py from official repo * save formatting * remove unused unnecessary pe_type argument - using only crel type * resolve import issue * remove unused model classes * remove unnecessary tests * remove unused classes * fix original code's bug - layer_module's argument order * clean up modeling auto * add bbox to prepare_config_and_inputs * set temporary value to hidden_size (32 is too low because of the of the Bros' positional embedding) * remove decoder test, update create_and_check* input arguemnts * add missing variable to model tests * do make fixup * update bros.mdx * add boilerate plate for no_head inference test * update BROS_PRETRAINED_MODEL_ARCHIVE_LIST (add naver-clova-ocr prefix) * add prepare_bros_batch_inputs function * update modeling_common to add bbox inputs in Bros Model Test * remove unnecessary model inference * add test case * add model_doc * add test case for token_classification * apply fixup * update modeling code * update BrosForTokenClassification loss calculation logic * revert logits preprocessing logic to make sure logits have original shape * - update class name * - add BrosSpadeOutput - update BrosConfig arguments * add boilerate plate for no_head inference test * add prepare_bros_batch_inputs function * add test case * add test case for token_classification * update modeling code * update BrosForTokenClassification loss calculation logic * revert logits preprocessing logic to make sure logits have original shape * apply masking on the fly * add BrosSpadeForTokenLinking * update class name put docstring to the beginning of the file * separate the logits calculation logic and loss calculation logic * update logic for loss calculation so that logits shape doesn't change when return * update typo * update prepare_config_and_inputs * update dummy node initialization * update last_hidden_states getting logic to consider when return_dict is False * update box first token mask param * bugfix: remove random attention mask generation * update keys to ignore on load missing * run make style and quality * apply make style and quality of other codes * update box_first_token_mask to bool type * update index.md * apply make style and quality * apply make fix-copies * pass check_repo * update bros model doc * docstring bugfix fix * add checkpoint for doc, tokenizer for doc * Update README.md * Update docs/source/en/model_doc/bros.md Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update bros.md * Update src/transformers/__init__.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Update docs/source/en/model_doc/bros.md Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> * apply suggestions from code review * apply suggestions from code review * revert test_processor_markuplm.py * Update test_processor_markuplm.py * apply suggestions from code review * apply suggestions from code review * apply suggestions from code review * update BrosSpadeELForTokenClassification head name to entity linker * add doc string for config params * update class, var names to more explicit and apply suggestions from code review * remove unnecessary keys to ignore * update relation extractor to be initialized with config * add bros processor * apply make style and quality * update bros.md * remove bros tokenizer, add bros processor that wraps bert tokenizer * revert change * apply make fix-copies * update processor code, update itc -> initial token, stc -> subsequent token * add type hint * remove unnecessary condition branches in embedding forward * fix auto tokenizer fail * update docstring for each classes * update bbox input dimension as standard 2 points and convert them to 4 points in forward pass * update bros docs * apply suggestions from code review : update Bros -> BROS in bros.md * 1. box prefix var -> bbox 2. update variable names to be more explicit * replace einsum with torch matmul * apply style and quality * remove unused argument * remove unused arguments * update docstrings * apply suggestions from code review: add BrosBboxEmbeddings, replace einsum with classical matrix operations * revert einsum update * update bros processor * apply suggestions from code review * add conversion script for bros * Apply suggestions from code review * fix readme * apply fix-copies --------- Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>

Prathyusha-Akundi · 2023-11-06T15:39:26Z

Hi @jinhopark8345,
Can you please provide examples of how to use logits from BrosSpadeELForTokenClassification to identify the intra-relationships?
TIA

jinhopark8345 · 2023-11-07T02:32:20Z

Hi @Prathyusha-Akundi,

You can refer to the example notebook for identifying intra-relationships.

If you are looking for information on entity linking versus entity extraction, you can check out the entity linking explanation vs entity extraction here.

Prathyusha-Akundi · 2023-11-09T06:19:42Z

Thank you @jinhopark8345 , this is extremely helpful!

jinhopark8345 force-pushed the add-bros branch 2 times, most recently from 042ab4a to ad32c01 Compare May 10, 2023 11:32

jinhopark8345 added 25 commits May 22, 2023 22:50

add Bros boilerplate

3db764f

copy and pasted modeling_bros.py from official Bros repo

2dc368a

update copyright of bros files

5603061

copy tokenization_bros.py from official repo and update import path

dbc56b8

copy tokenization_bros_fast.py from official repo and update import path

e2a2d9d

copy configuration_bros.py from official repo and update import path

90ce711

remove trailing period in copyright line

6126da1

copy and paste bros/__init__.py from official repo

63139eb

save formatting

596d1a7

remove unused unnecessary pe_type argument - using only crel type

764e8df

resolve import issue

f35348f

remove unused model classes

892dd2d

remove unnecessary tests

37c7d9f

remove unused classes

d878de0

fix original code's bug - layer_module's argument order

772d20e

clean up modeling auto

6ef6ca7

add bbox to prepare_config_and_inputs

c338261

set temporary value to hidden_size (32 is too low because of the of the

7379457

Bros' positional embedding)

remove decoder test, update create_and_check* input arguemnts

602e2d9

add missing variable to model tests

79b886c

do make fixup

5f35f68

update bros.mdx

3eace5d

add boilerate plate for no_head inference test

9f0e8ca

update BROS_PRETRAINED_MODEL_ARCHIVE_LIST (add naver-clova-ocr prefix)

66ff6ce

add prepare_bros_batch_inputs function

f3e9dab

jinhopark8345 added 3 commits September 14, 2023 09:55

apply suggestions from code review

44a0fc9

add conversion script for bros

19993a7

Apply suggestions from code review

8fe9f5a

Merge remote-tracking branch 'upstream/main' into add-bros

e1d0c73

amyeroberts changed the title ~~[WIP] Add BROS~~ Add BROS Sep 14, 2023

amyeroberts reviewed Sep 14, 2023

View reviewed changes

jinhopark8345 added 2 commits September 14, 2023 20:48

fix readme

9e883fb

apply fix-copies

8223fed

Merge remote-tracking branch 'upstream/main' into add-bros

187c411

amyeroberts merged commit 17fdd35 into huggingface:main Sep 14, 2023

jinhopark8345 mentioned this pull request Sep 20, 2023

Update bros checkpoint #26277

Merged

5 tasks

jinhopark8345 mentioned this pull request Sep 20, 2023

add bbox input validation #26294

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BROS #23190

Add BROS #23190

jinhopark8345 commented May 7, 2023 •

edited

Loading

amyeroberts commented May 10, 2023

jinhopark8345 commented May 14, 2023 •

edited

Loading

amyeroberts commented May 15, 2023

jinhopark8345 commented Sep 14, 2023

amyeroberts Sep 14, 2023

jinhopark8345 Sep 14, 2023

amyeroberts commented Sep 14, 2023

amyeroberts commented Sep 14, 2023

ydshieh commented Sep 19, 2023 •

edited

Loading

jinhopark8345 commented Sep 19, 2023 •

edited

Loading

ydshieh commented Sep 19, 2023

jinhopark8345 commented Sep 19, 2023 •

edited

Loading

ydshieh commented Sep 19, 2023

ydshieh commented Sep 20, 2023 •

edited

Loading

jinhopark8345 commented Sep 20, 2023

ydshieh commented Sep 20, 2023

NielsRogge commented Sep 20, 2023

Prathyusha-Akundi commented Nov 6, 2023

jinhopark8345 commented Nov 7, 2023

Prathyusha-Akundi commented Nov 9, 2023

		1. [Bros](https://huggingface.co/docs/transformers/model_doc/bros) (from NAVER CLOVA) released with the paper [BROS: A Pre-trained Language Model Focusing on Text and Layout for Better Key Information Extraction from Documents](https://arxiv.org/abs/2108.04539) by Teakgyu Hong, Donghyun Kim, Mingi Ji, Wonseok Hwang, Daehyun Nam, Sungrae Park.
		1. [BROS](https://huggingface.co/docs/transformers/main/model_doc/bros) (from <FILL INSTITUTION>) released with the paper [<FILL PAPER TITLE>](<FILL ARKIV LINK>) by <FILL AUTHORS>.

Add BROS #23190

Add BROS #23190

Conversation

jinhopark8345 commented May 7, 2023 • edited Loading

What does this PR do?

Before submitting

Who can review?

amyeroberts commented May 10, 2023

jinhopark8345 commented May 14, 2023 • edited Loading

amyeroberts commented May 15, 2023

jinhopark8345 commented Sep 14, 2023

amyeroberts Sep 14, 2023

Choose a reason for hiding this comment

jinhopark8345 Sep 14, 2023

Choose a reason for hiding this comment

amyeroberts commented Sep 14, 2023

amyeroberts commented Sep 14, 2023

ydshieh commented Sep 19, 2023 • edited Loading

jinhopark8345 commented Sep 19, 2023 • edited Loading

ydshieh commented Sep 19, 2023

jinhopark8345 commented Sep 19, 2023 • edited Loading

ydshieh commented Sep 19, 2023

ydshieh commented Sep 20, 2023 • edited Loading

jinhopark8345 commented Sep 20, 2023

ydshieh commented Sep 20, 2023

NielsRogge commented Sep 20, 2023

Prathyusha-Akundi commented Nov 6, 2023

jinhopark8345 commented Nov 7, 2023

Prathyusha-Akundi commented Nov 9, 2023

jinhopark8345 commented May 7, 2023 •

edited

Loading

jinhopark8345 commented May 14, 2023 •

edited

Loading

ydshieh commented Sep 19, 2023 •

edited

Loading

jinhopark8345 commented Sep 19, 2023 •

edited

Loading

jinhopark8345 commented Sep 19, 2023 •

edited

Loading

ydshieh commented Sep 20, 2023 •

edited

Loading