-
Notifications
You must be signed in to change notification settings - Fork 7.2k
Add test that returns number of registered vision ops #2807
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2807 +/- ##
=======================================
Coverage 73.31% 73.31%
=======================================
Files 99 99
Lines 8724 8724
Branches 1373 1373
=======================================
Hits 6396 6396
Misses 1909 1909
Partials 419 419 Continue to review full report at Codecov.
|
Thanks for the PR @bmanga ! For adding it together with CI, I think the easiest for now is to add a few more lines in vision/packaging/build_cmake.sh Lines 85 to 101 in 4db3dc6
If you need some more specific pointers on this, I think the right person to ask would be @andfoy |
@fmassa thanks for the pointer. How do I signal success/failure from that script? does |
@bmanga I would think so, although tests seems to have passed on Linux, so maybe there is something that we are missing here. |
@fmassa Indeed that's strange. I will have a look to see if the CI does something peculiar to make this work. Who is the person mainly responsible for the CI? I can confirm that with my local setup (master torch, torchvision) the ops are not registered automatically, and I would assume I'm not the only one given the various torchvision issues. |
Probably better not to pass the count as the return code, it's just pretty weird and non-idiomatic. Print it and then test it from the scaffolding script. This may also fix your CI problems. |
Hi @bmanga! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
1 similar comment
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Adds a simple executable that returns the number of registered torchvision ops. This should tell us whether the torchvision ops were successfully registered when
<torchvision/vision.h>
is included (pending PR #2798).It's still missing integration in the CI as I am not familiar with that. The CI script should check that the program return value is > 0 (should return 9 for now).
@fmassa who should I ask for help in that regard?
@ezyang Do you think this suffices, provided we test enough plat/compiler combinations?