Fix rpaths #18

raimis · 2022-06-03T15:12:05Z

Revert baf0463

Checklist

Used a personal fork of the feedstock to propose changes
Bumped the build number (if the version is unchanged)
Reset the build number to 0 (if the version changed)
Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
Ensured the license file is being packaged.

Revert conda-forge@baf0463

conda-forge-linter · 2022-06-03T15:12:08Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

raimis · 2022-06-03T15:13:25Z

@conda-forge-admin, please rerender

…nda-forge-pinning 2022.06.03.11.00.48

carterbox · 2022-06-03T15:48:06Z

Why? This package doesn't install anything into $SP/torch/lib and that namespace belongs to pytorch.

raimis · 2022-06-03T15:53:01Z

It needs libtorch.so and friends, which are at {{ SP_DIR }}/torch/lib.

carterbox · 2022-06-03T16:08:39Z

Makes sense. Seems like something @conda-forge/pytorch-cpu should do upstream if pytorch extension packages need to link against libtorch.so and friends.

mikemhenry · 2022-06-03T17:59:38Z

@hmaarrfk do we need to make a change upstream?

peastman · 2022-06-03T19:53:06Z

Seems like something https://github.com/orgs/conda-forge/teams/pytorch-cpu should do upstream if pytorch extension packages need to link against libtorch.so and friends.

That would be great, if it would save downstream packages from needing to add it.

hmaarrfk · 2022-06-03T19:57:16Z

We might need to fix the feedstock,

hmaarrfk · 2022-06-03T19:57:44Z

Why are the tests not catching errors?

carterbox · 2022-06-03T20:19:16Z

Why are the tests not catching errors?

Possibly because you can't test load/run any binaries that link against libcuda.so on the CI because there are no GPUs?

hmaarrfk · 2022-06-03T21:48:08Z

recipe/meta.yaml

  string: cuda{{ cuda_compiler_version | replace('.', '') }}py{{ CONDA_PY }}h{{ PKG_HASH }}_{{ PKG_BUILDNUM }}  # [cuda_compiler_version != "None"]
  string: cpu_py{{ CONDA_PY }}h{{ PKG_HASH }}_{{ PKG_BUILDNUM }}                                      # [cuda_compiler_version == "None"]
  skip: true  # [win or (linux and cuda_compiler_version in (undefined, 'None', '10.2'))]
+  rpaths:
+    - lib/
+    - {{ SP_DIR }}/torch/lib


can we try adding:

requirements: run: - cudnn # [cuda_compiler_version != "None"] - nccl # [cuda_compiler_version != "None"]

similar to https://github.com/conda-forge/pytorch-cpu-feedstock/blob/main/recipe/meta.yaml#L58 instead?

I guess these two lines won't help.

2022-06-01T04:06:06.6901147Z WARNING (openmm-torch,lib/libOpenMMTorch.so): $RPATH/libc10.so not found in packages, sysroot(s) nor the missing_dso_whitelist. 2022-06-01T04:06:06.6901798Z .. is this binary repackaging? 2022-06-01T04:06:06.6916172Z WARNING (openmm-torch,lib/libOpenMMTorch.so): $RPATH/libtorch_cpu.so not found in packages, sysroot(s) nor the missing_dso_whitelist. 2022-06-01T04:06:06.6917017Z .. is this binary repackaging? 2022-06-01T04:06:06.6930804Z WARNING (openmm-torch,lib/libOpenMMTorch.so): $RPATH/libtorch_cuda.so not found in packages, sysroot(s) nor the missing_dso_whitelist. 2022-06-01T04:06:06.6931457Z .. is this binary repackaging? 2022-06-01T04:06:06.6945306Z WARNING (openmm-torch,lib/libOpenMMTorch.so): $RPATH/libtorch.so not found in packages, sysroot(s) nor the missing_dso_whitelist. 2022-06-01T04:06:06.6945926Z .. is this binary repackaging?

Is a really strange error to me.

Truthfully, this is where my knowledge of the build systems ends.

It is unclear to me why we need the rpaths here, and not in torchvision.

https://github.com/conda-forge/torchvision-feedstock/blob/main/recipe/meta.yaml#L27

@conda-forge/cudatoolkit-dev if you could chime in here it owuld be helpful.

One reason why I think this is happening is that you are doing alot of custom configuration to find the file libtoch.so within the python sitepackages directory. This is somewhat frowned upon and is not OK. this is likely what all these RPATHs are trying to warn you about. I think, that for now, you should do what you need to do to link to this package. to "work". I think adding these "IGNORE RPATHS" is ok.

can we try adding:

requirements: run: - cudnn # [cuda_compiler_version != "None"] - nccl # [cuda_compiler_version != "None"]

similar to https://github.com/conda-forge/pytorch-cpu-feedstock/blob/main/recipe/meta.yaml#L58 instead?

Why?

