Skip to content

ci(cuda): overlap AOTI .pt2 compiles with GPU unit tests - #5882

Merged
wanghan-iapcm merged 8 commits into
deepmodeling:masterfrom
wanghan-iapcm:overlap-aoti-gpu-cuda-ci
Jul 23, 2026
Merged

ci(cuda): overlap AOTI .pt2 compiles with GPU unit tests#5882
wanghan-iapcm merged 8 commits into
deepmodeling:masterfrom
wanghan-iapcm:overlap-aoti-gpu-cuda-ci

Conversation

@wanghan-iapcm

@wanghan-iapcm wanghan-iapcm commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

What

The CUDA python job runs source/tests as one serial pytest process. The tests that freeze a .pt2 (torch.export + AOTInductor) are CPU-bound: inductor / g++ / ptxas generate code for minutes while the GPU sits idle. On the CI GPU the device measured ~98% idle (peak <250 MiB) for the entire duration of that test group, so those minutes are pure serialization in front of the GPU-bound tests.

Since #5896 moved the C++ build + ctest + LAMMPS to a separate test_cc job, this PR's scope is the python side plus caching:

  • Two pytest lanes in test_python, concurrently on the one GPU: lane 1 = the CPU-bound .pt2-freezing tests (-m aoti_compile), niced and capped to half the cores so the GPU lane keeps CPU priority; lane 2 = the GPU-bound remainder (-m "not aoti_compile"). Wall-clock becomes max(lane 1, lane 2) instead of their sum.
  • Persistent AOTInductor compile cache (actions/cache) restored in BOTH jobs: the test_cc fixture builds (gen_*.py) drive the same inductor backend on the same architectures, so they reuse kernels the python lanes (or previous runs) already compiled, and unchanged models never recompile across runs.
  • Per-lane JUnit reports uploaded as artifacts for lane-balance monitoring.

How the partition stays correct

The aoti_compile marker is auto-applied in source/tests/conftest.py to any test whose module references a .pt2 freeze entry point (deserialize_to_file / _trace_and_export / aoti_compile_and_package). No hand-maintained file list — new compiling tests are partitioned automatically. The two selections are complementary and exhaustive (verified locally: A + B = total, disjoint). A fail-fast guard aborts if an AOT compile happens in an untagged test.

The custom-op .so install in test_cc_local.sh is atomic (copy to a temp file + os.replace) rather than in place: overwriting the destination inode while another process has it mmap'd corrupts its live code pages and SIGSEGVs it. os.replace swaps the directory entry to a new inode, so live mappings stay intact.

Measured on CUDA CI

