Skip to content

Conversation

NicolasHug
Copy link
Member

Summary:
This diff adds a new target to torchvision which enables users to use torchvision ops from C++.

For now, the cpp_library is not used by the python_cpp_library. We should instead refactor the logic in torchvision to directly use cpp_library instead.

There is currently an inconsistency between fbcode and OSS users. OSS users can import torchvision via

#include <torchvision/vision.h>

while fbcode users need to do

#include <torchvision/csrc/vision.h>

It would be good to fix this discrepancy in the future.

I didn't directly use test_frcnn_tracing.cpp due to complications for getting the .pt file in a way that works for both OSS and fbcode, so instead we added a self-contained test that should validate that the torchvision ops are properly registered and visible to JIT

Reviewed By: @datumbox

Differential Revision: D26225669

fbshipit-source-id: 5dd9fb98dd58e854f95806e4860d02f54fc04ea4

Summary:
This diff adds a new target to torchvision which enables users to use torchvision ops from C++.

For now, the `cpp_library` is not used by the `python_cpp_library`. We should instead refactor the logic in torchvision to directly use `cpp_library` instead.

There is currently an inconsistency between fbcode and OSS users. OSS users can import torchvision via
```
#include <torchvision/vision.h>
```
while fbcode users need to do
```
#include <torchvision/csrc/vision.h>
```
It would be good to fix this discrepancy in the future.

I didn't directly use `test_frcnn_tracing.cpp` due to complications for getting the `.pt` file in a way that works for both OSS and fbcode, so instead we added a self-contained test that should validate that the torchvision ops are properly registered and visible to JIT

Reviewed By: datumbox

Differential Revision: D26225669

fbshipit-source-id: 5dd9fb98dd58e854f95806e4860d02f54fc04ea4
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@datumbox datumbox merged commit aa26498 into pytorch:master Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants