f_ pattern bloating

[JonathanWoollett-Light]

In my view it seems unnecessary to duplicate every function which may return an error, e.g. Tensor::reshape and Tensor::f_reshape.

This pattern is not seen in any other popular libraries (the Tensorflow bindings and ndarray being two examples), and I believe for good reason. It adds gigantic bloat to documentation and the codebase all to simply abstract an unwrap away (the inverse of propagating an error, so it both adds bloat and makes errors slightly less clear).

What is the reason behind this design choice?

[LaurentMazare]

The main reason for this is that this crate is a wrapper around the C++ PyTorch api which uses exceptions to report errors so it's not easy to know statically which C++ method could raise an exception and which shouldn't. The alternative would be to only provide the functions that can fail but that would result in all the model code having a lot of extra unwrap or ? which would not feel that nice. We've actually put some significant effort in having a wide variety of example which you could see in the examples directory, and this pattern has served us pretty well there, using the f_ functions at the few places where a failure is really a concern and the other ones at most of the places.
As to the additional "bloat" that this creates, one point to note is that almost all of this code is automatically generated so there isn't any additional code being written manually for these. For the documentation, we automatically generate methods for more than 500 underlying C++ methods so I doubt anyway would try to read the whole Tensor documentation anyway, I haven't found that to be an issue.
If you haven't already done so, I would encourage you to write some simple models with this and let us know how you find it in practice.

[JonathanWoollett-Light]

I can definitely see this may be a personal preference, but I would rather use .unwrap() or ? constantly (as tends to already be the case with many frameworks) than have 2x the documentation to read/search through (and of course this approach differs from almost every other Rust library including bindings, which is somewhat awkward. There is no f_ version for tensorflow::ops::reshape).

almost all of this code is automatically generated so there isn't any additional code being written manually for these

What about documentation, does this not require duplicating any rustdoc comments? (or make writting rustdoc comment more awkward?).

I suppose these factors become less significant if it is expected the Rust documentation shouldn't really be used for anything other than just matching the method names from the Python documentation.

using the f_ functions at the few places where a failure is really a concern and the other ones at most of the places.

I would argue that any method which could fail should return Result<_> (this tends to be the Rust'y approach).

I was thinking about submitting a pull request adding significant amount to documentation with bunch of examples, would this even work, is there some non-standard mechanism I would need to use here?

---

Also a slightly off topic question.

From readme.md

More idiomatic rust bindings could then be developed on top of this.

Does this mean a pull request adding a macro tensor! like array! from ndarray would be considered?

[LaurentMazare]

almost all of this code is automatically generated so there isn't any additional code being written manually for these

What about documentation, does this not require duplicating any rustdoc comments? (or make writting rustdoc comment more awkward?).

The automatically generated part has no documentation. It's a one to one mapping with the C++ api so one should refer to this bit for the up to date documentation.

I was thinking about submitting a pull request adding significant amount to documentation with bunch of examples, would this even work, is there some non-standard mechanism I would need to use here?

That would be a problem indeed as such documentation would be removed on the next pytorch updated. And even if it wasn't removed it might become out of sync. Ideally the file providing the function description that we use to generate the code would also include the documentation but that's not the case at the moment (tensorflow packages all this together btw which is neat).

Does this mean a pull request adding a macro tensor! like array! from ndarray would be considered?

The idea is that this crate provides some mostly low level wrappers around the c++ api. More opiniated/featurefull framework can probably be build on top of it. Re array! I would suggest implementing this in a separate small crate to experiment with it and give it a bit of usage, and we can probably revisit later down the road.

[danieldk]

I have once converted the main project that I use tch for from non-fallible to fallible, among other reasons because the errors that come from unwrap() are very difficult to read for users. Having a lot of f_-prefixed method calls looks a bit ugly.

I guess an alternative would be to make two separate traits for methods that have fallible/unwrapping counterparts and let the user import the TensorFallible or TensorUnwrap depending on which behavior they want.