I think adding the RPATH is correct. My longer term goal is to package libtorch to enable this kind of stuff to happen naturally.

The hackery i'm talking about is:
https://github.com/conda-forge/openmm-torch-feedstock/blob/main/recipe/build.sh#L12

Setting TORCH_DIR to this strange directory is the hack.

Though for now, I don't have a better solution. My latest attempts at refactoring the pytorch package are documented in conda-forge/pytorch-cpu-feedstock#108 but there is still a long way to go (especially with GPU) and with the python side of the package too.

My longer term goal is to package libtorch to enable this kind of stuff to happen naturally.

A straightforward way of doing that would be to have it put its libraries in the main lib directory instead of a private lib directory inside the Python module.

Thank you @peastman. Given the constraints of time, I don't have time to do this at the moment. But yes, it is in the longer term goal i outlined in conda-forge/pytorch-cpu-feedstock#108

So given the current state of things, I think we should be good to merge this in, I see how it is ideal but it seems there isn't really anything else to do until pytorch drops the libs in the correct place (or a separate package is created)

Correct, the only one i'm still unconvinced of is the lib RPATH. Not sure where that is coming from. But if you all want to merge, then I don't see "harm" that can be made from this.

mikemhenry · 2022-06-07T17:24:51Z

recipe/meta.yaml

  string: cuda{{ cuda_compiler_version | replace('.', '') }}py{{ CONDA_PY }}h{{ PKG_HASH }}_{{ PKG_BUILDNUM }}  # [cuda_compiler_version != "None"]
  string: cpu_py{{ CONDA_PY }}h{{ PKG_HASH }}_{{ PKG_BUILDNUM }}                                      # [cuda_compiler_version == "None"]
  skip: true  # [win or (linux and cuda_compiler_version in (undefined, 'None', '10.2'))]
+  rpaths:
+    - lib/
+    - {{ SP_DIR }}/torch/lib


So given the current state of things, I think we should be good to merge this in, I see how it is ideal but it seems there isn't really anything else to do until pytorch drops the libs in the correct place (or a separate package is created)

mikemhenry · 2022-06-07T19:29:14Z

@conda-forge-admin, please rerender

github-actions · 2022-06-07T19:40:12Z

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/openmm-torch-feedstock/actions/runs/2456955267.

Fix rpaths

7d3c0c3

Revert conda-forge@baf0463

raimis self-assigned this Jun 3, 2022

Bump build number

ce6e14f

MNT: Re-rendered with conda-build 3.21.9, conda-smithy 3.20.0, and co…

61dc6e2

…nda-forge-pinning 2022.06.03.11.00.48

raimis mentioned this pull request Jun 3, 2022

Fix the tutorial openmm/openmm-torch#77

Merged

2 tasks

raimis marked this pull request as ready for review June 3, 2022 16:30

raimis requested review from jaimergp, mikemhenry and peastman as code owners June 3, 2022 16:30

peastman approved these changes Jun 3, 2022

View reviewed changes

hmaarrfk reviewed Jun 3, 2022

View reviewed changes

mikemhenry approved these changes Jun 7, 2022

View reviewed changes

mikemhenry merged commit 804c6da into conda-forge:main Jun 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix rpaths #18

Fix rpaths #18

raimis commented Jun 3, 2022 •

edited

Loading

conda-forge-linter commented Jun 3, 2022

raimis commented Jun 3, 2022

carterbox commented Jun 3, 2022

raimis commented Jun 3, 2022

carterbox commented Jun 3, 2022

mikemhenry commented Jun 3, 2022

peastman commented Jun 3, 2022

hmaarrfk commented Jun 3, 2022

hmaarrfk commented Jun 3, 2022

carterbox commented Jun 3, 2022

hmaarrfk Jun 3, 2022

hmaarrfk Jun 3, 2022

hmaarrfk Jun 3, 2022

hmaarrfk Jun 5, 2022

raimis Jun 7, 2022

hmaarrfk Jun 7, 2022

peastman Jun 7, 2022

hmaarrfk Jun 7, 2022

mikemhenry Jun 7, 2022

hmaarrfk Jun 7, 2022

mikemhenry Jun 7, 2022

mikemhenry commented Jun 7, 2022

github-actions bot commented Jun 7, 2022

Fix rpaths #18

Fix rpaths #18

Conversation

raimis commented Jun 3, 2022 • edited Loading

conda-forge-linter commented Jun 3, 2022

raimis commented Jun 3, 2022

carterbox commented Jun 3, 2022

raimis commented Jun 3, 2022

carterbox commented Jun 3, 2022

mikemhenry commented Jun 3, 2022

peastman commented Jun 3, 2022

hmaarrfk commented Jun 3, 2022

hmaarrfk commented Jun 3, 2022

carterbox commented Jun 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikemhenry commented Jun 7, 2022

github-actions bot commented Jun 7, 2022

raimis commented Jun 3, 2022 •

edited

Loading