add ONNX support for BLOOM by NouamaneTazi · Pull Request #17961 · huggingface/transformers

NouamaneTazi · 2022-06-30T12:42:48Z

What does this PR do?

add ONNX support for BLOOM

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?

@michaelbenayoun

HuggingFaceDocBuilderDev · 2022-06-30T12:52:30Z

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

younesbelkada · 2022-06-30T13:17:53Z

As you told me offline that the slow tests were passing (under torch1.11.0), looks good to me! Thanks for working on that 🔥

lewtun

Thanks for adding the Bloom config @NouamaneTazi - a very clean PR 💮 !

If the slow tests pass, this PR looks good to me. Let's wait for approval from @LysandreJik or @sgugger before merging this

Edit: I see the CI is now failing for an unrelated issue. I've re-run it, but if it comes up red again, I suggest rebasing on main and pushing again

lewtun · 2022-06-30T13:44:17Z

src/transformers/models/bloom/configuration_bloom.py

        super().__init__(bos_token_id=bos_token_id, eos_token_id=eos_token_id, **kwargs)
+
+
+# Copied from transformers.models.gpt2.configuration_gpt2.GPT2OnnxConfig


lewtun · 2022-06-30T13:44:59Z

src/transformers/models/bloom/configuration_bloom.py

+    ):
+        super().__init__(config, task=task, patching_specs=patching_specs, use_past=use_past)
+        if not getattr(self._config, "pad_token_id", None):
+            # TODO: how to do that better?


Hehe @michaelbenayoun we should fix this sometime :)

lewtun · 2022-06-30T13:52:23Z

