revert(ci): undo AOTI .pt2 compile / GPU test overlap (#5882) - #5916
Conversation
) This reverts commit f62c3c9 (deepmodeling#5882), undoing the CUDA CI orchestration it added: the two-lane pytest split (`-m aoti_compile` vs the remainder), the `aoti_compile` marker machinery in source/tests/conftest.py, the persistent AOTInductor cache, the per-lane JUnit upload, and the phase-gating/atomic-.so-install changes in source/install/test_cc_local.sh. Conflict resolution against later commits that touched the same regions: - Preserve the actions/setup-python@v7 bump (deepmodeling#5898) and the actions/upload-artifact@v7 bump (deepmodeling#5900) -- unrelated dependency bumps that landed inside regions deepmodeling#5882 had modified. Only deepmodeling#5882's additions (the cache blocks and the JUnit upload step) are removed. - Preserve the native-spin / bridging DPA4 fixture generators (gen_dpa4_spin.py, gen_dpa4_zbl.py, gen_dpa4_spin_chgspin.py, gen_dpa4_spin_zbl.py) added by deepmodeling#5884; they are independent test fixtures that run under the restored single-lane flow. The matching entry deepmodeling#5884 added to _AOTI_COMPILE_MODULES is dropped along with that set, since the whole aoti_compile partition feature is being removed (the test still runs, just unpartitioned). Coding-Agent: opencode opencode-Version: 1.18.8 Model: ustc/glm-5.2 Reasoning-Effort: max
📝 WalkthroughWalkthroughThe pull request simplifies CUDA Python CI to one pytest run, removes the shared AOTInductor cache and marker declaration, adjusts conditional framework imports, and makes local C++ build, fixture generation, and ctest execution unconditional. ChangesCI test execution
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 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.
🧹 Nitpick comments (1)
source/install/test_cc_local.sh (1)
43-57: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNon-atomic overwrite of an already-loaded
.so.
shutil.copy2truncates and rewritesdstin place. IfSHARED_LIB_DIR/libdeepmd_op_pt.sois already mapped by another process (or a rerun on a dirty runner), the overwrite can fail or leave a torn library. Copy to a temp file in the same directory andos.replaceit, which is atomic and leaves existing mappings intact.🛠️ Suggested change
-import shutil, sys +import os, shutil, sys, tempfile @@ - shutil.copy2(str(so), str(dst)) + fd, tmp = tempfile.mkstemp(dir=str(SHARED_LIB_DIR), suffix=".so.tmp") + os.close(fd) + shutil.copy2(str(so), tmp) + os.replace(tmp, str(dst))🤖 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/install/test_cc_local.sh` around lines 43 - 57, Update the installation branch in the embedded Python script to copy the source library to a temporary file within SHARED_LIB_DIR, then atomically replace dst with os.replace. Import the required temporary-file and os utilities, preserve the existing same-file skip and missing-source behavior, and clean up any temporary file if replacement fails.
🤖 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/install/test_cc_local.sh`:
- Around line 43-57: Update the installation branch in the embedded Python
script to copy the source library to a temporary file within SHARED_LIB_DIR,
then atomically replace dst with os.replace. Import the required temporary-file
and os utilities, preserve the existing same-file skip and missing-source
behavior, and clean up any temporary file if replacement fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b1ddb14-c754-43dc-8f5a-6c105ced85c4
📒 Files selected for processing (4)
.github/workflows/test_cuda.ymlpyproject.tomlsource/install/test_cc_local.shsource/tests/conftest.py
💤 Files with no reviewable changes (1)
- source/tests/conftest.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5916 +/- ##
==========================================
- Coverage 79.07% 78.82% -0.25%
==========================================
Files 1067 1067
Lines 123841 123841
Branches 4527 4527
==========================================
- Hits 97926 97617 -309
- Misses 24297 24606 +309
Partials 1618 1618 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Reverts #5882 — "ci(cuda): overlap AOTI .pt2 compiles with GPU unit tests".
This undoes the CUDA CI orchestration that PR added:
test_python(-m aoti_compilevs-m "not aoti_compile").aoti_compilepytest marker machinery insource/tests/conftest.py(producer list,pytest_collection_modifyitemstagging, and the runtime drift guard).aoti_compilemarker registration inpyproject.toml.AOTInductorcompile cache (actions/cache) in both CUDA jobs.DP_CC_SKIP_BUILD/DP_CC_SKIP_CTEST) and atomic.soinstall changes insource/install/test_cc_local.sh.The serial
python -m pytest source/testsinvocation and the originaltest_cc_local.shflow are restored.Conflict resolution
#5882was followed by unrelated commits that touched the same regions. The revert preserves those later changes and only undoes#5882:actions/setup-python@v7(chore(deps): bump actions/setup-python from 6 to 7 #5898) andactions/upload-artifact@v7(chore(deps): bump actions/upload-artifact from 4 to 7 #5900) are kept. Only#5882's additions (theactions/cacheblocks and the JUnit upload step) are removed; the version bumps that landed in the same context stay.gen_dpa4_spin.py,gen_dpa4_zbl.py,gen_dpa4_spin_chgspin.py, andgen_dpa4_spin_zbl.pyare kept intest_cc_local.sh(re-indented to the restored single-lane flow). The matching entry#5884added to_AOTI_COMPILE_MODULESis dropped together with that set, since the wholeaoti_compilepartition feature is being removed — thetest_zbl_bridgingmodule still runs, just unpartitioned.docsdependency bumps inpyproject.toml(sphinx / myst) are untouched.Net result:
conftest.pymatches its pre-#5882state exactly;test_cuda.ymldiffers from pre-#5882only by the preservedsetup-python@v7bumps;test_cc_local.shdiffers only by the preserved DPA4 fixtures;pyproject.tomldiffers only by the preserveddocsdependency bumps.Coding agent: opencode
opencode version: 1.18.8
Model: ustc/glm-5.2
Reasoning effort: max
Summary by CodeRabbit
Tests
Chores