-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Use opset_version 16 instead of 18 since 16 is the latest supported version #619
Conversation
Signed-off-by: jcwchen <[email protected]>
Signed-off-by: jcwchen <[email protected]>
Signed-off-by: jcwchen <[email protected]>
Signed-off-by: jcwchen <[email protected]>
Signed-off-by: jcwchen <[email protected]>
Signed-off-by: jcwchen <[email protected]>
Signed-off-by: jcwchen <[email protected]>
Signed-off-by: jcwchen <[email protected]>
Signed-off-by: jcwchen <[email protected]>
if start == -1: | ||
raise Exception(f"Cannot find {cache_name} in {line}.") | ||
return line[start + len(cache_name) + 1:] | ||
raise Exception(f"Cannot find Build dir in {stdout}.") |
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 @ramkrishna2910 for the idea. In this PR it will use output to get the cache directory location.
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 would recommend reading the Build dir
field directly for the path.
The user can choose to specify --cache-dir
in which case .cache will not be in the path.
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.
Good point. I have updated it to use the last token from split("/"). PTAL. Thanks!
Signed-off-by: jcwchen <[email protected]>
Signed-off-by: jcwchen <[email protected]>
Signed-off-by: jcwchen <[email protected]>
Use opset_version 16 instead of 18 since 16 is the latest supported version by torch.onnx.export. There are some conversion failures when exporting opset_version 18. Use 16 for now and we will update opset_version in the future.