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

Fix LAMMPS plugin symlink path on macOS platform #3473

Merged
merged 1 commit into from
Mar 17, 2024

Conversation

chazeon
Copy link
Contributor

@chazeon chazeon commented Mar 16, 2024

This pull request fixes the broken symlink for dpplugin.so on macOS. It should point to libdeepmd_lmp.so but point to libdeepmd_lmp.dylib instead.

Details

The libdeepmd_lmp is a shared module.

add_library(${libname} MODULE ${LMP_SRC})

The build target name on macOS is libdeepmd_lmp.so.

Because on macOS, the CMAKE_SHARED_MODULE_SUFFIX (.so) is different from CMAKE_SHARED_LIBRARY_SUFFIX (.dynlib). As a result, in previous versions, on macOS the symbolic link dpplugin.so was pointed to libdeepmd_lmp.dylib, which does not exist. One can check the conda-forge builds to confirm (e.g., osx-arm64/deepmd-kit-2.2.9-cpu_py311hf5376d5_mpi_openmpi_0.conda).

Copy link

codecov bot commented Mar 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.49%. Comparing base (4b3a77b) to head (adf640a).

Additional details and impacted files
@@           Coverage Diff           @@
##            devel    #3473   +/-   ##
=======================================
  Coverage   77.49%   77.49%           
=======================================
  Files         432      432           
  Lines       37161    37161           
  Branches     1620     1620           
=======================================
  Hits        28797    28797           
  Misses       7496     7496           
  Partials      868      868           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@njzjz njzjz self-requested a review March 16, 2024 23:53
@njzjz njzjz added the bug label Mar 16, 2024
@njzjz
Copy link
Member

njzjz commented Mar 17, 2024

I'll first add this patch to conda-forge to see if the tests pass. (It's a known issue but I don't have a Mac to debug)

@njzjz njzjz added this to the v2.2.10 milestone Mar 17, 2024
njzjz added a commit to njzjz/deepmd-kit-feedstock that referenced this pull request Mar 17, 2024
@chazeon
Copy link
Contributor Author

chazeon commented Mar 17, 2024

OK. If you have a Linux machine please also check if it breaks the Linux build.

Copy link
Member

@njzjz njzjz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the conda-forge's tests have passed for both linux and macos. Failure is not related.

@njzjz njzjz added this pull request to the merge queue Mar 17, 2024
Merged via the queue into deepmodeling:devel with commit abf3477 Mar 17, 2024
51 checks passed
@njzjz njzjz mentioned this pull request Apr 2, 2024
njzjz pushed a commit to njzjz/deepmd-kit that referenced this pull request Apr 6, 2024
This pull request fixes the broken symlink for `dpplugin.so` on macOS.
It should point to `libdeepmd_lmp.so` but point to `libdeepmd_lmp.dylib`
instead.

### Details

The `libdeepmd_lmp` is a shared module.

https://github.com/deepmodeling/deepmd-kit/blob/b875ea8f6661b6e1567537ead7e2b4a8b14ea113/source/lmp/plugin/CMakeLists.txt#L72

The build target name on macOS is `libdeepmd_lmp.so`.

Because on macOS, the `CMAKE_SHARED_MODULE_SUFFIX` (`.so`) is different
from `CMAKE_SHARED_LIBRARY_SUFFIX` (`.dynlib`). As a result, in previous
versions, on macOS the symbolic link `dpplugin.so` was pointed to
`libdeepmd_lmp.dylib`, which does not exist. One can check the
conda-forge builds to confirm (e.g.,
[osx-arm64/deepmd-kit-2.2.9-cpu_py311hf5376d5_mpi_openmpi_0.conda](https://anaconda.org/conda-forge/deepmd-kit/2.2.9/download/osx-arm64/deepmd-kit-2.2.9-cpu_py311hf5376d5_mpi_openmpi_0.conda)).

(cherry picked from commit abf3477)
@njzjz njzjz mentioned this pull request Apr 6, 2024
njzjz pushed a commit that referenced this pull request Apr 6, 2024
This pull request fixes the broken symlink for `dpplugin.so` on macOS.
It should point to `libdeepmd_lmp.so` but point to `libdeepmd_lmp.dylib`
instead.

### Details

The `libdeepmd_lmp` is a shared module.

https://github.com/deepmodeling/deepmd-kit/blob/b875ea8f6661b6e1567537ead7e2b4a8b14ea113/source/lmp/plugin/CMakeLists.txt#L72

The build target name on macOS is `libdeepmd_lmp.so`.

Because on macOS, the `CMAKE_SHARED_MODULE_SUFFIX` (`.so`) is different
from `CMAKE_SHARED_LIBRARY_SUFFIX` (`.dynlib`). As a result, in previous
versions, on macOS the symbolic link `dpplugin.so` was pointed to
`libdeepmd_lmp.dylib`, which does not exist. One can check the
conda-forge builds to confirm (e.g.,
[osx-arm64/deepmd-kit-2.2.9-cpu_py311hf5376d5_mpi_openmpi_0.conda](https://anaconda.org/conda-forge/deepmd-kit/2.2.9/download/osx-arm64/deepmd-kit-2.2.9-cpu_py311hf5376d5_mpi_openmpi_0.conda)).

(cherry picked from commit abf3477)
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.

2 participants