-
Notifications
You must be signed in to change notification settings - Fork 511
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 CvTONNX Config #1131
Add CvTONNX Config #1131
Conversation
The documentation is not available anymore as the PR was closed or merged. |
Hi, thank you for the addition! Tests indeed pass, except the style one. Could you run |
@@ -550,6 +550,11 @@ def inputs(self) -> Dict[str, Dict[int, str]]: | |||
return {"pixel_values": {0: "batch_size", 1: "num_channels", 2: "height", 3: "width"}} | |||
|
|||
|
|||
class CvTOnnxConfig(ViTOnnxConfig): | |||
DEFAULT_ONNX_OPSET = 13 | |||
ATOL_FOR_VALIDATION = 1e-2 |
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 seems pretty high tbh, do you have any idea which op makes it fail?
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.
Are you talking about the tolerance or the opset version? Im not sure why lower tolerance doesn't work, but einsum
causes an error for lower opset versions
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 am talking about the tolerance, we might have a hidden bug if we need 1e-2
, or we might not, but just wanted to raise the potential issue here.
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.
Any way this can be checked?
@fxmarty 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, thank you for the addition!
@rishabbala Maybe before merging, could you try to run the export on a larger CvT model and see what the diff is versus pytorch in the validation?
The diff for cvt-13 is 3.5e-3, for cvt-13-384-22k it is 6e-3 and for cvt-21 it is 2.7e-3. Should I reduce the tolerance? |
Sounds fine to me, thanks! |
What does this PR do?
Added changes for CvT model
Fixes part of #555
Before submitting
@michaelbenayoun @fxmarty