Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add visualbert onnx suport #2091

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tedasdf
Copy link

@tedasdf tedasdf commented Nov 7, 2024

What does this PR do?

Fixes #555 (issue)

Support VisualBERT model config.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

Who can review?

Copy link
Collaborator

@echarlaix echarlaix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution @tedasdf !

tests/exporters/exporters_utils.py Outdated Show resolved Hide resolved
@@ -1108,6 +1108,12 @@ class TasksManager:
"text-to-audio",
onnx="VitsOnnxConfig",
),
"visualbert": supported_tasks_mapping(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be

Suggested change
"visualbert": supported_tasks_mapping(
"visual-bert": supported_tasks_mapping(

https://huggingface.co/hf-internal-testing/tiny-random-VisualBertModel/blob/main/config.json#L26

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldnt that be visual_bert

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

model_type is defined as follows

model_type = model_type.lower().replace("_", "-")
so here "visual-bert"

optimum/exporters/onnx/model_configs.py Show resolved Hide resolved
optimum/exporters/tasks.py Outdated Show resolved Hide resolved
@tedasdf
Copy link
Author

tedasdf commented Dec 3, 2024

Hi @echarlaix, I wanted to thank you for reviewing the pull request. Frankly, I’m struggling a bit. I understand the files that need to be altered and the function to be implemented, but I’m having trouble finding the information related to the model, especially given that I’m not very familiar with such a large codebase. I would like to know if there are ways I can contribute more to this issue. I understand you’re busy, but if possible, could you guide me further?

I found the structure of inputs and outputs from the forward function of the model here. However, I’m not sure if the model needs the normalized text configuration or where to locate the appropriate _task_mapping attribute. I chose these areas based on the tasks the model has been fine-tuned for, but I could use some guidance on where to look.

@echarlaix echarlaix added the onnx Related to the ONNX export label Dec 3, 2024
@HuggingFaceDocBuilderDev

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.

tests/exporters/exporters_utils.py Outdated Show resolved Hide resolved
tests/exporters/exporters_utils.py Outdated Show resolved Hide resolved
optimum/exporters/tasks.py Outdated Show resolved Hide resolved
@tedasdf tedasdf closed this Dec 4, 2024
@echarlaix
Copy link
Collaborator

echarlaix commented Dec 4, 2024

Hi @tedasdf, thanks for your contribution! I see that you closed this PR yesterday but happy to help if you want to re-open it and continue working on the integration.

I found the structure of inputs and outputs from the forward function of the model here. However, I’m not sure if the model needs the normalized text configuration or where to locate the appropriate _task_mapping attribute. I chose these areas based on the tasks the model has been fine-tuned for, but I could use some guidance on where to look.

The inputs used to export the model needs to be a subset of the model's inputs (a check is done here https://github.com/huggingface/optimum/blob/v1.23.3/optimum/exporters/onnx/convert.py#L96 for this). So you need to first make sure that the inputs defined in your VisualBertOnnxConfig is a subset of https://github.com/huggingface/transformers/blob/v4.46.3/src/transformers/models/visual_bert/modeling_visual_bert.py#L700-L712

In your case I see that you have pixel_values which is not part of the forward signature, do you mean visual_embeds ? If yes, you'll need to rename it using the rename_ambiguous_inputs method. Also pixel_values inputs are generated as tensors of shape [batch_size, num_channels, height, width]

shape=[self.batch_size, self.num_channels, self.height, self.width],
which I don't think is what you expect from visual_embeds. You can add one using an DummyInputGenerator

@tedasdf tedasdf reopened this Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
onnx Related to the ONNX export
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Community contribution - optimum.exporters.onnx support for new models!
3 participants