[jit] Initial torchbind prototype#21098
Conversation
jamesr66a
left a comment
There was a problem hiding this comment.
Cool! I think you're on the right track. Can you add some documentation about what this currently breaks in the PR description?
facebook-github-bot
left a comment
There was a problem hiding this comment.
@Chillee is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
There was a problem hiding this comment.
@Chillee is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary:
I have some test code in there as well, along with a script "test_libtorch" to run it. You'll need to modify `test_libtorch` to point to where you have `pytorch` built. I currently require that `pybind11` is included as a subdirectory of the test, but added it to the `.gitignore` to make this reviewable.
Currently, something like this works:
```cpp
struct Foo {
int x, y;
Foo(): x(2), y(5){}
Foo(int x_, int y_) : x(x_), y(y_) {}
void display() {
cout<<"x: "<<x<<' '<<"y: "<<y<<endl;
}
int64_t add(int64_t z) {
return (x+y)*z;
}
};
static auto test = torch::jit::class_<Foo>("Foo")
.def(torch::jit::init<int64_t, int64_t>())
.def("display", &Foo::display)
.def("add", &Foo::add)
.def("combine", &Foo::combine);
```
with
```py
torch.jit.script
def f(x):
val = torch._C.Foo(5, 3)
val.display()
print(val.add(3))
```
results in
```
x: 5 y: 3
24
```
Current issues:
- [x] The python class created by torchscript doesn't interactly properly with the surrounding code.
```
torch.jit.script
def f(x):
val = torch._C.Foo(5, 3)
return val
```
- [x] Doesn't properly take in non-pointer classes. Can't define this function signature in cpp (We don't want to support this I believe).
```cpp
void combine(Foo x) {
```
- [x] Has some issues with memory for blobs when constructing multiple objects (fix constant propagation pass to not treat capsules as the same object).
```py
torch.jit.script
def f(x):
val = torch._C.Foo(5, 3)
val2 = torch._C.Foo(100, 0)
val.display()
print(val.add(3))
```
- [ ] Can't define multiple constructors (need to define overload string. Currently not possible since we don't support overloaded methods).
- [x] `init` is a little bit different syntax than `pybind`. `.init<...>()` instead of `.def(py::init<>())`
- [x] I couldn't figure out how to add some files into the build so they'd be copied to the `include/` directories, so I symlinked them manually.
- [ ] Currently, the conversion from Python into Torchscript doesn't work.
- [ ] Torchbind also currently requires Python/Pybind dependency. Fixing this would probably involve some kind of macro to bind into Python when possible.
- [ ] We pass back into Python by value, currently. There's no way of passing by reference.
- [x] Currently can only register one method with the same type signature. This is because we create a `static auto opRegistry`, and the function is templated on the type signature.
Somewhat blocked on pytorch/pytorch#21177. We currently use some structures that will be refactored by his PR (namely `return_type_to_ivalue` and `ivalue_to_arg_type`.
Pull Request resolved: pytorch/pytorch#21098
Differential Revision: D16634872
Pulled By: Chillee
fbshipit-source-id: 1408bb89ea649c27d560df59e2cf9920467fe1de
|
@Chillee Is there a high level explanation of why pybind11 is inappropriate for pytorch's jit module? I'm just curious. |
|
@galv: we would like to be able to bind types into Python and the JIT at the same time, and ensure they have the same underlying implementation. pybind11 is great (and we try to create a similar interface here) but of course does not know about the JIT |
|
Ah, yes, that would be wonderful! No more creating separate pybind
interfaces and torchscript modules!
…On Wed, Aug 7, 2019 at 9:56 PM Michael Suo ***@***.***> wrote:
@galv <https://github.com/galv>: we would like to be able to bind types
into Python and the JIT at the same time, and ensure they have the same
underlying implementation. pybind11 is great (and we try to create a
similar interface here) but of course does not know about the JIT
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#21098?email_source=notifications&email_token=ABEL6UAUEZTLLEBABUF6QJLQDORRBA5CNFSM4HQW7GQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD32NWBY#issuecomment-519363335>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABEL6UENAARGAIUELUDZ3HTQDORRBANCNFSM4HQW7GQA>
.
--
Daniel Galvez
http://danielgalvez.me
https://github.com/galv
|

I have some test code in there as well, along with a script "test_libtorch" to run it. You'll need to modify
test_libtorchto point to where you havepytorchbuilt. I currently require thatpybind11is included as a subdirectory of the test, but added it to the.gitignoreto make this reviewable.Currently, something like this works:
with
results in
Current issues:
initis a little bit different syntax thanpybind..init<...>()instead of.def(py::init<>())include/directories, so I symlinked them manually.static auto opRegistry, and the function is templated on the type signature.Somewhat blocked on #21177. We currently use some structures that will be refactored by his PR (namely
return_type_to_ivalueandivalue_to_arg_type.