Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add libtorch/1.8.1 #5100

Closed
wants to merge 5 commits into from
Closed

Conversation

SpaceIm
Copy link
Contributor

@SpaceIm SpaceIm commented Mar 31, 2021

Specify library name and version: lib/1.0

This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!

#621

closes #6861


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot

This comment has been minimized.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Mar 31, 2021

Well, I guess that it won't build in CI without #1510. Moreover pyyaml module will be required for build scripts.
If I allow python 3.7, pyyaml + typing_extensions modules are required.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Apr 1, 2021

Here is a summary of the current status for this PR:

  • the set of default options is not too far from the pre build "CPU" binaries available on https://pytorch.org/. Except that we can't use MKL as default BLAS implementation. Moreover, default binaries should be built against fbgemm & gloo, not yet available.
  • binaries available on https://pytorch.org/ don't seem to honor default options in pytorch build files, I don't know which default options values should be followed in this recipe.
  • for the moment I've disabled lapack, but libtorch expects to use the implementation provided by the BLAS backend.
  • dependency on xnnpack (optional, but enabled by default) indirectly restricts to Visual Studio 2019.
  • with these default options, it builds with many configurations on Windows, Linux & Macos (see https://github.com/SpaceIm/conan-libtorch, even if it fails sometimes due to out of disk space in the container).
  • it requires python3 and few external modules (at least pyyaml) at build time to generate several source files. Since cpython recipe is not yet available, libtorch recipe highly relies on python 3.8 (to avoid more external modules) installed on the system.
  • I've tested locally with Visual Studio and most options enabled, it works with few limitations:
    • In 1.8.0, with_vulkan failed with Visual Studio at least, and after several bug fixes (usage of C++20 features while pytorch is C++14, missing friend classes) I gave up. I can test again in 1.8.1
    • with_cuda fails with VC++ 14.28+ (known issue Windows CUDA builds are failing in HUD pytorch/pytorch#54382, seems to be a regression in Visual Studio), so I was not able to fully test CUDA build (fails around 1300 on 1800 compilation units).
    • I've not tested others BLAS backend than OpenBLAS
  • There is still something wrong in headers installation due to cmake wrapper (a source_subfolder directory is installed). I need to dig more deeply into build files to find where is comes from. => fixed
  • I allow build with CUDA, but CUDA libraries are not propagated in package_info().
  • I believe that mkl & cuda virtual recipe should be created.
  • if static, several torch libs must be linked with whole archive. I've implemented a tedious workaround in package_info() for that.
  • If static, and depending on compiler avx support, Caffe2_perfkernels_avx, Caffe2_perfkernels_avx2 and Caffe2_perfkernels_avx512 libs can be built & installed or not. Instead of maintaining the list of compilers with these supports (tedious and error prone), package_info() tests if these libs are available.
  • On Linux, clang with libc++ fails in configure due to a simple compilation test, I don't know why.
  • On Linux, when with_numa=True (default value), libnuma must be shared, because numa static can't be linked into shared libs.
  • "mobile" build is not tested an likely fails
  • The "CPU" build can be quite fast (12 minutes with my desktop computer on Windows) or very long (1h15 on github action, ages on Appveyor). "CUDA" buids are very long.

@SpaceIm SpaceIm mentioned this pull request Apr 3, 2021
@conan-center-bot

This comment has been minimized.

@SpaceIm SpaceIm mentioned this pull request Apr 10, 2021
4 tasks
@SpaceIm
Copy link
Contributor Author

SpaceIm commented May 5, 2021

@jgsogo Would it be possible to install python 3.8 or 3.9 in agents? It would be nice to have libtorch recipe in CCI, even if build_requirements are not perfect.

@jgsogo
Copy link
Contributor

jgsogo commented May 5, 2021

@jgsogo Would it be possible to install python 3.8 or 3.9 in agents? It would be nice to have libtorch recipe in CCI, even if build_requirements are not perfect.

💯 !! We need to upgrade it in all instances (Win/Lin/Mac). It will help us to learn about how some machines are provisioned.

@jgsogo jgsogo added the infrastructure Waiting on tools or services belonging to the infra label May 5, 2021
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

OpenLarry added a commit to NaoDevils/conan-packages that referenced this pull request Dec 20, 2021
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Feb 21, 2022

@SSE4, adding python in build requirements is not sufficient since few py files invoked at build time depend on few external modules. I'll work again on this PR when I have time.

@SSE4
Copy link
Contributor

SSE4 commented Feb 21, 2022

@SSE4, adding python in build requirements is not sufficient since few py files invoked at build time depend on few external modules. I'll work again on this PR when I have time.

okay, I just wanted to do some quick tests if cpython resolves our infrastructure issues or not.

@conan-center-bot

This comment has been minimized.

@Tumb1eweed
Copy link

Tumb1eweed commented Jul 7, 2022

Specify library name and version: lib/1.0

This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!

#621

closes #6861

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

A small problem with this recipe, if build libtorch with "USE_CUDA" option, then it would require nccl as collective communicate library, but nccl
only provide makefile, so when you invoke CMake(self) method, it wouldn't build nccl, end up causing failure. Nccl provide source, so we can forge a nccl recipe from the start.

@OpenLarry
Copy link

Are there any plans to continue work on this PR in the near future? I would appreciate an updated version, but I don't think I can do it myself.... :-(

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Oct 27, 2022

Yes, when I have time. It's a very complex recipe, and it should be ported to conan v2, use cpython recipe as a tool_require (not easy since build step depends on at least one external python dependency), and be updated to latest libtorch version.

@CLAassistant
Copy link

CLAassistant commented May 18, 2023

CLA assistant check
All committers have signed the CLA.

@conan-center-bot conan-center-bot added Failed and removed infrastructure Waiting on tools or services belonging to the infra labels Jun 25, 2023
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ❌

Failure in build 7 (5d357c8e555c6d99a92d4f2437dfc27750f845f3):

  • libtorch/1.8.1@:
    Error running command conan info libtorch/1.8.1@#919663d16e3ca32c3b68dd0ee680ca09 --json {jsonName} -pr {profileName}:
    [settings]
    arch=x86_64
    build_type=Release
    compiler=gcc
    compiler.libcxx=libstdc++11
    compiler.version=5
    os=Linux
    [options]
    libtorch:shared=False
    
    ...
    WARN: tcl/8.6.10: requirement zlib/1.2.13 overridden by tk/8.6.10 to zlib/1.2.11 
    WARN: fontconfig/2.13.93: requirement expat/2.5.0 overridden by tk/8.6.10 to expat/2.4.1 
    WARN: freetype/2.13.0: requirement zlib/1.2.13 overridden by fontconfig/2.13.93 to zlib/1.2.11 
    WARN: libpng/1.6.39: requirement zlib/1.2.13 overridden by freetype/2.13.0 to zlib/1.2.11 
    ERROR: Conflict in protobuf/3.17.1:
        'protobuf/3.17.1' requires 'zlib/1.2.13' while 'libpng/1.6.39' requires 'zlib/1.2.11'.
        To fix this conflict you need to override the package 'zlib' in your root package.
    

Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.


Conan v2 pipeline (informative, not required for merge) ❌

Note: Conan v2 builds are informative and they are not required for the PR to be merged.

The v2 pipeline failed. Please, review the errors and note this will be required for pull requests to be merged in the near future.

See details:

Sorry, the system is under maintenance and it doesn't accept builds right now. Thanks for your understanding and help with the Conan Center Index!

@stale
Copy link

stale bot commented Aug 7, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 7, 2023
@stale
Copy link

stale bot commented Oct 15, 2023

This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions.

@elvisdukaj
Copy link
Contributor

I saw that also the vcpkg of this recipe is missing :\

@valgur valgur mentioned this pull request Jul 30, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[request] pytorch/1.9
8 participants