Adds IBERT to models exportable with ONNX #14868

MaximovaIrina · 2021-12-21T18:31:35Z

Adds IBERT to models exportable with ONNX

LysandreJik

Nice!! Pinging @michaelbenayoun and @lewtun

lewtun

Thank you for this very clean contribution @MaximovaIrina !

I've left a minor remark, but otherwise this is looking really good. Could you please check that the "slow" tests pass for this model:

RUN_SLOW=1 pytest tests/test_onnx_v2.py -k "ibert" -rp

These tests aren't run until the PR is merged, so it's good to know in advance if there's any problems :)

lewtun · 2021-12-22T09:05:01Z

src/transformers/models/ibert/configuration_ibert.py

@@ -123,3 +127,22 @@ def __init__(
        self.position_embedding_type = position_embedding_type
        self.quant_mode = quant_mode
        self.force_dequant = force_dequant
+
+
+class IBertOnnxConfig(OnnxConfigWithPast):


For encoder-based models like IBERT, I think it makes more sense to use OnnxConfig instead of OnnxConfigWithPast. The OnnxConfigWithPast is usually reserved for decoder-based and encoder-decoder models where having access to the past key values is important for text generation.

Is there a reason why you chose this particular config?

I followed the recommendations in this tutorial. In particular, I was looking at this patch.

Thanks for the information @MaximovaIrina - we're in the process of revamping the docs / tutorial, so hopefully it will be less confusing in future!

The patch you linked to is for MBART, which is an encoder-decoder model where having access to past key values is important. In your case with IBERT, I recommend looking at the DistilBERT example which inherits from OnnxConfig.

So my suggestion would be to replace your use of OnnxConfigWithPast with OnnxConfig.

As @michaelbenayoun notes elsewhere, you can also use DistilBERT as a guide for adding all the supported features.

michaelbenayoun · 2021-12-23T13:28:59Z

src/transformers/models/ibert/configuration_ibert.py

+        )
+
+    @property
+    def outputs(self) -> Mapping[str, Mapping[int, str]]:


You do not need to override the outputs property as it is already taken care of by OnnxConfig for each of the tasks supported by IBERT (the outputs will depend on the task at hand, so the base class outputs property takes care of returning the proper outputs for a given task).

michaelbenayoun · 2021-12-23T13:33:06Z

src/transformers/onnx/features.py

@@ -63,6 +64,7 @@ class FeaturesManager:
        "bart": supported_features_mapping("default", onnx_config_cls=BartOnnxConfig),
        "mbart": supported_features_mapping("default", onnx_config_cls=MBartOnnxConfig),
        "bert": supported_features_mapping("default", onnx_config_cls=BertOnnxConfig),
+        "ibert": supported_features_mapping("default", onnx_config_cls=IBertOnnxConfig),


Can you please add all the supported features for IBERT?
Those are:

masked-lm

sequence-classification

~~multiple-choice~~ (this one is not supported yet)

token-classification

question-answering

lewtun · 2021-12-23T13:52:37Z

Hey @MaximovaIrina we recently switched all of our documentation from RST to MDX, so you can rebase on master to eliminate the conflict with docs/source/serialization.rst

MaximovaIrina · 2021-12-23T14:35:49Z

Checking slow tests for ibert

lewtun · 2021-12-23T14:40:35Z

docs/source/serialization.rst

@@ -0,0 +1,433 @@
+.. 
+    Copyright 2020 The HuggingFace Team. All rights reserved.


This file shouldn't be included (must be a Git problem with the switch from RST to MDX). Could you please delete it from the PR?

Hmm I still see the file in the PR - is it possible you only reverted the diff? If a normal rm docs/source/serialization.rst + git commit doesn't fix it, perhaps running

git rm docs/source/serialization.rst

does?

Now it should be okay
TU

lewtun

Thank you very much for these last round of changes @MaximovaIrina !

Once the CI tests pass, this PR LGTM 🚀 - OK for you too @LysandreJik ?

lewtun · 2022-01-10T15:18:03Z

Hey @MaximovaIrina would you mind rebasing your branch on master and resolving the merge conflicts?

Gently pinging @LysandreJik for his blessing on this PR too :)

…nnxConfig

LysandreJik

