search

FluxML / Torch.jl

Sensible extensions for exposing torch in Julia.
Other
213 stars 15 forks source link

Update wrapper #54

Open stemann opened 2 years ago

stemann commented 2 years ago

@DhairyaLGandhi Can you help with some hints for how to most easily update the wrapper (e.g. for Torch 1.10) ?

It seems there has been a change on the return type from void to int - is there some ocaml generator code somewhere?

I've compared the contents of c37c480710b5ce173ede1458e3c2e6a2ff9fddb5 and https://github.com/LaurentMazare/ocaml-torch, and found that the original source for the wrapper seems to have been tag 0.8, i.e. https://github.com/LaurentMazare/ocaml-torch/tree/85da09301df133e9d97a8340c1ce74a13651ca60/src/wrapper

Re-applying the complete set of changes to the build dir. (e.g. https://github.com/IHPSystems/Torch.jl/commit/e86f40e559e1dce0c8281b7cac3819f261b72939) on top of an updated ocaml-torch copy (https://github.com/LaurentMazare/ocaml-torch/tree/a6499811f40282a071179d4306afbbb6023dcc4a/src/wrapper) is not completely problem-free.

Relates to https://github.com/JuliaPackaging/Yggdrasil/pull/4554

DhairyaLGandhi commented 2 years ago

Absolutely! The cmake Script does need to be updated. Could you say what the issue specifically was? Happy to help get this updated

stemann commented 2 years ago

OK.

I just tried simply cherry picking https://github.com/IHPSystems/Torch.jl/commit/e86f40e559e1dce0c8281b7cac3819f261b72939 on top of https://github.com/IHPSystems/Torch.jl/tree/stemann/wrap_torch_1.10 and ran into quite a lot of merge conflicts. So I thought that there might be a small change to https://github.com/LaurentMazare/ocaml-torch/tree/main/src/gen which could make most of the merge conflicts go away :-)

stemann commented 2 years ago

Making some progress - I've re-done the edits to the gen.ml script needed for outputting the current generated C api: https://github.com/IHPSystems/Torch.jl/tree/stemann/build_wrapper/build/wrapper_generator

DhairyaLGandhi commented 2 years ago

Why are some functions excluded? Is there a way to build wrappers for these as well? I also noticed that you'd redefined types in ocaml to match cpp

stemann commented 2 years ago

Which functions do you mean ("functions excluded")?

I just redefined types in ocaml to match the torch_api_generated files that are currently in this repo. - to be sure I could replicate the same process with a newer libtorch.

ToucheSir commented 2 years ago

Presumably "functions excluded" refers to https://github.com/IHPSystems/Torch.jl/blob/stemann/build_wrapper/build/wrapper_generator/bin/main.ml#L8, but my understanding is that list is integral to the build process of ocaml-torch and was implicitly used anyhow for the initial binding generation.

Would it be easier to directly update to the latest binding generator and then try to revamp the Julia API if anything's changed? Also, would using https://github.com/LaurentMazare/tch-rs/tree/main/gen help since it appears to be more up to date than https://github.com/LaurentMazare/ocaml-torch/tree/main/src/gen?

stemann commented 2 years ago

Yes, we should definitely use the up to date tch-rs wrapper generator - just wanted to reproduce the steps for generating 1.4 bindings.

The wrapper generators are identical wrt. generating C code if one compares ocaml-torch for torch 1.10 with tch-rs for torch 1.10.

stemann commented 2 years ago

Planned course of action:

  1. Pin Torch_jll dependency to v1.4 (#55)
  2. Build/Register Torch_jll v1.10.2+ (just libtorch - without wrapper)
  3. Build/Register TorchWrapper_jll - which builds/depends on Torch_jll
  4. Update Torch.jl to use TorchWrapper_jll
stemann commented 1 year ago

Finally got around to doing something more wrt. bullet point 4 - cf. #56.

stemann commented 12 months ago

Updated plan:

  1. ✅ #60
  2. ✅ #59 to have that part cleaned-up
  3. ✅ #61
  4. Update Torch_jll recipe in Yggdrasil - it is missing CUDA platform augmentation, I believe. Seems like this was already done.
  5. ✅https://github.com/JuliaPackaging/Yggdrasil/pull/6004 : Build the C wrapper in Yggdrasil, e.g. as TorchCAPI_jll - having Torch_jll as a dependency.

  6. 64: Update Torch.jl to use the updated C wrapper

rayegun commented 6 months ago

@stemann what can I do to get this through the finish line? Also we'd probably like to go with a recent 2.x release here soon

stemann commented 6 months ago

This reminder helps a lot :-)

But push for and remind me about #61 - and help review it - should help.

2.x should be done by first updating Torch_jll and its dependencies - cf. T/Torch/deps.md in Yggdrasil.

stemann commented 3 months ago

With #61 now merged, all should be set to get points 4 and 5 completed 🚀