Lane wall-clocks from the green run at sha 86128ef7 (measured under the pre-#5896 single-job layout; the pytest lanes are unchanged by the rebase):

  • Lane 1 (aoti_compile): 4063 s (~68 min)
  • Lane 2 (-m "not aoti_compile"): 8158 s (~2h16m)

Serial python pytest is by construction the sum, 12221 s (~3h24m); the overlap runs them in max(4063, 8158) = 8158 s (~2h16m) — the test_python job's pytest step drops by ~68 min (~33%), with the entire compile stream hidden behind the GPU lane. The inductor cache additionally removes recompilation of unchanged models from both jobs on subsequent runs.

Notes / limitations

  • Overlap quality scales with the runner's core count (nice is a no-op unless the CPU saturates); the numbers above are for the CI runner.
  • Whole-file granularity: a compiling module's few non-compiling tests also run in the compile lane (harmless).
  • The compile lane tolerates pytest exit 5 ("no tests collected") so the split is safe on branches without .pt2-freezing tests.
  • The two jobs share one cache key per sha; if both try to save it, the second save is skipped with a warning (caches are immutable), which is harmless.
  • Generic CI orchestration, independent of any feature branch.

Summary by CodeRabbit

  • Tests / CI
    • Improved CUDA CI by running AOT-related and non-AOT GPU test groups concurrently, with separate JUnit reports and “no tests collected” treated as success.
    • Persisted the compilation cache across workflow runs to speed up repeated runs.
    • Added formal AOT compilation test tagging via a new pytest marker and a fail-fast guard if AOT compilation occurs without the expected tag.
  • Build & Test Scripts
    • Updated local C++ test runner to support skipping build vs ctest phases, improve runtime library setup, handle leak-sanitizer generator behavior, and generate fixtures more reliably (clean stale outputs and run generation in parallel).

The CUDA job ran source/tests as one serial process, so the .pt2-freezing
tests (torch.export + AOTInductor) held the step for minutes while
inductor/g++/ptxas generated code on the CPU -- the GPU measured ~98% idle
(<250 MiB) during those compiles. Split the pytest step into two groups that
run concurrently on the one GPU: a CPU-bound compile group (-m aoti_compile,
niced so the GPU group keeps CPU priority) and the GPU-bound remainder
(-m "not aoti_compile"), so the compile CPU-time overlaps the GPU unit tests.
The compile group's GPU footprint is negligible, so the two coexist on one
device without contention; wall-clock becomes max(compile, gpu) rather than
their sum.

The aoti_compile marker is auto-applied in source/tests/conftest.py to any
test whose module references a .pt2 freeze entry point, so the partition
stays correct as tests are added without a hand-maintained file list.
@wanghan-iapcm
wanghan-iapcm requested a review from njzjz July 20, 2026 00:15
@wanghan-iapcm wanghan-iapcm added the Test CUDA Trigger test CUDA workflow label Jul 20, 2026
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

CUDA pytest collection assigns an explicit aoti_compile marker and detects unmarked AOTInductor compilations. CUDA CI concurrently runs AOTI and non-AOTI tests, caches compilation output, prepares C++ fixtures, uploads both JUnit reports, and runs C++ tests without rebuilding.

Changes

AOTI CUDA test partitioning

Layer / File(s) Summary
Classify AOTI compilation tests
source/tests/conftest.py, pyproject.toml
An explicit module allowlist is normalized against collected paths, and matching tests receive the registered aoti_compile marker.
Detect marker drift
source/tests/conftest.py
Compilation entry points are wrapped to associate unmarked .pt2 compilations with test items, fail otherwise-successful sessions, and report offending nodeids.
Prepare C++ fixtures
source/install/test_cc_local.sh
Build and ctest phases become independently gated; runtime library paths, sanitizer handling, stale artifact cleanup, and parallel fixture generation are updated.
Run partitioned CUDA tests
.github/workflows/test_cuda.yml
CUDA CI caches the Inductor directory, overlaps C++ preparation with concurrent AOTI and non-AOTI pytest groups, uploads both JUnit reports, and runs ctest after fixture preparation without rebuilding.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested labels: CUDA, enhancement, C

Sequence Diagram(s)

sequenceDiagram
  participant CUDA_CI
  participant AOTI_Pytest
  participant GPU_Pytest
  participant CPP_Preparation
  participant JUnit_Artifact
  CUDA_CI->>AOTI_Pytest: run aoti_compile tests
  CUDA_CI->>GPU_Pytest: run non-aoti_compile tests
  AOTI_Pytest->>CPP_Preparation: convert models and prepare fixtures
  AOTI_Pytest-->>CUDA_CI: return test and preparation status
  GPU_Pytest-->>CUDA_CI: return test status
  CUDA_CI->>JUnit_Artifact: upload both JUnit reports
  CUDA_CI->>CPP_Preparation: run ctest without rebuilding
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: overlapping AOTI .pt2 compilation with GPU unit tests in CUDA CI.
✨ 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.

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

Actionable comments posted: 1

🤖 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/tests/conftest.py`:
- Around line 36-39: Update the file-reading exception handler in the conftest
collection logic to catch UnicodeDecodeError alongside OSError, preserving the
existing fallback of assigning an empty string to src for unreadable or
non-UTF-8 files.
🪄 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: abaf51b2-a9cb-488f-879f-939eaa65a1a4

📥 Commits

Reviewing files that changed from the base of the PR and between 20c7b75 and a9dab3a.

📒 Files selected for processing (3)
  • .github/workflows/test_cuda.yml
  • pyproject.toml
  • source/tests/conftest.py

Comment thread source/tests/conftest.py Outdated
@njzjz-bot

Copy link
Copy Markdown
Contributor

The overlap direction is worth exploring, but I don't think the current automatic aoti_compile marker is reliable enough to use as the scheduling boundary.

The marker is inferred by grepping source text, which is not equivalent to “this test actually triggers a .pt2 compile”:

  • It appears to miss real freeze(...)-driven .pt2 cases such as test_dp_freeze.py, test_change_bias.py, and test_finetune.py (the first two even document ~82 s per .pt2), because they do not contain the scanned strings directly.
  • It can over-classify whole files: some hits only produce .pth, mock aoti_compile_and_package, or test .pte; moving a large module such as test_deep_eval.py also moves many cases that are not necessarily CPU-only compilation work.
  • The premise that this lane is CPU-only is not fully established: the .pt2 path in serialization.py can use a CUDA target, run autotuning on that device, and move the exported program to CUDA before AOTInductor. The resulting tests also run GPU inference.

Could we instead mark the concrete .pt2-producing tests/fixtures/classes explicitly (including the freeze(...) cases), then benchmark both lanes on a CUDA runner? I would want a few repeated runs with --durations, GPU utilization/VRAM, and end-to-end wall time before relying on concurrency. If the overlap remains beneficial, adding a CPU/compiler-thread constraint to the compile lane would also help reduce GPU contention and timeout variance.

Keeping separate JUnit reports is a good part of the implementation.

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

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.61%. Comparing base (5f7b747) to head (5c6aee2).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5882      +/-   ##
==========================================
- Coverage   78.87%   78.61%   -0.26%     
==========================================
  Files        1054     1054              
  Lines      121770   121770              
  Branches     4410     4412       +2     
==========================================
- Hits        96041    95728     -313     
- Misses      24163    24469     +306     
- Partials     1566     1573       +7     

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

Han Wang added 2 commits July 20, 2026 14:41
Replace the source-grep aoti_compile auto-marker with an explicit curated list
of the modules that actually freeze a .pt2, plus an always-on runtime guard.
The grep both missed freeze(...)-driven compiles (test_dp_freeze, test_finetune,
test_multitask, test_dp_test, test_change_bias, test_models -- none name the
low-level entry points) and over-tagged files that only mock the compiler or
emit .pth/.pte. The producer list was derived by running the suite under a
detector that wraps the real compile entry points (aoti_compile_and_package,
_trace_and_export).

The guard wraps those same entry points every run and fails the session, with
the offending nodeids, if any test freezes a .pt2 without the aoti_compile
marker -- so the list cannot silently drift, and a compile can never escape into
the GPU lane unnoticed. It runs in the CPU test_python CI too, so drift is
caught cheaply rather than only in the expensive CUDA job.
- echo each lane's wall-clock ([lane aoti_compile] / [lane gpu]) so T_A and T_B
  (vs the ~3h46m serial baseline) appear directly in the log
- cap the compile lane at TORCHINDUCTOR_COMPILE_THREADS=nproc/2 (plus nice) so
  the GPU lane keeps CPU for its host-side dispatch
- upload junit-aoti.xml / junit-gpu.xml as artifacts for post-run inspection
- share one AOTInductor compile cache across all steps via a job-level
  TORCHINDUCTOR_CACHE_DIR, so the C++ fixture builds (gen_*.py in
  test_cc_local.sh) reuse the kernels the Python lane compiled, and persist it
  across runs with actions/cache (10 GB repo-cache limit noted inline)

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

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 @.github/workflows/test_cuda.yml:
- Line 36: Update the actions/checkout step in the CUDA test workflow to
explicitly disable credential persistence by setting persist-credentials to
false, while leaving the existing checkout behavior unchanged.
- Around line 97-108: Update both pytest lane subshells in the parallel test
execution block to initialize rc=0 before running pytest and append || rc=$? to
each pytest command. Preserve the existing wall-time echo and exit $rc reporting
so failures are captured without set -e bypassing the reporting.

In `@source/tests/conftest.py`:
- Around line 118-123: Update the pytest_runtest_protocol hookwrapper so the
yield executes inside a try...finally block, and move the _current_item["item"]
reset into the finally clause to guarantee cleanup when the wrapped hook raises.
🪄 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: 2d10c758-b81a-495e-9f64-75a7057c1315

📥 Commits

Reviewing files that changed from the base of the PR and between a9dab3a and b4a7996.

📒 Files selected for processing (2)
  • .github/workflows/test_cuda.yml
  • source/tests/conftest.py

Comment thread .github/workflows/test_cuda.yml
Comment thread .github/workflows/test_cuda.yml Outdated
Comment thread source/tests/conftest.py
@wanghan-iapcm

Copy link
Copy Markdown
Collaborator Author

Thanks — you're right that a source grep is the wrong basis for the scheduling boundary, on both counts. Replaced it (f660993, b4a7996).

The aoti_compile set is now an explicit curated list of the modules that actually freeze a .pt2, derived by running the suite under a detector that wraps the real compile entry points (aoti_compile_and_package, _trace_and_export) instead of scanning text:

  • Missed freeze(...) casestest_dp_freeze, test_finetune, test_multitask, test_dp_test, test_change_bias, infer/test_models are now in the compile lane (none name the low-level entry points, so the grep skipped them all).
  • Over-tagging — the detector confirmed 8 grep-candidate files (test_dpa1, test_dpa4, test_dpa1_triton, test_get_model_dpa4, test_dpa2_graph_lower, test_sezm_model, test_entrypoint, test_descriptor_dpa1_triton) never actually compile (mock / .pth / .pte), so they stay in the GPU lane.

An always-on guard wraps the same entry points every run and fails the session (with the offending nodeids) if any test freezes a .pt2 without the marker, so the list can't silently drift; it runs in the CPU test_python CI too, so a missed producer is caught cheaply rather than only in the CUDA job.

Other points: compile-thread constraint added (TORCHINDUCTOR_COMPILE_THREADS=nproc/2 + nice); each lane now echoes its wall-clock and uploads its own JUnit report to compare T_A/T_B against the ~3h46m serial baseline. The current labelled CUDA run is on the pre-fix commit (grep split, so a lower bound); I'll re-trigger on the fixed markers and post the lane timings + GPU util/VRAM before relying on the concurrency beyond what those numbers support. Agreed the lane isn't categorically CPU-only — the ~98% idle / <250 MiB is empirical and the benchmark will confirm on the fixed split.

Also made the inductor compile cache explicit (shared TORCHINDUCTOR_CACHE_DIR + actions/cache) so the C++ fixture builds reuse the Python lane's kernels. (The UnicodeDecodeError note is moot now — the conftest no longer reads test source.)

- CUDA lanes: init rc=0 and `|| rc=$?` so the step's default `set -e` cannot
  skip the per-lane wall-clock echo when a lane fails (that timing is exactly
  what we want on a failure).
- conftest guard: wrap the hookwrapper yield in try/finally so _current_item is
  reset even if the wrapped hook raises, avoiding misattributed compiles in
  teardown hooks.
@wanghan-iapcm wanghan-iapcm added Test CUDA Trigger test CUDA workflow and removed Test CUDA Trigger test CUDA workflow labels Jul 20, 2026
@github-actions github-actions Bot removed the Test CUDA Trigger test CUDA workflow label Jul 20, 2026

@njzjz-bot njzjz-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.

Thanks for replacing the source-grep classifier with an explicit producer list and a runtime drift guard. That fixes the main issue in the first revision. I still think this needs changes before merge.

  1. The scheduling boundary remains module-level, while the safety/performance premise is test-level. Every test in each _AOTI_COMPILE_MODULES file is sent to the aoti_compile process, including tests that do GPU inference or other GPU-heavy work but do not freeze a .pt2. Both pytest processes are pinned to CUDA_VISIBLE_DEVICES=0, so the implementation can still create real GPU/VRAM contention between two independent CUDA processes. The marker/guard establishes that .pt2 producers do not escape the compile lane; it does not establish that the compile lane has negligible GPU use.

    Please either partition at concrete test/fixture/class granularity, or provide repeated CUDA-run evidence for the current explicit partition: per-lane --durations, GPU-utilization/VRAM traces, end-to-end wall time, and a few clean repeat runs. I would not merge based only on the earlier aggregate 98%-idle observation.

  2. Make the persistent Inductor cache environment-safe. restore-keys: inductor-cuda- can restore artifacts created under a different Torch/Inductor, CUDA/toolchain, or GPU architecture. Please key the cache at least on the dependency/workflow lock inputs and the relevant Torch/CUDA environment (and GPU capability if runners may vary), rather than restoring from every historical CUDA cache. A miss is fine; cache correctness and reproducibility matter more than a hit.

  3. Minor security hygiene: set persist-credentials: false on actions/checkout, since this workflow executes PR code/tests and does not need to push.

The separate JUnit reports and the per-lane exit-code/timing handling look good.

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

The C++ portion ran serially after the Python tests: cmake --build plus the
gen_*.py .pt2 fixtures are ~1h of CPU-bound work with the GPU idle (measured:
56m gen + 7.5m build + 5m convert-models), followed by ~39m of GPU tests
(ctest + LAMMPS). The Python GPU lane meanwhile has a ~1h23m idle-CPU tail
(compile lane ends ~1h13m, GPU lane runs to ~2h36m).

Run the C++ CPU-prep as a background step (GitHub Actions parallel steps) so
it overlaps the Python GPU lane, with a `- wait: cc-prep` barrier before the
ctest step consumes the fixtures. The prep is niced -19 so both Python lanes
keep CPU priority and it only fills idle cores. test_cc_local.sh gains
DP_CC_SKIP_CTEST / DP_CC_SKIP_BUILD phase gates (default runs both, so
test_cc.yml is unaffected); convert-models.sh moves into the prep step.

Projected: hides ~1h14m of GPU-idle compile, whole CUDA job ~4h57m -> ~3h43m.
@wanghan-iapcm wanghan-iapcm added the Test CUDA Trigger test CUDA workflow label Jul 21, 2026
@github-actions github-actions Bot removed the Test CUDA Trigger test CUDA workflow label Jul 21, 2026

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/test_cuda.yml (1)

23-47: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Shared AOTInductor cache persists across all runs/PRs — cache-poisoning risk on a privileged self-hosted runner.

The restore-keys: inductor-cuda- prefix means any run gated by the Test CUDA label (or workflow_dispatch/merge_group) can read/write the same on-disk compile cache used by all future runs. Inductor's FX graph cache is content-addressed (keyed by graph hash), so a labeled PR with attacker-influenced code could deliberately populate cache entries for the exact hash keys of the repo's fixed test fixtures (gen_dpa1, gen_dpa2, etc.) with malicious compiled artifacts; those entries would then be silently reused (and executed) by subsequent, unrelated runs on the same self-hosted GPU runner — without ever re-triggering a recompile.

Since the label-gate already requires a maintainer's explicit action per PR, this isn't trivially exploitable, but the cache's blast radius extends indefinitely into the future once poisoned, well past the single approved run. Consider scoping the cache key to a trusted ref (e.g. only restore from main's key on pull_request, and only write on merge_group/push to protected branches) rather than a global prefix shared by every triggering event.

🤖 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 @.github/workflows/test_cuda.yml around lines 23 - 47, Restrict the
AOTInductor cache in the workflow’s “Cache AOTInductor compile artifacts” step
so untrusted pull-request and manual runs cannot restore or persist artifacts
across unrelated refs. Use trusted protected-branch or merge-group cache keys
and restore sources, and disable cache save or use isolated keys for
pull-request executions; remove the global inductor-cuda- restore prefix while
preserving cache reuse for trusted runs.
🧹 Nitpick comments (1)
source/install/test_cc_local.sh (1)

96-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Fragile PID variable reuse across gen-script groups.

PID9 is assigned for gen_chg_spin.py (line 115) and later reassigned for gen_dpa4.py (line 122). This is currently safe only because the earlier wait $PID9 (line 119) happens before the reassignment — but it's an easy footgun if someone reorders/adds scripts later and forgets a wait, silently dropping a background job from set -e failure detection.

Consider tracking PIDs in an array instead of ad hoc PID1..PID10 variables, to remove the reuse risk while keeping the same staggered-group behavior.

♻️ Suggested refactor (per group)
-		env ${_GEN_ENV} python ${INFER_SCRIPT_PATH}/gen_dpa3.py &
-		PID4=$!
-		env ${_GEN_ENV} python ${INFER_SCRIPT_PATH}/gen_fparam_aparam.py &
-		PID5=$!
-		env ${_GEN_ENV} python ${INFER_SCRIPT_PATH}/gen_model_devi.py &
-		PID6=$!
-		env ${_GEN_ENV} python ${INFER_SCRIPT_PATH}/gen_chg_spin.py &
-		PID9=$!
-		wait $PID4
-		wait $PID5
-		wait $PID6
-		wait $PID9
+		pids=()
+		for script in gen_dpa3 gen_fparam_aparam gen_model_devi gen_chg_spin; do
+			env ${_GEN_ENV} python "${INFER_SCRIPT_PATH}/${script}.py" &
+			pids+=($!)
+		done
+		for pid in "${pids[@]}"; do wait "$pid"; done
🤖 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 96 - 133, Refactor the
background-process tracking in the generation block to use a PID array per
staggered script group instead of ad hoc PID1–PID10 variables. Update the
launches and waits for each group while preserving the existing grouping,
execution order, and failure propagation under set -e; ensure every started
gen_*.py process is waited on exactly once.
🤖 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.

Outside diff comments:
In @.github/workflows/test_cuda.yml:
- Around line 23-47: Restrict the AOTInductor cache in the workflow’s “Cache
AOTInductor compile artifacts” step so untrusted pull-request and manual runs
cannot restore or persist artifacts across unrelated refs. Use trusted
protected-branch or merge-group cache keys and restore sources, and disable
cache save or use isolated keys for pull-request executions; remove the global
inductor-cuda- restore prefix while preserving cache reuse for trusted runs.

---

Nitpick comments:
In `@source/install/test_cc_local.sh`:
- Around line 96-133: Refactor the background-process tracking in the generation
block to use a PID array per staggered script group instead of ad hoc PID1–PID10
variables. Update the launches and waits for each group while preserving the
existing grouping, execution order, and failure propagation under set -e; ensure
every started gen_*.py process is waited on exactly once.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3cee2ab9-6e8f-4bd9-9ea1-b2d4aaff1f1d

📥 Commits

Reviewing files that changed from the base of the PR and between 6f8d76b and 70b5175.

📒 Files selected for processing (2)
  • .github/workflows/test_cuda.yml
  • source/install/test_cc_local.sh

@njzjz-bot njzjz-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.

Thanks for the explicit producer list and runtime drift guard; that resolves the earlier source-grep classification issue. The new C++-prep overlap still needs changes before merge.

  1. The background C++ prep is not guaranteed CPU-only. test_cc_local.sh generates multiple .pt2 fixtures while inheriting the CUDA job environment. Those generation paths deserialize/export models and can run CUDA-targeted autotuning/AOTInductor work. At the same time both Python lanes are explicitly on GPU 0. This can create three independent CUDA processes competing for GPU/VRAM; nice only changes CPU scheduling. Please either serialize the GPU-active fixture generation (or use a verified CPU-only path), or provide repeated current-head measurements proving stability and benefit: GPU-utilization/VRAM traces, per-lane durations, end-to-end wall time, and cold/warm-cache runs.

  2. The persistent Inductor cache is neither environment-safe nor appropriately scoped. The key is only inductor-cuda-${{ github.sha }} and the inductor-cuda- restore prefix can reuse executable compilation artifacts across Torch/Inductor, CUDA/toolkit/driver, Python, dependency, or GPU-architecture changes. The broad prefix also permits labeled PR/manual-run artifacts to become candidates for subsequent unrelated runs. Please remove the global fallback and use trusted-ref cache policy; make any reusable key include the relevant environment/dependency inputs. A safe cache miss is preferable to an incorrect or untrusted artifact hit.

  3. Please set persist-credentials: false on actions/checkout@v7. This workflow executes PR code but does not need authenticated Git operations after checkout.

The current CUDA job is still pending, so it cannot yet validate the new three-way concurrency. The phase gates and wait: cc-prep barrier themselves look correct.

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

@njzjz njzjz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

…y contention

The previous layout ran the C++ build + gen_*.py fixture compiles as a third
`background:` step concurrent with BOTH Python lanes. That 3-way overlap (two
pytest lanes + the C++ build, all sharing one TORCHINDUCTOR_CACHE_DIR)
SIGSEGV'd both Python lanes; the identical suite passed on the prior sha that
had only the two Python lanes.

Collapse it back to two workstreams. Lane 1 now runs the aoti_compile pytest
group and THEN, sequentially, the C++ build + fixture generation -- both are
CPU-bound with the GPU idle, so serializing them keeps only one inductor
compiler writing the cache at a time and lets cc-prep reuse the kernels the
aoti tests just built. Lane 2 keeps the GPU-bound bulk. ctest + LAMMPS run once
both lanes join. At any instant only two heavy streams run, matching the
green baseline, and the whole CPU stream stays hidden under lane 2.

Drops the `background:`/`- wait:` steps; test_cc_local.sh's DP_CC_SKIP_CTEST /
DP_CC_SKIP_BUILD gating is unchanged.
@wanghan-iapcm wanghan-iapcm added the Test CUDA Trigger test CUDA workflow label Jul 21, 2026
@github-actions github-actions Bot removed the Test CUDA Trigger test CUDA workflow label Jul 21, 2026
…rrent test lane

The C++ prep copies the freshly built libdeepmd_op_pt.so into the shared Python
site-packages (SHARED_LIB_DIR) so gen_*.py can import deepmd.pt. shutil.copy2
rewrites the destination inode in place; when that .so is already dlopen'd by a
concurrently running Python test process -- as in the CUDA CI overlap where the
GPU test lane runs alongside this build -- overwriting its bytes corrupts the
live mmap'd code pages and SIGSEGVs the test process (deepmodeling#5882). Two overlap runs
crashed both differently: the segfault landed immediately after "Installed ...
libdeepmd_op_pt.so", on whatever deepmd.pt test happened to be executing.

Install atomically instead: copy to a temp file in the same directory, then
os.replace() onto the destination. The rename swaps the directory entry to a new
inode, so any process holding the old mapping keeps its intact (refcounted)
inode and new dlopens pick up the new file. No live mapping is ever mutated.
@wanghan-iapcm wanghan-iapcm added the Test CUDA Trigger test CUDA workflow label Jul 22, 2026
@github-actions github-actions Bot removed the Test CUDA Trigger test CUDA workflow label Jul 22, 2026
@njzjz
njzjz added this pull request to the merge queue Jul 22, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 22, 2026
@njzjz
njzjz added this pull request to the merge queue Jul 22, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 22, 2026
Master's deepmodeling#5896 split the CUDA workflow into separate test_python and
test_cc jobs on separate runners, superseding this PR's lane-1 cc-prep
tail. Resolution keeps deepmodeling#5896's two-job structure and re-applies this
PR's remaining pieces: the aoti_compile/gpu two-lane pytest overlap
inside test_python (hides the CPU-bound .pt2 compile tests behind the
GPU lane), the per-lane JUnit reports, and the persistent AOTInductor
compile cache -- now restored in BOTH jobs, since the test_cc fixture
builds (gen_*.py) drive the same inductor backend and reuse the cached
kernels.
@wanghan-iapcm
wanghan-iapcm enabled auto-merge July 22, 2026 23:52
@wanghan-iapcm
wanghan-iapcm added this pull request to the merge queue Jul 23, 2026
Merged via the queue into deepmodeling:master with commit f62c3c9 Jul 23, 2026
58 checks passed
@wanghan-iapcm
wanghan-iapcm deleted the overlap-aoti-gpu-cuda-ci branch July 23, 2026 04:22
hanaol pushed a commit to hanaol/deepmd-kit that referenced this pull request Jul 28, 2026
) (deepmodeling#5916)

Reverts deepmodeling#5882 — "ci(cuda): overlap AOTI .pt2 compiles with GPU unit
tests".

This undoes the CUDA CI orchestration that PR added:

- The two-lane pytest split in `test_python` (`-m aoti_compile` vs `-m
"not aoti_compile"`).
- The `aoti_compile` pytest marker machinery in
`source/tests/conftest.py` (producer list,
`pytest_collection_modifyitems` tagging, and the runtime drift guard).
- The `aoti_compile` marker registration in `pyproject.toml`.
- The persistent `AOTInductor` compile cache (`actions/cache`) in both
CUDA jobs.
- The per-lane JUnit report upload.
- The phase-gating (`DP_CC_SKIP_BUILD` / `DP_CC_SKIP_CTEST`) and atomic
`.so` install changes in `source/install/test_cc_local.sh`.

The serial `python -m pytest source/tests` invocation and the original
`test_cc_local.sh` flow are restored.

## Conflict resolution

`deepmodeling#5882` was followed by unrelated commits that touched the same regions.
The revert preserves those later changes and only undoes `deepmodeling#5882`:

- **Dependabot bumps preserved:** `actions/setup-python@v7` (deepmodeling#5898) and
`actions/upload-artifact@v7` (deepmodeling#5900) are kept. Only `deepmodeling#5882`'s additions
(the `actions/cache` blocks and the JUnit upload step) are removed; the
version bumps that landed in the same context stay.
- **DPA4 test fixtures preserved (deepmodeling#5884):** the native-spin / bridging
fixture generators `gen_dpa4_spin.py`, `gen_dpa4_zbl.py`,
`gen_dpa4_spin_chgspin.py`, and `gen_dpa4_spin_zbl.py` are kept in
`test_cc_local.sh` (re-indented to the restored single-lane flow). The
matching entry `deepmodeling#5884` added to `_AOTI_COMPILE_MODULES` is dropped
together with that set, since the whole `aoti_compile` partition feature
is being removed — the `test_zbl_bridging` module still runs, just
unpartitioned.
- Unrelated `docs` dependency bumps in `pyproject.toml` (sphinx / myst)
are untouched.

Net result: `conftest.py` matches its pre-`deepmodeling#5882` state exactly;
`test_cuda.yml` differs from pre-`deepmodeling#5882` only by the preserved
`setup-python@v7` bumps; `test_cc_local.sh` differs only by the
preserved DPA4 fixtures; `pyproject.toml` differs only by the preserved
`docs` dependency bumps.

---

Coding agent: opencode
opencode version: 1.18.8
Model: ustc/glm-5.2
Reasoning effort: max


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Tests**
* Streamlined CUDA test execution with a single comprehensive test run
and simplified environment settings.
  * Improved test setup compatibility for Paddle and TensorFlow imports.
  * Updated test marker configuration to support the current test suite.

* **Chores**
* Improved local C++ build, installation, fixture generation, and
validation workflows.
  * C++ tests now run automatically after successful builds.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Co-authored-by: njzjz-bot <njzjz.bot@gmail.com>
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