Repository navigation
Feedback #2
Description
Hey, I just did a simple integration of libtorch as an addon for nodejs. The API worked very nice and I really enjoyed the design, it's very well done ! I'm adding in this issue some feedbacks (don't know if here is the best place, but here you go):
- The
torch::jit::loadisn't exposed by just adding thetorch/torch.hheader, you have to include thetorch/script.has well, don't know if it is intentional but it took me some time to find the right header, maybe a documentation improvement could help; - There are some local paths hardcoded into the ATen cmake files, such as in
share/cmake/ATen/ATenConfig.cmake:/Users/administrator/nightlies/2018_10_01/wheel_build_dirs/libtorch_2.7/pytorch/torch/lib/tmp_install/include; - Documentation is still missing some parts, especially regarding loading TorchScript, etc. Given that a lot of people will start to use the libtorch, I would be nice to have a expanded documentation on ATen/Torch API with some examples (I can contribute as well);
- Problem when seeing the docs for the
torch::jit::load(https://pytorch.org/cppdocs/api/function_namespacetorch_1_1jit_1ace2c44fb8af5905ae17834e81086b8a3.html#exhale-function-namespacetorch-1-1jit-1ace2c44fb8af5905ae17834e81086b8a3) - Many broken broken in the initial page of the docs (
torch::nn,torch::optim,torch::data,torch::serialize,torch::jitandtorch::python)
This is what I remember, if I see anything else I'll add it here as well.
Reacted by Adam Paszke, walkacross, Peter Goldsborough and Dendi Suhubdy
Metadata
Metadata
Assignees
Labels
No labels
Activity
goldsborough commentedon Oct 8, 2018
Hey, thanks a lot for the feedback @perone. I'll respond to everything in detail shortly.
goldsborough commentedon Oct 10, 2018
So, for the first point: This is sort of intended. The
torch/torch.hheader is for the C++ frontend, which is not strictly the same as the TorchScript C++ API.torch/script.his intended to expose all necessary headers to interface with TorchScript, whiletorch/torch.his intended to expose all necessary headers for defining and training models with the C++ frontend.I will fix the ATenConfig.cmake thing. Did that cause an error for your, or are you just mentioning it? It looks wrong, but I haven't seen errors from this yet.
For documentation: Did you see the tutorial on exporting a model to C++ and loading it, https://pytorch.org/tutorials/advanced/cpp_export.html? Which tutorial would you liked to have had? I can write more.
I will fix the
torch::jit::loadissue. It also doesn't seem to have good docs anyway.I fixed the various
torch::*namespace links in pytorch/pytorch#12521.Let me know your thoughts and thanks so much for all the very helpful feedback!
perone commentedon Oct 12, 2018
Hi @goldsborough thanks a lot for the quick reply ! Regarding the
ATenConfig.cmakeI haven't faced errors, just mentioned because I saw it by accident. I didn't saw the https://pytorch.org/tutorials/advanced/cpp_export.html tutorial, looks amazing. I guess the useful tutorials would be the ones like this one you did and focusing on the custom types (like thetorch::jit::IValuethat you mentioned for instance) and how to handle tensors, what are the copy/move semantics for them, etc. Also, I would avoid theautoon tutorials, it kind of omits the return type for who is reading, I understand it's very useful in code but for tutorials seems to hide important information.Thanks again for the amazing work ! 👍
Some documentation fixes (#12521)
goldsborough commentedon Oct 16, 2018
I fixed the
torch::jit::loaddocs. The text needs an update too, but at least the overloads show up correctly.