tests/onnx/test_onnx_v2.py

 }

 PYTORCH_EXPORT_WITH_PAST_MODELS = {
+    ("bloom", "bigscience/bloom-350m"),


Have you checked that the slow tests pass for this checkpoint? You can run:

RUN_SLOW=1 pytest tests/onnx/test_onnx_v2.py -k "bloom"

Thank you for reminding me. All tests are passing now 🙂

michaelbenayoun

Great, thanks for handling this!

src/transformers/models/bloom/configuration_bloom.py

michaelbenayoun · 2022-06-30T14:35:47Z

src/transformers/models/bloom/modeling_bloom.py

+        seq_ids = torch.arange(max_positions, device=input.device)
        causal_mask = (
-            torch.tril(torch.ones((max_positions, max_positions), dtype=torch.bool))
+            (seq_ids[None, None, :] <= seq_ids[None, :, None])


Dont know if this is relevant but the original implementation outputs a tensor of rank 2, and your change outputs a tensor of rank 3. Should not be a big deal since we do reshape it afterwards but just wanted to point this out.

Do we keep it like that?

Fixed :) Thank you for the notice 🤗

sgugger · 2022-06-30T15:18:13Z

I'm not too sure about the changes in modeling_bloom.py. Looks like not leveraging the bool type and converting to int32 will hurt performance. Wdyt @younesbelkada ?

michaelbenayoun · 2022-06-30T15:41:45Z

I think the changes in modeling_bloom.py come from the fact that boolean tensors cannot be added in ONNX (not 100% sure). Two suggestions then:

Reformulate the addition to torch.logical_or
Cast the input to int8

I think that the first solution is both faster and more aligned with the original implementation.
WDYT?

younesbelkada · 2022-06-30T15:46:52Z

@sgugger I do not think this will hurt performances in terms of logits since slow tests are passing, but might hurt indeed the inference time performance for large and/or batched sequences.. We need to benchmark that though to be sure

sgugger · 2022-06-30T15:49:49Z

@michaelbenayoun I think option 1 sounds good, yes!

michaelbenayoun

LGTM

src/transformers/models/bloom/modeling_bloom.py

michaelbenayoun · 2022-07-01T10:16:12Z

src/transformers/models/bloom/modeling_bloom.py

+        seq_ids = torch.arange(max_positions, device=input.device)
        causal_mask = (
-            torch.tril(torch.ones((max_positions, max_positions), dtype=torch.bool))
+            (seq_ids[None, None, :] <= seq_ids[None, :, None])


Do we keep it like that?

src/transformers/models/bloom/configuration_bloom.py

michaelbenayoun · 2022-07-01T10:19:54Z

Also make sure all the tests pass before merging.

NouamaneTazi · 2022-07-01T10:25:49Z

All tests for tests/onnx/test_onnx_v2.py -k "bloom" and tests/models/bloom are passing.
Here are the ones that are skipped (which is fine according to @younesbelkada)

================================================================================= short test summary info =================================================================================
SKIPPED [1] tests/test_modeling_common.py:2006: test is PT+FLAX test
SKIPPED [1] tests/test_modeling_common.py:1934: test is PT+FLAX test
SKIPPED [1] tests/test_modeling_common.py:1758: test is PT+TF test
SKIPPED [1] tests/test_tokenization_common.py:1960: This test is only for slow tokenizers
SKIPPED [1] tests/test_tokenization_common.py:2189: test is PT+TF test
================================================================= 159 passed, 5 skipped, 35 warnings in 449.50s (0:07:29)

sgugger · 2022-07-01T13:18:31Z

There is a difference between a copy in BLOOM and the original in GPT-2 which is why the CI is failing. Make sure to run make fic-copies or remove the Copied from.

src/transformers/models/bloom/configuration_bloom.py

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

sgugger

Thanks a lot!

* add onnx support for BLOOM * use TYPE_CHECKING for type annotations * fix past_shape for bloom (different from gpt2) * use logical_or instead of `+` for onnx support * bigger `atol_for_validation` for larger bloom models * copied -> taken because it's no longer an exact copy * remove "copied from" comment Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com> Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

add onnx support for BLOOM

2009710

lewtun approved these changes Jun 30, 2022

View reviewed changes

michaelbenayoun approved these changes Jun 30, 2022

View reviewed changes

NouamaneTazi added 3 commits July 1, 2022 10:34

use TYPE_CHECKING for type annotations

c15f12a

fix past_shape for bloom (different from gpt2)

fb1b76c

use logical_or instead of + for onnx support

0f9199d

NouamaneTazi requested a review from michaelbenayoun July 1, 2022 10:12

michaelbenayoun approved these changes Jul 1, 2022

View reviewed changes

NouamaneTazi added 2 commits July 1, 2022 16:09

bigger atol_for_validation for larger bloom models

cf43ce2

copied -> taken because it's no longer an exact copy

a6fb1be

sgugger reviewed Jul 1, 2022

View reviewed changes

src/transformers/models/bloom/configuration_bloom.py Outdated Show resolved Hide resolved

remove "copied from" comment

71ecf5d

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

sgugger approved these changes Jul 1, 2022

View reviewed changes

sgugger merged commit b68d408 into huggingface:main Jul 1, 2022

		super().__init__(bos_token_id=bos_token_id, eos_token_id=eos_token_id, **kwargs)


		# Copied from transformers.models.gpt2.configuration_gpt2.GPT2OnnxConfig

Conversation

NouamaneTazi commented Jun 30, 2022

What does this PR do?

Before submitting

Who can review?

Uh oh!

HuggingFaceDocBuilderDev commented Jun 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

younesbelkada commented Jun 30, 2022

Uh oh!

lewtun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelbenayoun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgugger commented Jun 30, 2022

Uh oh!

michaelbenayoun commented Jun 30, 2022

Uh oh!

younesbelkada commented Jun 30, 2022

Uh oh!

sgugger commented Jun 30, 2022

Uh oh!

michaelbenayoun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

michaelbenayoun commented Jul 1, 2022

Uh oh!

NouamaneTazi commented Jul 1, 2022

Uh oh!

sgugger commented Jul 1, 2022

Uh oh!

Uh oh!

sgugger left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

HuggingFaceDocBuilderDev commented Jun 30, 2022 •

edited

Loading

lewtun left a comment •

edited

Loading