Skip to content

fix(lammps): share Kokkos runtime with LAMMPS - #5989

Open
njzjz-bot wants to merge 3 commits into
deepmodeling:masterfrom
njzjz-bot:fix/lammps-kokkos-runtime
Open

fix(lammps): share Kokkos runtime with LAMMPS#5989
njzjz-bot wants to merge 3 commits into
deepmodeling:masterfrom
njzjz-bot:fix/lammps-kokkos-runtime

Conversation

@njzjz-bot

@njzjz-bot njzjz-bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Backport 0005-share-lammps-kokkos-runtime.patch from conda-forge/deepmd-kit-feedstock@d02138263b0b30629fe28c68c8bcecc2b8644513.

A runtime LAMMPS plugin should reuse the Kokkos runtime already linked into liblammps. Linking Kokkos::kokkos again can create a second CUDA/Kokkos runtime instance and corrupt shutdown state when the plugin unloads.

Import the Kokkos include paths, compile definitions/options/features, and link options needed for device compilation while leaving runtime Kokkos symbols to liblammps; retain the CUDA and LIBDL helper targets when available.

Validation

  • Single-file backport matching the feedstock patch.
  • Split from fix: backport conda-forge build fixes #5985 so the runtime-linking change can be reviewed independently.
  • Full build/runtime validation is left to upstream CI.

Agent: ChatGPT
Model: GPT-5.6 Sol

Summary by CodeRabbit

  • Bug Fixes
    • Improved Kokkos plugin build configuration by correctly inheriting required include paths, compiler settings, and linker options.
    • Added conditional support for CUDA and dynamic loading capabilities when available.

Backport conda-forge/deepmd-kit-feedstock patch 0005 at d02138263b0b30629fe28c68c8bcecc2b8644513.

Agent: ChatGPT
Model: GPT-5.6 Sol
@njzjz
njzjz marked this pull request as ready for review August 22, 2026 19:17
Copilot AI lite review requested due to automatic review settings August 22, 2026 19:17

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@njzjz
njzjz requested a review from OutisLi August 22, 2026 19:18
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8c140c4d-d974-438c-aa3a-bb8fd0fb5b15

📥 Commits

Reviewing files that changed from the base of the PR and between 6d45869 and c121e01.

📒 Files selected for processing (1)
  • source/lmp/plugin/CMakeLists.txt

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The Kokkos plugin CMake configuration collects interface properties from available umbrella and component targets. It applies those properties to the plugin and preserves optional Kokkos::CUDA and Kokkos::LIBDL linkage.

Changes

Kokkos plugin configuration

Layer / File(s) Summary
Propagate Kokkos interface properties
source/lmp/plugin/CMakeLists.txt
The plugin collects Kokkos include directories, compile definitions, compile options, compile features, and link options from available targets. Optional Kokkos::CUDA and Kokkos::LIBDL targets remain linked.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to c121e

This localized build-configuration change is merge-ready after normal checks and review; no actionable merge-blocking risk remains.

Suggested reviewers: outisli

🚥 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 clearly describes the main change: sharing the Kokkos runtime between the LAMMPS plugin and LAMMPS.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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.

@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.84%. Comparing base (8cfd46e) to head (c121e01).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5989      +/-   ##
==========================================
- Coverage   79.10%   78.84%   -0.26%     
==========================================
  Files        1105     1105              
  Lines      130981   130981              
  Branches     4771     4771              
==========================================
- Hits       103609   103273     -336     
- Misses      25686    26024     +338     
+ Partials     1686     1684       -2     

☔ 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.

Collect non-link interface properties from the Kokkos umbrella and component targets so LAMMPS build-tree packages do not inject NOTFOUND include paths.

Coding-Agent: Codex
Codex-Version: codex-cli 0.149.0
Model: gpt-5.6-sol
Reasoning-Effort: xhigh
@njzjz

njzjz commented Aug 23, 2026

Copy link
Copy Markdown
Member

During the requested Kokkos-enabled plugin validation, I found and fixed an additional build-tree compatibility issue in c121e013a: Kokkos::kokkos is an umbrella target in LAMMPS builds and may not directly define INTERFACE_INCLUDE_DIRECTORIES. The previous code therefore passed a NOTFOUND path to CMake. The fix collects non-link interface properties from the umbrella and component targets while still avoiding the static Kokkos runtimes.

Validated against stable_22Jul2025_update5, patch_10Sep2025, and patch_20Aug2026 with PKG_PLUGIN=ON, PKG_KOKKOS=ON, and DEEPMD_LAMMPS_KOKKOS=ON:

  • all three plugin builds completed;
  • plugin load and plugin list succeeded;
  • all six DeepMD/DPA4Spin Kokkos aliases were registered;
  • the plugin link lines contain no libkokkos*.a;
  • the plugin defines no Kokkos runtime initialization/finalization symbols and has no Kokkos dynamic dependency;
  • all three LAMMPS processes exited normally after loading the plugin.

Coding agent: Codex
Codex version: codex-cli 0.149.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants