[RT-DETR] Fix onnx inference bug for Optype (Where) by YHallouard · Pull Request #33877 · huggingface/transformers

YHallouard · 2024-10-01T23:11:33Z

What does this PR do?

~~Implement RTDetrOnnxConfig for RT-DETR~~
Fix Error Error: Type parameter (T) of Optype (Where) bound to different types (tensor(float) and tensor(double) in node (/model/decoder/Where) during onnx inference. Already fixed on lyuwenyu/RT-DETR (Error: Type parameter (T) of Optype (Where) bound to different types (tensor(float) and tensor(double) in node (/model/decoder/Where). lyuwenyu/RT-DETR#307)

Fixes

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. Error: Type parameter (T) of Optype (Where) bound to different types (tensor(float) and tensor(double) in node (/model/decoder/Where). lyuwenyu/RT-DETR#307
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? No, I didn't found any tests related to OnnxConfig. But I still open to add tests if you think it is required.

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.

…type (Where)

HuggingFaceDocBuilderDev · 2024-10-01T23:11:46Z

Hey! 🤗 Thanks for your contribution to the transformers library!

Before merging this pull request, slow tests CI should be triggered. To enable this:

Add the run-slow label to the PR
When your PR is ready for merge and all reviewers' comments have been addressed, push an empty commit with the command [run-slow] followed by a comma separated list of all the models to be tested, i.e. [run_slow] model_to_test_1, model_to_test_2
- If the pull request affects a lot of models, put at most 10 models in the commit message
A transformers maintainer will then approve the workflow to start the tests

(For maintainers) The documentation for slow tests CI on PRs is here.

YHallouard · 2024-10-02T08:24:23Z

Hi @SangbumChoi, how are you ? I saw that you were the main maintainer of RT-DETR. Thank you very much for your work !

I propose a fix for the anchor generation to avoid a bug in onnx inference : Error: Type parameter (T) of Optype (Where) bound to different types (tensor(float) and tensor(double) in node (/model/decoder/Where). It has already been discussed here: lyuwenyu/RT-DETR#307

What do you feel about it ?

SangbumChoi

Thanks for the contribution! For me it looks great,

I think it would be helpful to add some ONNX conversion description script in rt_detr.md?

Requesting @qubvel for the final approvement!

SangbumChoi · 2024-10-02T08:37:39Z

        valid_mask = ((anchors > eps) * (anchors < 1 - eps)).all(-1, keepdim=True)
        anchors = torch.log(anchors / (1 - anchors))
-        anchors = torch.where(valid_mask, anchors, torch.inf)
+        anchors = torch.where(valid_mask, anchors, torch.tensor(float("inf"), dtype=torch.float32, device=device))


Suggested change

anchors = torch.where(valid_mask, anchors, torch.tensor(float("inf"), dtype=torch.float32, device=device))

anchors = torch.where(valid_mask, anchors, torch.tensor(float("inf"), dtype=dtype, device=device))

Added some doc f157406 :) Tell me what do you think

YHallouard · 2024-10-02T10:09:31Z

Thanks for the contribution! For me it looks great,

I think it would be helpful to add some ONNX conversion description script in rt_detr.md?

Requesting @qubvel for the final approvement!

Thanks for your reply ! Yes it would be interesting, what do you think about adding a section :

ONNX Tips

See RTDetrConfig in this section. (put the link to that section)

from transformers.models.rt_detr.import RTDetrConfig, RTDetrOnnxConfig  # type: ignore[import-untyped]
from transformers.onnx.convert import export_pytorch  # type: ignore[import-untyped]

rtdetr_onnx_config = RTDetrOnnxConfig(config=RTDetrConfig(), task="object-detection")

export_pytorch(
    preprocessor=preprocessor,
    model=model,
    config=rtdetr_onnx_config,
    opset=17,
    output=output_path,
    tokenizer=None,
    device="cuda",
)

qubvel

Hi @YHallouard, thanks a lot for your contribution! Overall looks good to me!

The only concern I have is that onnx export in transformers is no longer maintained and at some moment we might end up removing it.
It will be better to add this config to Optimum, leaving here just a snippet of code on how to export the model using Optimum.

cc @ArthurZucker regarding onnx

qubvel · 2024-10-02T16:57:20Z

        valid_mask = ((anchors > eps) * (anchors < 1 - eps)).all(-1, keepdim=True)
        anchors = torch.log(anchors / (1 - anchors))
-        anchors = torch.where(valid_mask, anchors, torch.inf)
+        anchors = torch.where(valid_mask, anchors, torch.tensor(float("inf"), dtype=dtype, device=device))


cc @yonigozlan re dtype/device for torch.compile

YHallouard · 2024-10-03T10:49:18Z

t will be better to add this config to Optimum,

Hi @qubvel, honestly I wasn't aware of this repository ! Very interesting discovery !
I see how to put it in optimum, but since there is no ORTModelForObjectDetection yet in Optimum (thing i can work on maybe in the future). What do you think about putting this config (RTDetrOnnxConfig) in transformers and in optimum ?

But I can rework the rt_detr.mdOnnx section, specifying in transformer section that is deprecated and also put a optimum section

ArthurZucker · 2024-10-05T13:48:38Z

Hey @YHallouard sorry for the confusion, I think we need better doc on this 😢 let's not add anything that we know is already deprecated! If you need guidance on ONNX and Optimum contribution for this, I am sure @michaelbenayoun will be happy to help!

YHallouard · 2024-10-06T11:55:53Z

Hi @ArthurZucker, @michaelbenayoun,

No problem, I removed th Onnx config and openned a pull request in optimum. huggingface/optimum#2040

ArthurZucker

Thanks! 🤗

ArthurZucker · 2024-10-07T07:10:49Z

        valid_mask = ((anchors > eps) * (anchors < 1 - eps)).all(-1, keepdim=True)
        anchors = torch.log(anchors / (1 - anchors))
-        anchors = torch.where(valid_mask, anchors, torch.inf)
+        anchors = torch.where(valid_mask, anchors, torch.tensor(float("inf"), dtype=dtype, device=device))


Suggested change

anchors = torch.where(valid_mask, anchors, torch.tensor(float("inf"), dtype=dtype, device=device))

anchors = torch.where(valid_mask, anchors, torch.finfo(dtype).min, dtype=dtype, device=device))

if the dtype is the dtype of valid_mask then this would make more sense ! Otherwise the min (float32 -inf) is not gonna be the same!

dtype is the dtype of anchors, valid_mask is a condition, dtypes are the same but just by construction.

But you're right, torch.finfo(dtype).max is good :)

YHallouard · 2024-10-10T14:40:44Z

Hi @ArthurZucker, should I run a run-slow ?

ArthurZucker

LGTM now thanks for updating!

HuggingFaceDocBuilderDev · 2024-10-22T13:37:38Z

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

* feat: [RT-DETR] Add onnx runtime config and fix onnx inference bug Optype (Where) * fix lint * use dtype istead of torch.float32 * add doc * remove onnx config * use dtype info * use tensor to fix lint

feat: [RT-DETR] Add onnx runtime config and fix onnx inference bug Op…

111746e

…type (Where)

YHallouard force-pushed the feat/add_config_for_RT_DETR_onnx branch from 01e5d38 to b146090 Compare October 1, 2024 23:33

fix lint

70e48fa

YHallouard force-pushed the feat/add_config_for_RT_DETR_onnx branch from b146090 to 70e48fa Compare October 1, 2024 23:36

SangbumChoi reviewed Oct 2, 2024

View reviewed changes

use dtype istead of torch.float32

2ea9b75

add doc

f157406

qubvel reviewed Oct 2, 2024

View reviewed changes

YHallouard mentioned this pull request Oct 6, 2024

[RT-Detr] - Add RT-Detr Onnx Config huggingface/optimum#2040

Closed

3 tasks

remove onnx config

e38ba18

YHallouard force-pushed the feat/add_config_for_RT_DETR_onnx branch from 2b90511 to e38ba18 Compare October 6, 2024 11:48

YHallouard changed the title ~~[RT-DETR] Add onnx config and fix onnx inference bug for Optype (Where)~~ [RT-DETR] Fix onnx inference bug for Optype (Where) Oct 6, 2024

ArthurZucker reviewed Oct 7, 2024

View reviewed changes

YHallouard added 2 commits October 7, 2024 11:01

use dtype info

bde496e

use tensor to fix lint

1aa73fe

YHallouard force-pushed the feat/add_config_for_RT_DETR_onnx branch from 6b0f18a to 1aa73fe Compare October 7, 2024 13:41

ArthurZucker approved these changes Oct 22, 2024

View reviewed changes

ArthurZucker merged commit eb6a734 into huggingface:main Oct 22, 2024

	anchors = torch.where(valid_mask, anchors, torch.tensor(float("inf"), dtype=torch.float32, device=device))
	anchors = torch.where(valid_mask, anchors, torch.tensor(float("inf"), dtype=dtype, device=device))

Uh oh!

Conversation

YHallouard commented Oct 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Before submitting

Who can review?

Uh oh!

HuggingFaceDocBuilderDev commented Oct 1, 2024

Uh oh!

YHallouard commented Oct 2, 2024

Uh oh!

SangbumChoi 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!

SangbumChoi Oct 2, 2024

Choose a reason for hiding this comment

Uh oh!

YHallouard Oct 2, 2024

Choose a reason for hiding this comment

Uh oh!

YHallouard Oct 2, 2024

Choose a reason for hiding this comment

Uh oh!

YHallouard commented Oct 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ONNX Tips

Uh oh!

qubvel 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!

qubvel Oct 2, 2024

Choose a reason for hiding this comment

Uh oh!

YHallouard commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArthurZucker commented Oct 5, 2024

Uh oh!

YHallouard commented Oct 6, 2024

Uh oh!

ArthurZucker left a comment

Choose a reason for hiding this comment

Uh oh!

ArthurZucker Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

YHallouard Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

YHallouard Oct 7, 2024 • 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!

YHallouard commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArthurZucker left a comment

Choose a reason for hiding this comment

Uh oh!

HuggingFaceDocBuilderDev commented Oct 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

YHallouard commented Oct 1, 2024 •

edited

Loading

SangbumChoi left a comment •

edited

Loading

YHallouard commented Oct 2, 2024 •

edited

Loading

qubvel left a comment •

edited

Loading

YHallouard commented Oct 3, 2024 •

edited

Loading

YHallouard Oct 7, 2024 •

edited

Loading

YHallouard commented Oct 10, 2024 •

edited

Loading