Looks good to me! Thanks for your PR, @MaximovaIrina!

LysandreJik · 2022-01-11T09:07:33Z

Could you just run the code quality tool to ensure that the code quality passes? You can install them with the following, from the root of your clone:

pip install -e ".[quality]"

And then run them with:

make fixup

lewtun · 2022-01-11T11:17:01Z

Merging this since all the CI checks now pass and we have approval from a core maintainer. Thank you for your contribution @MaximovaIrina 🤗 !

LysandreJik reviewed Dec 22, 2021

View reviewed changes

lewtun reviewed Dec 22, 2021

View reviewed changes

michaelbenayoun reviewed Dec 23, 2021

View reviewed changes

MaximovaIrina force-pushed the add-onnx-ibert branch from ed36409 to d1be26d Compare December 23, 2021 14:07

lewtun reviewed Dec 23, 2021

View reviewed changes

MaximovaIrina force-pushed the add-onnx-ibert branch from 46c3689 to 5d378df Compare December 23, 2021 15:09

lewtun approved these changes Dec 23, 2021

View reviewed changes

MaximovaIrina added 5 commits January 10, 2022 18:38

Add IBertOnnxConfig and tests

5d8efd3

add all the supported features for IBERT and remove outputs in IbertO…

98bf4cf

…nnxConfig

use OnnxConfig

915ccf9

fix codestyle

0b03b3b

remove serialization.rst

76c8659

MaximovaIrina force-pushed the add-onnx-ibert branch from 987f06c to 76c8659 Compare January 10, 2022 15:38

LysandreJik approved these changes Jan 11, 2022

View reviewed changes

codestyle

7fbaf1f

lewtun merged commit c4fa908 into huggingface:master Jan 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds IBERT to models exportable with ONNX #14868

Adds IBERT to models exportable with ONNX #14868

MaximovaIrina commented Dec 21, 2021

LysandreJik left a comment

lewtun left a comment

lewtun Dec 22, 2021

MaximovaIrina Dec 23, 2021

lewtun Dec 23, 2021 •

edited

Loading

michaelbenayoun Dec 23, 2021

michaelbenayoun Dec 23, 2021

lewtun commented Dec 23, 2021

MaximovaIrina commented Dec 23, 2021

lewtun Dec 23, 2021

MaximovaIrina Dec 23, 2021

lewtun Dec 23, 2021

MaximovaIrina Dec 23, 2021 •

edited

Loading

lewtun left a comment •

edited

Loading

lewtun commented Jan 10, 2022

LysandreJik left a comment

LysandreJik commented Jan 11, 2022

lewtun commented Jan 11, 2022

		@@ -0,0 +1,433 @@
		..
		Copyright 2020 The HuggingFace Team. All rights reserved.

Adds IBERT to models exportable with ONNX #14868

Adds IBERT to models exportable with ONNX #14868

Conversation

MaximovaIrina commented Dec 21, 2021

LysandreJik left a comment

Choose a reason for hiding this comment

lewtun left a comment

Choose a reason for hiding this comment

lewtun Dec 22, 2021

Choose a reason for hiding this comment

MaximovaIrina Dec 23, 2021

Choose a reason for hiding this comment

lewtun Dec 23, 2021 • edited Loading

Choose a reason for hiding this comment

michaelbenayoun Dec 23, 2021

Choose a reason for hiding this comment

michaelbenayoun Dec 23, 2021

Choose a reason for hiding this comment

lewtun commented Dec 23, 2021

MaximovaIrina commented Dec 23, 2021

lewtun Dec 23, 2021

Choose a reason for hiding this comment

MaximovaIrina Dec 23, 2021

Choose a reason for hiding this comment

lewtun Dec 23, 2021

Choose a reason for hiding this comment

MaximovaIrina Dec 23, 2021 • edited Loading

Choose a reason for hiding this comment

lewtun left a comment • edited Loading

Choose a reason for hiding this comment

lewtun commented Jan 10, 2022

LysandreJik left a comment

Choose a reason for hiding this comment

LysandreJik commented Jan 11, 2022

lewtun commented Jan 11, 2022

lewtun Dec 23, 2021 •

edited

Loading

MaximovaIrina Dec 23, 2021 •

edited

Loading

lewtun left a comment •

edited

Loading