-
Notifications
You must be signed in to change notification settings - Fork 27.6k
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 ONNX support for LayoutLMv3 #17953
Conversation
@NielsRogge The supported tasks are question answering, token classification and sequence classification. Is there any other use case that should be supported? Also, the order of input arguments for the |
The documentation is not available anymore as the PR was closed or merged. |
@lewtun All slow tests passed |
if self.task in ["question-answering", "sequence-classification"]: | ||
return OrderedDict( | ||
[ | ||
("input_ids", {0: "batch", 1: "sequence"}), | ||
("attention_mask", {0: "batch", 1: "sequence"}), | ||
("bbox", {0: "batch", 1: "sequence"}), | ||
("pixel_values", {0: "batch", 1: "sequence"}), | ||
] | ||
) | ||
else: | ||
return OrderedDict( | ||
[ | ||
("input_ids", {0: "batch", 1: "sequence"}), | ||
("bbox", {0: "batch", 1: "sequence"}), | ||
("attention_mask", {0: "batch", 1: "sequence"}), | ||
("pixel_values", {0: "batch", 1: "sequence"}), | ||
] | ||
) |
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.
This is unfortunate, but we probably can't change it due to backwards compatibility.
cc @LysandreJik
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 wonder if we could add a deprecation warning to notify users that the order of arguments will change in v5? (Or is that type of breaking change too extreme?)
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.
Thanks a lot for adding another novel ONNX config @regisss - you're on 🔥 !
This PR LGTM, so gently pinging @LysandreJik or @sgugger for final approval 🙏
Edit: I just saw that some of the CI tests failed. I've re-run them as they don't seem to be related to your changes. If they fail again, I suggest rebasing on main
and pushing again to see if that resolves it. We should wait until the CI is green before merging ;-)
@@ -40,6 +40,7 @@ | |||
|
|||
if TYPE_CHECKING: | |||
from ..feature_extraction_utils import FeatureExtractionMixin | |||
from ..processing_utils import ProcessorMixin |
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.
Nice use of the type checks!
CI fails because of the following error: Traceback (most recent call last):
File "utils/check_repo.py", line 768, in <module>
check_repo_quality()
File "utils/check_repo.py", line 762, in check_repo_quality
check_all_objects_are_documented()
File "utils/check_repo.py", line 675, in check_all_objects_are_documented
+ "\n - ".join(undocumented_objs)
Exception: The following objects are in the public init so should be documented:
- OptionalDependencyNotAvailable
- dummy_scatter_objects
- sys It seems to come from the following line in from ...processing_utils import ProcessorMixin |
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.
That is what happens when you introduce new cyclical imports, it creates hard-to-debug errors everywhere ;-)
So let's contain imports for type checking as forward references and not do top-level imports :-)
Wow thanks a lot @sgugger for the clear explanation, it makes complete sense! |
Thanks! |
@regisss Thank you for your great work, when convert layoutxlm LayoutLMv2ForRelationExtraction to onnx, we are blocked by relation extraction layer for some reasons, can you try to export LayoutLMv2ForRelationExtraction model to onnx and give us for some help? gret thanks for you! |
@NielsRogge Thanks for your great work, when I convert LayoutLMv2ForRelationExtraction to onnx, I can not export relation extraction layer to onnx, can you help me to solve it? because the deadline is coming for the project, I hope you can help me, Thank you very much. |
@githublsk Where did you find |
@regisss it is just in microsoft/unlim,please refer to the link: |
@regisss if you have time, please help us, I am a newer to it, great thanks for you! |
@githublsk Open an issue because it is not related to this PR. And provide the command/script you ran with the complete error message please, screenshots are not very helpful. |
* Add ONNX support for LayoutLMv3 * Update docstrings * Update empty description in docstring * Fix imports and type hints
@githublsk |
@gjj123 replace torch.nn.Bilinear with this one
|
What does this PR do?
This PR adds ONNX support for LayoutLMv3. Linked to #16308.
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
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.