Uh oh!
There was an error while loading. Please reload this page.
build: make Cython/extension artifacts CUDA-major aware - #2472
Open
rparolin wants to merge 5 commits into
Open
Conversation
Moving a checkout between the cu12 and cu13 pixi environments failed at compile time, pointing at code that is perfectly fine. Two independent caches are to blame, and neither tool notices the configuration changed: - Cython's up-to-date check hashes the .pyx and its cimport dependencies, but not compile_time_env. cuda.core feeds CUDA_CORE_BUILD_MAJOR through compile_time_env, so a cu13 -> cu12 switch silently reuses the cu13 generated C++. Reproduced on main: the cu12 build dies on 'CUdevWorkqueueConfigScope was not declared' in a build/cython/*.cpp generated under CUDA 13. - setuptools' build_ext compares source mtimes against the output .so. In an editable install that .so lives in the source tree under a name keyed by the Python ABI tag alone -- there is nowhere to record the CUDA major. So on a cu12 -> cu13 -> cu12 round trip the final build finds an older generated source next to a newer .so and skips the rebuild entirely. Both build backends now compute a build identity (CUDA major, plus the debug and coverage flags, which likewise change the generated C++ that neither tool tracks). Generated sources go to build/cython/<identity>, and build/.build-identity records the last completed build so setup.py can force build_ext when the configuration changes. Python version and platform stay out of the identity: setuptools already encodes them in its own build/lib.* and build/temp.* names. CUDA_PYTHON_COVERAGE keeps generating in-tree (build_dir=".") so it can still package the generated sources; it only contributes to the identity. Migration is self-healing. An existing unkeyed build/cython is orphaned and ignored; the first build after this change regenerates into a keyed directory and overwrites the in-tree extensions. Verified end to end with pixi on linux-64: cu13 -> cu12 -> cu13 for cuda_core. Before, the cu12 leg failed to compile; after, all three legs succeed, each major keeps its own build/cython/cu1X-debug directory, and the second cu13 extension is byte-identical to the first (md5 ae4090a4f66ab9cee67b5b2f64b43781), proving it was recompiled rather than left as the cu12 artifact. The new CI job covers cuda_core only. cuda_bindings cannot be source-built in its cu12 environment at all -- the 13.x sources reference CUDA 13-only symbols (CUatomicOperation, nvrtcBundledHeadersInfo, CUstreamCigCaptureParams), so even freshly generated cu12 sources fail against CUDA 12 headers. That is a pre-existing problem, unrelated to artifact staleness; the identical identity logic in cuda_bindings is covered by unit tests instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two _determine_cuda_major_version implementations are annotated "keep in sync"; a missing cuda.h surfaced as a bare FileNotFoundError in cuda_bindings instead of the RuntimeError naming CUDA_PATH/CUDA_HOME. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Cuts the change to the two mechanisms that are actually load-bearing for the cu12/cu13 problem, and drops everything that was speculative. Removed the debug and coverage identity axes. The stated justification for debug -- that gdb_debug changes the cythonize output -- is wrong: with gdb_debug=True the generated .c is byte-identical, and Cython only writes a side cython_debug/ directory. Keying by it produced a duplicate directory of identical sources and re-ran cythonize on every editable/wheel switch for no benefit. Coverage does change the generated C (linetrace), but toggling it is a separate defect from the one this PR is about. Removed the cuda_bindings half entirely. It has no compile_time_env, so its generated C is CUDA-major independent, and it cannot be source-built against CUDA 12 at all today -- the scenario the code guarded against is unreachable. That also drops a second _determine_cuda_major_version, a duplicated 50-line block, and a test file. What remains: cuda_core generates into build/cython/cu<major>, and build/.build-cuda-major records the last completed build so setup.py forces build_ext when the major changes. The stamp is a bare major rather than a composite identity string, so the "no build ran" guard and its test are gone too. Re-verified cu13 -> cu12 -> cu13 on linux-64: all three legs build, both build/cython/cu12 and build/cython/cu13 exist, and the second cu13 extension is byte-identical to the first (md5 1c53ab83a7201d54576175c912d03e93). 18 unit tests pass; pre-commit clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author's call. The convention stays in CLAUDE.md and on the ~30 tests that already carry it; only the four added by this PR are affected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rparolin
marked this pull request as ready for review
July 31, 2026 19:20
|
mdboom
requested changes
Aug 4, 2026
| # Records the CUDA major of the last completed build, so setup.py can force | ||
| # build_ext when it changes. Written by record_build_major(). | ||
| _BUILD_MAJOR_STAMP = os.path.join("build", ".build-cuda-major") |
Contributor
There was a problem hiding this comment.
This will be relative to pwd, and you can build a project from anywhere. It should instead be something like:
Path(__file__).parent / "build" / ".build-cuda-major"
(and we should use pathlib.Path for all new code, not the soft-deprecated os.path APIs).
| except FileNotFoundError: | ||
| previous = None | ||
| if previous is not None and previous != cuda_major: |
Contributor
There was a problem hiding this comment.
IIUC, if previous is None, we failed to determine whether we need a full rebuild, so we should do a full rebuild.
Suggested change
| ifpreviousisnotNoneandprevious!=cuda_major: | |
| ifprevious!=cuda_major: |
| setup.py calls this after build_ext succeeds, so that a build which failed | ||
| partway through does not claim outputs it never produced. | ||
| """ | ||
| os.makedirs(os.path.dirname(_BUILD_MAJOR_STAMP), exist_ok=True) |
mdboom
reviewed
Aug 4, 2026
Comment on lines
+126
to
+168
| build-identity-roundtrip: | ||
| name: "cu13 -> cu12 -> cu13 round trip (linux-64, CPU)" | ||
| if: ${{ github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' }} | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 60 | ||
| steps: | ||
| - name: Checkout ${{ github.event.repository.name }} | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| # Full history + tags so setuptools-scm derives the real (13.x) | ||
| # package version; a shallow checkout yields 0.1.dev1, which trips | ||
| # cuda.core's "cuda.bindings 12.x or 13.x must be installed" guard. | ||
| fetch-depth: 0 | ||
| - name: Setup pixi | ||
| # Pinned to a commit SHA; install logic lives in the action and is | ||
| # auditable/pinned (vs. a curl|bash of an unverified installer). | ||
| uses: prefix-dev/setup-pixi@5185adfbffb4bd703da3010310260805d89ebb11 # v0.9.6 | ||
| with: | ||
| pixi-version: ${{ env.PIXI_VERSION }} | ||
| run-install: false | ||
| - name: Build cu13, then cu12, then cu13 again in one checkout | ||
| run: | | ||
| for cuda_env in cu13 cu12 cu13; do | ||
| echo "::group::${cuda_env}" | ||
| pixi run --manifest-path cuda_core -e "${cuda_env}" \ | ||
| python -c "import cuda.core; print('core import OK')" | ||
| echo "::endgroup::" | ||
| done | ||
| # The last build was cu13, and each major must have kept its own | ||
| # generated sources rather than overwriting the other's. | ||
| stamp=$(cat cuda_core/build/.build-cuda-major) | ||
| if [ "${stamp}" != "13" ]; then | ||
| echo "::error::build stamp is '${stamp}', expected 13" | ||
| exit 1 | ||
| fi | ||
| for major in cu12 cu13; do | ||
| if [ ! -d "cuda_core/build/cython/${major}" ]; then | ||
| echo "::error::no ${major} generated-source directory" | ||
| exit 1 | ||
| fi | ||
| done |
Contributor
There was a problem hiding this comment.
Maybe to address later -- but I worry about the time of this test for sort of a niche problem. Is there a way we could "simulate" a build doing the wrong thing rather than doing a full build?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Moving a checkout between the
cu12andcu13pixi environments fails at compile time, pointing at code that is perfectly fine. Reproduced onmainat 755cf61 —cu13build, thencu12:Nothing tells you the real cause: leftover generated C++ from the previous CUDA major.
This is the configuration-aware approach @kkraus14 asked for in #2443, in place of a destructive cleaner.
Why it happens
Two caches, neither of which notices the CUDA major changed.
Cython does not hash
compile_time_env.cuda.corefeedsCUDA_CORE_BUILD_MAJORthrough it, so acu13 -> cu12switch silently reuses thecu13generated C++. That is the error above.setuptools'
build_extcompares source mtimes against the output.so. In an editable install that.solives in the source tree under a name keyed by the Python ABI tag alone — there is nowhere to record the CUDA major. On acu12 -> cu13 -> cu12round trip the final build finds an older generated source next to a newer.soand skips the rebuild entirely.The change
Two things, both in
cuda_core:build/cython/cu<major>instead of a single sharedbuild/cython, so each major keeps its own cache.build/.build-cuda-majorrecords the last completed build (written afterbuild_extsucceeds, so a failed build does not claim outputs it never produced). When it differs,setup.pysetsbuild_ext.force.The second is what handles the return leg, where directory keying can't help.
CUDA_PYTHON_COVERAGEis untouched, as requested — it still generates in-tree (build_dir=".") so it can package the generated sources.Migration is self-healing. An existing unkeyed
build/cythonis orphaned and ignored; the first build after this change regenerates into a keyed directory and overwrites the in-tree extensions. No cleanup step required.Verification
cu13 -> cu12 -> cu13with pixi on linux-64. Before this PR thecu12leg fails to compile. After, all three legs build,build/cython/cu12andbuild/cython/cu13both exist, and the extension from the secondcu13leg is byte-identical to the first (md5 1c53ab83a7201d54576175c912d03e93).That last point is the load-bearing assertion. Leg 1's generated
.cppis older than leg 2's.so, so without the forced rebuildnewer_group()would have skipped and left thecu12artifact in place — and the build would still have "succeeded". Only the byte-identical result distinguishes a real rebuild from a wrong skip.New
build-identity-roundtripjob inci-pixi-source-test.ymlruns that sequence on every relevant PR. 18 unit tests cover the stamp logic;pre-commitclean.Scope
Deliberately narrow — the CUDA major is the only axis keyed, and only
cuda_coreis touched.cuda_bindingsis not changed. It has nocompile_time_env, so its generated C++ is CUDA-major independent, and it cannot be source-built against CUDA 12 at all today (its sources referenceCUatomicOperation,nvrtcBundledHeadersInfo,CUstreamCigCaptureParams). That is a separate pre-existing problem, worth its own issue; I can file it.debugand coverage are not keyed. Both change the compiled output, and toggling either has a similar staleness problem — but it is a different defect from the one this PR is titled for. Worth noting for anyone tempted to add them:gdb_debug=Truedoes not change the generated C++ (I tested it — the.cis byte-identical; Cython only writes a sidecython_debug/directory), so keying a directory by it would only duplicate identical sources.linetracegenuinely does change the generated C++.cu12leg resolves publishedcuda-bindings12.x, not the local checkout —cuda_core'scu12environment omitslocal-depsby design.Relationship to #2443
#2443 proposed
pixi run cleanas the workflow. Per review feedback, this PR makes the artifacts configuration-aware so cleanup is not the correctness boundary. #2443 can stand down to narrowed recovery tooling.PLC Local Security Evidence (Advisory)
Local advisory checks only. Authoritative release gates remain Pulse, SonarQube, Coverity, BlackDuck, nSpect, ScanSpect, OSRB, and Anchore — none of the rows below are release-gate results.
gitlab-master.nvidia.com:5005(needs a PAT withread_registry).plc/security/pulse-secret-scan.json(skip-stub).github/workflows/ci-pixi-source-test.yml run-shell-injection(the existing nightlyfull-testjob), plustempfile-without-flush/dangerous-subprocess-use-auditincuda_bindings, which this PR no longer modifies.plc/security/semgrep.json.plc/security/dependency-*.json.plc/security/pip-licenses.json(skip-stub)build/, contents are a CUDA major string with no user-controlled input, and nothing is deleted. No shell invocation added