ocaml / opam-repository

Main public package repository for opam, the source package manager of OCaml.
https://opam.ocaml.org
Creative Commons Zero v1.0 Universal
516 stars 1.12k forks source link

Package torch.v0.17.0 #26141

Closed public-release closed 2 months ago

public-release commented 3 months ago

torch.v0.17.0

Torch bindings for OCaml The ocaml-torch project provides some OCaml bindings for the Torch library. This brings to OCaml NumPy-like tensor computations with GPU acceleration and tape-based automatic differentiation.



:camel: Pull-request generated by opam-publish v2.3.0

dkalinichenko-js commented 3 months ago

torch requires a system-wide installation of PyTorch to function. The libtorch package provides such an installation, but it's CPU-only, and we don't want to require users to override their system install with it. Is there a way to use the libtorch package for testing in the CI while prompting users to install the right version of PyTorch on their own?

mseri commented 2 months ago

I think using with-test for the dependency is the most reasonable compromise, unfortunately it does not really work to have the tests running.

For the users you could add a post-install failure message, after all if they don't have (the right) PyTorch installed they will see the failure that we see in the build in the CI:

#=== ERROR while compiling torch.v0.17.0 ======================================#
# context              2.2.0~rc1 | linux/x86_64 | ocaml-base-compiler.5.1.1 | pinned(https://github.com/janestreet/torch/archive/refs/tags/v0.17.0.tar.gz)
# path                 ~/.opam/5.1/.opam-switch/build/torch.v0.17.0
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p torch -j 39
# exit-code            1
# env-file             ~/.opam/log/torch-7-d547ca.env
# output-file          ~/.opam/log/torch-7-d547ca.out
### output ###
# File "src/wrapper/dune", line 4, characters 9-18:
# 4 |   (names torch_api)
#              ^^^^^^^^^
# (cd _build/default/src/wrapper && /usr/bin/gcc -std=c++17 -fPIC -D_GLIBCXX_USE_CXX11_ABI=1 -g -I /home/opam/.opam/5.1/lib/ocaml -I /home/opam/.opam/5.1/lib/base -I /home/opam/.opam/5.1/lib/base/base_internalhash_types -I /home/opam/.opam/5.1/lib/base/md5 -I /home/opam/.opam/5.1/lib/base/shadow_stdlib -I /home/opam/.opam/5.1/lib/base_quickcheck -I /home/opam/.opam/5.1/lib/base_quickcheck/ppx_quickcheck/runtime -I /home/opam/.opam/5.1/lib/bigarray-compat -I /home/opam/.opam/5.1/lib/bin_prot -I /home/opam/.opam/5.1/lib/bin_prot/shape -I /home/opam/.opam/5.1/lib/ctypes -I /home/opam/.opam/5.1/lib/ctypes-foreign -I /home/opam/.opam/5.1/lib/ctypes/stubs -I /home/opam/.opam/5.1/lib/fieldslib -I /home/opam/.opam/5.1/lib/integers -I /home/opam/.opam/5.1/lib/jane-street-headers -I /home/opam/.opam/5.1/lib/ocaml/str -I /home/opam/.opam/5.1/lib/ocaml/threads -I /home/opam/.opam/5.1/lib/ocaml/unix -I /home/opam/.opam/5.1/lib/ocaml_intrinsics_kernel -I /home/opam/.opam/5.1/lib/parsexp -I /home/opam/.opam/5.1/lib/ppx_assert/runtime-lib -I /home/opam/.opam/5.1/lib/ppx_bench/runtime-lib -I /home/opam/.opam/5.1/lib/ppx_compare/runtime-lib -I /home/opam/.opam/5.1/lib/ppx_enumerate/runtime-lib -I /home/opam/.opam/5.1/lib/ppx_expect/config -I /home/opam/.opam/5.1/lib/ppx_expect/config_types -I /home/opam/.opam/5.1/lib/ppx_expect/make_corrected_file -I /home/opam/.opam/5.1/lib/ppx_expect/runtime -I /home/opam/.opam/5.1/lib/ppx_hash/runtime-lib -I /home/opam/.opam/5.1/lib/ppx_here/runtime-lib -I /home/opam/.opam/5.1/lib/ppx_inline_test/config -I /home/opam/.opam/5.1/lib/ppx_inline_test/runtime-lib -I /home/opam/.opam/5.1/lib/ppx_log/syntax -I /home/opam/.opam/5.1/lib/ppx_log/types -I /home/opam/.opam/5.1/lib/ppx_module_timer/runtime -I /home/opam/.opam/5.1/lib/ppx_sexp_conv/runtime-lib -I /home/opam/.opam/5.1/lib/ppx_stable_witness/runtime -I /home/opam/.opam/5.1/lib/ppx_stable_witness/stable_witness -I /home/opam/.opam/5.1/lib/ppx_string/runtime -I /home/opam/.opam/5.1/lib/ppxlib/print_diff -I /home/opam/.opam/5.1/lib/sexplib -I /home/opam/.opam/5.1/lib/sexplib0 -I /home/opam/.opam/5.1/lib/splittable_random -I /home/opam/.opam/5.1/lib/stdio -I /home/opam/.opam/5.1/lib/stdlib-shims -I /home/opam/.opam/5.1/lib/time_now -I /home/opam/.opam/5.1/lib/typerep -I /home/opam/.opam/5.1/lib/variantslib -I ../bindings -o torch_api.o -c torch_api.cpp)
# torch_api.cpp:1:10: fatal error: torch/csrc/autograd/engine.h: No such file or directory
#     1 | #include <torch/csrc/autograd/engine.h>
#       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# compilation terminated.
mseri commented 2 months ago

@shonfeder do you know?

shonfeder commented 2 months ago

I'm not totally clear on the question for me, sorry. Could you clarify, @mseri ?

mseri commented 2 months ago

Is there a way to test this package using the libtorch package only on the CI, but not require the package when user install it (instead fail with the error message in that case)?

So a flag to say that libtorch is a "ci" dependency only. Right now the build fails and so tests are skipped, since libtorch is only a test dependency.

shonfeder commented 2 months ago

I see. Thank you for clarifying. There is no way to add CI only dependencies currently. I'm also not sure that we want to support this. The purpose of the opam-repo-ci, as I understand it, is to ensure that installed packages work just based on the opam metadata. This gives users of the tested platforms a decent guarantee that, if the CI succeeds on the platform, then simply doing an opam install will give them a working package. If we open to door to installing CI-only dependencies, I think we would break this contract.

It seems to me that if the package cannot specify all the dependencies that it needs in order to give users a working installation, than a CI failure is the expected outcome for us, since this CI is testing deployability, not correctness of the code as in dev pipeline. WDYT?

mseri commented 2 months ago

@shonfeder I agree with you. @dkalinichenko-js is that ok with you? If so, please add the failure message recommending the PyTorch installation and keep libtorch only as depopt, and we can merge even if it fails to build in the CI

dkalinichenko-js commented 2 months ago

Would merging this package while it's failing in the CI really be ok? This will add unnecessary burden when updating dependencies of torch, since the revdeps build will fail spuriously. Perhaps there's a config option to disable some CI builds?

mseri commented 2 months ago

Would merging this package while it's failing in the CI really be ok?

We have a few packages that have this issue. Until we have a workaround for this, it is ok. We recognize those packages immediately and will not slow us down much. Unfortunately I don't think that we can do better for the moment

dkalinichenko-js commented 2 months ago

Done.

mseri commented 2 months ago

Thanks!