-
Notifications
You must be signed in to change notification settings - Fork 371
feat: C++ runtime on Windows #2806
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
Conversation
Some observations about the failed tests.
|
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 looks great and thank you very much for the contribution. Added a few questions/comments. The comments on the test cases + added fixes look good and switching np.dtype("i")
to np.int32
for that case is fine to include in the converter.
No ciflow labels are configured for this repo. |
Hi @HolyWu - I wanted to share a few notes on the C++ runtime on Windows after some local testing:
Based on the GHA-installed package, I ran the CI-failing tests, for which I have the following results:
|
Thanks for the notes. I have fixed the version of transformers and also add a missing test. As for the error you encountered during ninja build, I haven't seen the exact same error. But I suspect you probably didn't run |
Thanks for the suggestions. I also added Python310\include\pyconfig.h(59): fatal error C1083: Cannot open include file: 'io.h': No such file or directory |
|
Hi @HolyWu - thanks for the suggestion. Ultimately, the issue was that the repo directory was not its original name, and so the For some reason when importing the built package however, it shows The line causing the issue is where the library is registered with PyTorch and ultimately loaded, here:
|
I have no clue. |
Hi @HolyWu - ultimately, the issue was my usage of |
Yes, Bazel 6.2.1 can find MSVC utilities on my local build. Only solution you can try is setting |
Hi @gs-olive. It seems that Windows tests often encounter |
No more |
Was added in #2750
Linux workflow also does this way so the cuda version of TensorRT doesn't necessarily have to match up PyTorch's cuda version.
They don't always get installed automatically on CI and hence lead to "Can not find nvinfer" error.
Installing torchvision with legacy resolver could cause the installed version of torch conflict with the required version from torchvision, leading to errors like `RuntimeError: operator torchvision::nms does not exist` or `AttributeError: partially initialized module 'torchvision' has no attribute 'extension'`
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 all the hard work!
Description
Fix and enable C++ runtime on Windows.
Fixes #2247
Fixes #2371
Fixes #2484
Type of change
Checklist: