ci(package-c): build pytorch plugin without bundled runtime#5706
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates the C library packaging pipeline for PyTorch runtime handling, adds container and CI checks around the packaged tarball, revises the installation guide, and changes the CUDA architecture default selection. ChangesPyTorch-aware packaging pipeline
CUDA architecture default
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@source/install/package_c.sh`:
- Around line 119-215: The PyTorch runtime URL derivation in package_c.sh can
fail silently when ENABLE_PYTORCH is true but PYTHON_BIN is missing or the torch
version heuristic does not produce a supported URL. Add a clear warning or
diagnostic in the URL-generation block before the README and
download_libtorch.sh generation is skipped, using the existing
PYTORCH_RUNTIME_DOWNLOAD_URL, ENABLE_PYTORCH, and PYTHON_BIN checks to explain
why no PyTorch runtime package was emitted. Keep the warning tied to the same
install-script flow so local/manual runs surface the missing libtorch packaging
immediately.
- Around line 122-138: The libtorch URL generation in the torch version handling
logic is stripping nightly `.devYYYYMMDD` data by splitting on `.dev`, so fix
the version parsing in the package_c script to preserve the full nightly version
string when building the download URL. Update the `version`/`variant` extraction
path around the `torch.__version__` and `torch.version.cuda` handling so nightly
builds keep their `.dev` suffix while still producing the correct `cpu` or `cu*`
libtorch archive name.
In `@source/lib/src/gpu/CMakeLists.txt`:
- Around line 9-17: The CUDA architecture fallback in CMakeLists.txt is too
narrow because the `all-major` branch in the `find_package(CUDAToolkit)` /
`CMAKE_CUDA_ARCHITECTURES` logic stops at CUDA 12.x. Update the version check so
the `all-major` fallback also applies to CUDA 13.x, keeping the existing
`CUDAToolkit_VERSION` guard and the `set(CMAKE_CUDA_ARCHITECTURES ...)` logic in
place while extending the upper bound appropriately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3872c764-c654-40a6-9a12-928f465e34ab
📒 Files selected for processing (7)
.github/workflows/package_c.ymldoc/install/install-from-c-library.mdsource/api_c/CMakeLists.txtsource/install/docker_package_c.shsource/install/docker_test_package_c.shsource/install/package_c.shsource/lib/src/gpu/CMakeLists.txt
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5706 +/- ##
==========================================
- Coverage 81.20% 81.15% -0.05%
==========================================
Files 978 990 +12
Lines 108858 111018 +2160
Branches 4139 4237 +98
==========================================
+ Hits 88393 90101 +1708
- Misses 18946 19390 +444
- Partials 1519 1527 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
Tests
Fork CI: https://github.com/njzjz/deepmd-kit/actions/runs/28547373402
Summary by CodeRabbit
LD_LIBRARY_PATH).