pytorch / cppdocs

PyTorch C++ API Documentation
https://pytorch.org/cppdocs
206 stars 33 forks source link

Feedback #2

Open perone opened 5 years ago

perone commented 5 years ago

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):

This is what I remember, if I see anything else I'll add it here as well.

goldsborough commented 5 years ago

Hey, thanks a lot for the feedback @perone. I'll respond to everything in detail shortly.

goldsborough commented 5 years ago

So, for the first point: This is sort of intended. The torch/torch.h header is for the C++ frontend, which is not strictly the same as the TorchScript C++ API. torch/script.h is intended to expose all necessary headers to interface with TorchScript, while torch/torch.h is 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::load issue. It also doesn't seem to have good docs anyway.

I fixed the various torch::* namespace links in https://github.com/pytorch/pytorch/pull/12521.

Let me know your thoughts and thanks so much for all the very helpful feedback!

perone commented 5 years ago

Hi @goldsborough thanks a lot for the quick reply ! Regarding the ATenConfig.cmake I 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 the torch::jit::IValue that you mentioned for instance) and how to handle tensors, what are the copy/move semantics for them, etc. Also, I would avoid the auto on 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 ! 👍

goldsborough commented 5 years ago

I fixed the torch::jit::load docs. The text needs an update too, but at least the overloads show up correctly.