-
Notifications
You must be signed in to change notification settings - Fork 862
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
Export TIMM image models to onnx #2564
Conversation
0de339b
to
eaba988
Compare
764c5ba
to
ce2bd08
Compare
Job PR-2564-ce2bd08 is done. |
Job PR-2564-5b7e30d is done. |
torch.onnx.export( | ||
self._model.eval(), | ||
batch, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we still use the dict format as model input? It seems that @FANGAreNotGnu made it work when export hf_text models.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason we are moving away from dict input comes with two folds: 1) there are limitations in exporting onnx, 2) there are lots of benefits if we keep model inputs / outputs clear, see huggingface/transformers#16059
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dict input design is to flexibly support various modalities, backbones, and fusions in the AutoMM. I think changing the design may cause issues for some cases. For example, it seems difficult to list arguments of a fusion model since it may fuse any combinations of image, text, and tabular backbones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For most functions in AutoMM, we have used the type hints. The dict design can make each model more modular. A batch may contain inputs for several backbones and a model only needs to extract its necessary information from the batch. This is especially useful in various fusion settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the dict does not work, does it make sense for us to switch to dataclasses or namedtuple @liangfu @zhiqiangdon ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using namedtuple is feasible. Similar to https://github.com/huggingface/transformers/blob/45da7cec5aa1b1bf1031af9caa9085e95e262e11/examples/research_projects/bertabs/run_summarization.py#L28
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can try namedtuple, but not sure whether it is compatible with onnx.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
namedtuple is now experimentally working as inputs for tracing, but not yet ready to be part of collate
function. will try integrate it into collate
function in a follow-up PR.
Job PR-2564-be11941 is done. |
Job PR-2564-03670f4 is done. |
Job PR-2564-b732b91 is done. |
Job PR-2564-aac3c98 is done. |
Job PR-2564-16430e3 is done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for fixing onnx export! Remember resolving the conflicts with the master branch.
Job PR-2564-7411c95 is done. |
Issue #, if available:
Description of changes:
a. onnx 1.13.x requires protobuf >=3.12.2,<=3.20.1, (onnx 1.12.x requires protobuf >=3.12.2,<=3.20.1)
b. tensorboardx 2.5.1 requires protobuf >=3.8.0,<=3.20.1
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.