Skip to content

fix(lmp): link torch for builtin shared builds#5535

Open
njzjz-bot wants to merge 1 commit into
deepmodeling:masterfrom
njzjz-bothub:fix/issue-5516
Open

fix(lmp): link torch for builtin shared builds#5535
njzjz-bot wants to merge 1 commit into
deepmodeling:masterfrom
njzjz-bothub:fix/issue-5516

Conversation

@njzjz-bot

@njzjz-bot njzjz-bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Problem

  • LAMMPS builtin builds only link DeePMD::deepmd_c.
  • When DeePMD-kit is installed with the PyTorch backend, libdeepmd_cc.so can still require torch/c10 symbols at the final lmp link step in shared LAMMPS builds.

Change

  • Export PyTorch backend/link metadata from the DeePMD CMake package config.
  • Teach source/lmp/builtin.cmake to link Torch into the LAMMPS target when the installed DeePMD package reports PyTorch support.
  • Keep a fallback for existing concrete Torch library paths and warn when Torch cannot be found.

Verification

  • uvx --from cmakelang==0.6.13 cmake-format --check source/cmake/Config.cmake.in source/lmp/builtin.cmake
  • Configured a synthetic LAMMPS/DeePMD/Torch CMake project and verified lammps links DeePMD::deepmd_c;torch.

Fixes #5516

Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)

Summary by CodeRabbit

  • Chores
    • Enhanced PyTorch integration by exposing additional configuration variables in the build configuration.
    • Improved LAMMPS linking behavior to properly integrate Torch libraries when PyTorch support is enabled, with enhanced library resolution and fallback handling.

Expose PyTorch backend metadata from the DeePMD CMake package and use it in the LAMMPS builtin integration to link Torch when DeePMD was built with the PyTorch backend. This resolves shared LAMMPS builds that fail to resolve torch/c10 symbols from libdeepmd_cc.so.

Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)
@dosubot dosubot Bot added the build label Jun 14, 2026
@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds four PyTorch-related variables to the CMake package config template (Config.cmake.in) so downstream consumers can read them, then introduces a _deepmd_lammps_link_torch() helper in builtin.cmake that uses those exported values to resolve and link Torch libraries into the lammps target when DeePMD_ENABLE_PYTORCH is on.

Changes

PyTorch Torch Linking for LAMMPS Builtin Mode

Layer / File(s) Summary
Exported PyTorch config variables
source/cmake/Config.cmake.in
Adds set() declarations for _ENABLE_PYTORCH, _USE_PT_PYTHON_LIBS, _PYTORCH_CMAKE_PREFIX_PATH, and _TORCH_LIBRARIES using @...@ substitutions so downstream consumers of the installed package can read these values.
LAMMPS Torch linking helper and invocation
source/lmp/builtin.cmake
Adds _deepmd_lammps_link_torch(target_name) which reads the exported prefix path and library list, runs find_package(Torch QUIET), links ${TORCH_LIBRARIES} on success, or falls back to filtered entries from DeePMD_TORCH_LIBRARIES, emitting a WARNING if neither yields usable libraries. Calls the function on the lammps target after DeePMD sources are added.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

C

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(lmp): link torch for builtin shared builds' accurately describes the main change, which is implementing torch linking for LAMMPS builtin integration in shared library mode.
Linked Issues check ✅ Passed The PR successfully addresses issue #5516 by exporting PyTorch backend metadata in Config.cmake.in and implementing torch linking logic in builtin.cmake to resolve undefined torch/c10 symbol errors in shared library builds.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the torch linking issue: Config.cmake.in exports PyTorch variables and builtin.cmake implements the linking helper function. No unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
source/lmp/builtin.cmake (1)

78-83: Proposed filtering change is defensive but addresses valid edge cases for multi-config CMake setups.

The current filter (lines 78-83) keeps items that are CMake targets, existing files, or linker flags starting with "-", dropping everything else. While PyTorch's TORCH_LIBRARIES typically contains only absolute file paths (without optimized/debug/general keywords per PyTorch's CMake design), the proposed fix is worth considering for robustness:

  1. It explicitly preserves optimized/debug/general keyword pairs if they ever appear
  2. It adds support for CMake generator expressions ($<...>) which can be useful in multi-config scenarios
  3. It relaxes filtering of unresolved library names

The current filtering isn't known to cause immediate issues with typical PyTorch setups, but the proposed change improves defensiveness against edge cases and future CMake configuration variations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@source/lmp/builtin.cmake` around lines 78 - 83, The filtering condition in
the foreach loop that checks each _deepmd_torch_library item is too restrictive
for multi-config CMake setups. Expand the if condition to also preserve items
that start with the keywords optimized, debug, or general (which may appear in
multi-config scenarios), items that are CMake generator expressions (matching
patterns like $<...>), and consider relaxing the filter to be more permissive
with unresolved library names. Modify the condition following the TARGET,
EXISTS, and "-" prefix checks to add these additional cases using logical OR
operators to ensure robustness across different PyTorch CMake configurations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@source/lmp/builtin.cmake`:
- Around line 78-83: The filtering condition in the foreach loop that checks
each _deepmd_torch_library item is too restrictive for multi-config CMake
setups. Expand the if condition to also preserve items that start with the
keywords optimized, debug, or general (which may appear in multi-config
scenarios), items that are CMake generator expressions (matching patterns like
$<...>), and consider relaxing the filter to be more permissive with unresolved
library names. Modify the condition following the TARGET, EXISTS, and "-" prefix
checks to add these additional cases using logical OR operators to ensure
robustness across different PyTorch CMake configurations.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3f506c85-e078-4d57-b661-53940cb5d751

📥 Commits

Reviewing files that changed from the base of the PR and between 87d8557 and 02a1911.

📒 Files selected for processing (2)
  • source/cmake/Config.cmake.in
  • source/lmp/builtin.cmake

@codecov

codecov Bot commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.18%. Comparing base (87d8557) to head (02a1911).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5535      +/-   ##
==========================================
- Coverage   82.18%   82.18%   -0.01%     
==========================================
  Files         890      890              
  Lines      101358   101358              
  Branches     4240     4240              
==========================================
- Hits        83301    83300       -1     
  Misses      16756    16756              
- Partials     1301     1302       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

[BUG] builtin.cmake + BUILD_SHARED_LIBS + PyTorch backend: undefined reference to torch/c10 when linking lmp

1 participant