build: add Pixi development environments for every supported platform - #1150
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 742e8844d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bd589bcd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1150 +/- ##
=======================================
Coverage 67.92% 67.92%
=======================================
Files 208 208
Lines 22354 22354
Branches 72 72
=======================================
Hits 15184 15184
Misses 7148 7148
Partials 22 22
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7deed58e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20e7ce8948
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 939e397026
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43f68fd9f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4428b98def
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84ee9f94cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Setting up a cytnx build means installing a compiler, CMake, Ninja, a BLAS
and LAPACK pair, Boost, ARPACK, GoogleTest, Python and the contributor
tooling, and every platform spelled that differently -- mamba on Linux CI,
Homebrew or conda on macOS, and on Windows nothing at all, because MSVC
needs an activation step no package manager was performing.
Add a Pixi workspace that resolves all of it from conda-forge and locks it
for linux-64, linux-aarch64, osx-64, osx-arm64 and win-64 at once, so a
contributor on any of them gets identical versions. Three environments:
default OpenBLAS, every platform. Matches the openblas-cpu preset that
pyproject.toml builds wheels with.
mkl Intel MKL. The feature declares platforms = linux-64, osx-64,
win-64, because MKL is x86-only; the two ARM platforms simply
have no mkl environment rather than an unsolvable one.
cuda MKL plus CUDA 13.3 and cuTENSOR from NVIDIA's PyPI wheels.
Declared win-64 only: it exists to avoid a system CUDA Toolkit
on Windows, whereas Linux CUDA builds use one and macOS has no
CUDA.
Neither vendor's BLAS stack is listed in the shared [dependencies]. Each
vendor feature pins libblas, liblapack and liblapacke to its own builds, so
an environment cannot end up with a half-OpenBLAS, half-netlib mixture --
which is what happens when only libopenblas is pinned and the solver is
left free to satisfy liblapacke from the netlib variant. The OpenBLAS
feature also takes `openblas` itself, not just `libopenblas`: the
development split is what provides the libopenblas.so symlink CMake's
FindBLAS looks for and the lapacke.h header morse_cmake's FindLAPACKE
needs, and the runtime-only package has neither. Its build string is
pinned to the OpenMP-threaded variant, matching ci-cmake_tests.yml, because
the pthreads build runs a thread pool of its own that oversubscribes the
CPU whenever a BLAS call originates inside one of cytnx's or HPTT's OpenMP
regions.
The tasks drive the presets in CMakePresets.json rather than re-spelling
their cache variables, so a Pixi build and a manual `cmake --preset` build
land in the same build/<preset>/ tree. Which preset is an activation
variable, CYTNX_PRESET, that the mkl feature overrides -- not a second copy
of every task. A task defined by two features at once makes `pixi run
<task>` ambiguous and forces every invocation to carry -e, so each task is
defined exactly once. The C++ tasks configure with BUILD_PYTHON=OFF:
pybind11 is a PyPI dependency, so its CMake package config sits under
site-packages where find_package() does not look. scikit-build-core puts
that directory on CMAKE_PREFIX_PATH itself, which is why the
install-python task needs no override while a bare cmake invocation would
fail to configure. Splitting the two also keeps `pixi run test-cpp` from
building a Python extension it never loads.
Windows overrides every task, because there each command has to run inside
tools/activate_windows.bat. That script resolves the Visual Studio 2022
installation with vswhere, calls vcvars64, and restores the Pixi
environment's directories ahead of the ambient PATH, all in one process --
Pixi's own activation cannot do it, because activation.env values are
literal and do not expand %CONDA_PREFIX%, and Pixi can cache ordinary
variables independently of PATH across an environment update. Windows also
forces CMAKE_INTERPROCEDURAL_OPTIMIZATION off, which no other platform
does, because MSVC's /GL rejects the mixed static-archive link cytnx
produces while CMakeLists.txt enables it for every non-Apple platform.
tools/prepare_windows_import_libraries.py repairs two dependency layouts
CMake cannot consume as shipped, deriving what is missing from the
installed files rather than vendoring anything: conda-forge's ARPACK
provides a MinGW DLL with a GNU import archive, so the script reads the
DLL's PE export table and rebuilds an MSVC .lib with lib.exe; and NVIDIA's
CUDA 13 math-library wheels omit their MSVC import libraries while the NVVM
wheel installs its DLL under bin/x86_64 where CUDA 13's nvlink searches
bin/x64. It is idempotent and has a --check mode that needs no lib.exe.
The CUDA tasks configure the mkl-cuda-windows preset, which carries
USE_CUQUANTUM=OFF: NVIDIA publishes no Windows build of cuTensorNet or
cuStateVec (#1111), while cuTENSOR does ship one and stays enabled.
Verified on linux-64: `pixi install` materializes both the default and mkl
environments from the lock; `pixi run doctor` reports the conda-forge GCC
14.4, CMake 3.31, Ninja 1.13 and Python 3.10 Pixi resolved; and a configure
through `pixi run` resolves Boost 1.91, ARPACK, OpenBLAS, LAPACKE (header
and library) and GoogleTest 1.17 entirely from the Pixi environment, with
no system package involved. `pixi task list` shows the eight Linux tasks as
runnable under default and mkl while every cuda task is correctly reported
as not runnable on this platform. The macOS and Windows legs have no runner
available here.
Co-Authored-By: Claude <noreply@anthropic.com>
The Pixi workspace covers Linux, macOS and Windows, so the setup instructions belong where every contributor will look for them. CONTRIBUTING.md gains a "Development environment" section with the install commands, the task table, and the three environments with the platforms each one exists for; its opening paragraph stops pointing at Readme.md for build instructions, which does not carry any. That section also documents the one workaround a Linux or macOS contributor needs today. Pixi's default layout puts the environment in .pixi/ inside the checkout, and cytnx adds its LAPACKE include directory to the cytnx target's PUBLIC usage requirements, so CMake refuses to generate -- an exported include directory may not lie inside the source tree, because that path would be baked into CytnxTargets.cmake. Setting Pixi's detached-environments moves the environment out of the checkout and the generate step succeeds. #1120 owns the export contract itself; the note says so, so the workaround comes out again when that lands rather than becoming folklore. docs/dev/windows.md keeps only what is genuinely Windows-specific and links to that section for the rest: the Visual Studio 2022 prerequisite Pixi cannot supply, why every task re-enters tools/activate_windows.bat, the one option forced off there, the dependency layouts tools/prepare_windows_import_libraries.py repairs, and the CUDA environment built on NVIDIA's PyPI wheels. Readme.md's contribution section now points at CONTRIBUTING.md first and at the Windows notes second, rather than offering the Windows guide as the only development entry point. Co-Authored-By: Claude <noreply@anthropic.com>
… preset Two ways a fresh checkout could fail to build with nothing but the packages this workspace advertises. CMAKE_GENERATOR=Ninja was set only for win-64, and the presets in CMakePresets.json name no generator, so on Linux and macOS CMake fell back to its documented default of Unix Makefiles. ninja is in [dependencies]; make is not, and nothing else pulls it in. `pixi run test-cpp` therefore died looking for a make program on any host without a system make -- the build that appeared to work here was using /usr/bin/gmake from outside the environment. Move the variable to the shared [activation.env] so every platform gets Ninja; win-64 keeps only CMAKE_GENERATOR_PLATFORM and CMAKE_GENERATOR_TOOLSET, which mean something to the Visual Studio generator alone. The Windows tasks named the mkl-cpu preset outright while the default environment resolves OpenBLAS, so `pixi install` followed by `pixi run test-cpp` configured with USE_MKL=ON in an environment holding no MKL and failed in CMake's Intel BLAS lookup. Windows now follows CYTNX_RELEASE_PRESET, the same activation variable the mkl feature overrides, so selecting an environment selects the BLAS vendor on every platform. It is the release preset rather than CYTNX_PRESET because the debug presets turn on USE_DEBUG, whose address sanitizer is outside the validated MSVC toolchain; RUN_TESTS=ON is passed so the GoogleTest binary is still built. CYTNX_WHEEL_PRESET is renamed to CYTNX_RELEASE_PRESET now that the Windows C++ tasks use it too and it is no longer wheel-specific. Verified on linux-64: a configure through `pixi run configure` writes CMAKE_GENERATOR:INTERNAL=Ninja and a build.ninja, where it previously generated Makefiles. Co-Authored-By: Claude <noreply@anthropic.com>
…them Two settings were declared once but needed twice, and the second consumer in each pair was silently getting the default. CMAKE_ARGS is a scikit-build-core convention; plain cmake does not read it. The Windows configure and configure-cuda tasks invoke cmake directly, so they never saw -DCMAKE_INTERPROCEDURAL_OPTIMIZATION=OFF and CMakeLists.txt enabled interprocedural optimization for them -- which MSVC's /GL rejects against the mixed static-archive link cytnx produces, so the link would fail. Both tasks now pass the argument on their command line. CMAKE_ARGS stays for the install-python tasks, which do go through scikit-build-core. CMAKE_BUILD_PARALLEL_LEVEL=2 is likewise only consulted when nothing more specific is given. The build presets in CMakePresets.json carry jobs: 8, which cmake-presets(7) defines as equivalent to --parallel, so `cmake --build --preset mkl-cuda-windows` ran eight cudafe++ instances regardless of the variable -- the opposite of the cap it was there to impose on a memory-constrained host. build-cuda now passes -j 2 itself; the variable stays for install-python-cuda, whose build scikit-build-core drives. Co-Authored-By: Claude <noreply@anthropic.com>
Four things the workspace was getting wrong. The Python packages were declared twice. numpy, beartype, graphviz, pytest, pytest-cov, mypy and pybind11-stubgen are all in pyproject.toml already, and a second copy in pixi.toml is a list to keep in sync for no benefit. They are gone; install-python now installs `.[dev]` rather than `.` with --no-deps, so pip reads them from pyproject.toml. Only pybind11 stays, because find_package(pybind11) has to succeed for a bare `cmake --preset` before any editable install has happened. The C++ tasks and the editable install used different build directories, build/<preset> and build/python-<preset>, so a contributor building both compiled cytnx twice and neither tree served the other incrementally. Both now use build/<preset>, and BUILD_PYTHON keeps the preset's own ON: the reason it was forced off was that a direct cmake call cannot find pybind11's package config under site-packages, which -Dpybind11_DIR fixes properly. ctest and pytest now run against one set of objects. CUDA was Windows-only. conda-forge carries the whole stack for linux-64 -- cuda-toolkit, cuda-nvcc, cutensor, and cuquantum with custatevec and cutensornet -- so Linux gets a CUDA environment that needs no system toolkit and gives nothing up: it builds the ordinary mkl-cuda preset with USE_CUQUANTUM on, and needs none of the import-library repair Windows does. Windows keeps NVIDIA's PyPI wheels and mkl-cuda-windows, because conda-forge has no win-64 cuTensorNet or cuStateVec to offer. The dependencies, the activation variables and the tasks are split per target accordingly, with CYTNX_CUDA_PRESET naming the preset for each. Verified on linux-64: `pixi install` materializes from the lock, the lock resolves cuquantum 26.06, custatevec 1.14 and cutensornet 2.13 for the cuda environment, and a configure through `pixi run` reports "Build Python Wrapper: YES" with pybind11 3.0.4, OpenBLAS and LAPACKE all resolved from the Pixi environment. Co-Authored-By: Claude <noreply@anthropic.com>
A separate Windows page was the wrong shape for its contents. Most of it was either general -- how to install, which environment to pick, what the tasks do -- or a description of settings that already live in pixi.toml, where the reason for each one belongs beside the declaration rather than in a file that has to be kept in step with it. Delete the page. The general half joins CONTRIBUTING.md's development section, including the Visual Studio 2022 prerequisite Pixi cannot supply and the one-paragraph summary of what tools/prepare_windows_import_libraries.py repairs. The mechanical detail stays in pixi.toml's comments, which the section points at. CONTRIBUTING.md also stops enumerating the dependencies Pixi provides. That list is pixi.toml, and a prose copy of it is another thing to keep in sync; the section now says what Pixi covers without naming the packages. Readme.md loses its second link, since there is no longer a separate Windows page to send contributors to. Co-Authored-By: Claude <noreply@anthropic.com>
CYTNX_PRESET named a debug-* preset, which sets USE_DEBUG. CMakeLists.txt then instruments cytnx with AddressSanitizer and exports the sanitizer link option through the target, so the editable extension built in that same tree aborts the moment `pixi run test-python` imports it into this environment's ordinary Python: the ASan runtime is not first in the process. ci-cmake_tests.yml documents the same constraint from the other side, keeping USE_DEBUG off precisely so pytest needs no LD_PRELOAD shim. That constraint only bit once the C++ and Python tasks started sharing one build tree, since the sanitizer now reaches the extension the Python tests import. Point CYTNX_PRESET at the release presets, openblas-cpu and mkl-cpu, and pass RUN_TESTS explicitly so the tree still carries the GoogleTest binary -- through cmake.define for the editable install, which is additive and so does not displace the preset already in cmake.args. A sanitizer build stays available as `cmake --preset debug-openblas-cpu`, outside the shared tree. This also removes CYTNX_RELEASE_PRESET: Windows introduced it to avoid the debug presets for the same reason, and with one release preset for every platform the two variables collapse into one. Co-Authored-By: Claude <noreply@anthropic.com>
The Linux CUDA environment drew its toolkit from conda-forge, which is a different toolchain from the one cytnx-cuda is actually released against. A local CUDA build should compile against what the wheels compile against, so take the NVIDIA PyPI wheels instead: nvidia-cuda-nvcc, nvidia-cuda-cccl, nvidia-cuda-runtime, nvidia-cublas, nvidia-cusparse, nvidia-curand, nvidia-cusolver, cutensor-cu13 and cutensornet-cu13, at the version ranges tools/prepare_cuda_release.py declares in CUDA_BUILD_TOOLCHAIN and CUDA_RUNTIME_DEPENDENCIES. That file stays the source of truth for the release path; pixi.toml says so where the versions are repeated. Those wheels need two repairs before CMake and nvlink can use them, which tools/cibuildwheel_before_all_cuda.sh already performed for the release build: the libraries ship only versioned sonames while find_library() only matches the unversioned name, and nvidia-nvvm relocates libnvvm out of the nvvm/lib64 where nvlink's device-LTO path looks for it. Rather than write that twice, both halves move into tools/prepare_cuda_layout.sh, which takes the prefix the nvidia/, cutensor/ and cuquantum/ namespace packages were installed under -- an isolated --target directory for the release build, an environment's site-packages for Pixi. The release script now calls it, and a prepare-cuda task calls it for linux-64 before configure-cuda runs. Linux keeps USE_CUQUANTUM on through the ordinary mkl-cuda preset, because cuTensorNet does publish Linux wheels; Windows is unchanged and still uses mkl-cuda-windows, since NVIDIA publishes no Windows cuTensorNet or cuStateVec (#1111). Verified: the lock resolves the nine wheels and their transitive NVIDIA dependencies for the linux-64 cuda environment, and both shell scripts pass `bash -n`. Compiling a CUDA build against them is not exercised here. Co-Authored-By: Claude <noreply@anthropic.com>
The development section still said the Linux CUDA toolkit comes from conda-forge, and said nothing about why the tasks drive the release presets rather than the debug ones. Both now match pixi.toml: the toolkit comes from the NVIDIA PyPI wheels the release build installs, with tools/prepare_cuda_release.py named as where the versions are decided, and the note on AddressSanitizer says why a debug-* preset cannot be the shared tree and how to get one when it is wanted. Co-Authored-By: Claude <noreply@anthropic.com>
Two defects from the previous commit, which moved Linux CUDA onto the PyPI wheels. The edit that replaced the Linux dependency table reached one section too far and deleted [feature.cuda.target.win-64.pypi-dependencies] with it, so the win-64 cuda environment resolved with no CUDA packages at all: no nvcc for tools/activate_windows.bat to find, CUDA_PATH left unset, and prepare-cuda exiting before any Windows CUDA task could configure. The table is back, with a note on why Windows names the cuda-toolkit meta-package while Linux names the individual nvidia-* distributions -- the release specification in tools/prepare_cuda_release.py marks its entries `sys_platform == 'linux'`, so it does not cover win-64. install-python-cuda kept `depends-on = ["setup"]` while configure-cuda gained prepare-cuda, so an editable CUDA install on a clean Linux environment ran scikit-build-core against the unrepaired wheel layout, where find_library() cannot see the cuTENSOR and cuQuantum libraries for want of their unversioned names. It now depends on prepare-cuda as well, matching the Windows task that always did. The comment claiming Linux needs no layout repair went with them; both platforms repair a wheel layout now, and only the repair differs. Co-Authored-By: Claude <noreply@anthropic.com>
…ests Moving the tasks off the debug presets dodged the sanitizer rather than handling it, and cost the thing the presets were there for: USE_DEBUG is the configuration CONTRIBUTING.md gates a pull request on, so `pixi run test-cpp` stopped running the suite under AddressSanitizer at all. .claude/skills/build-test-workflow/scripts/build_preset.sh already solves this for its own runs, and tools/run_with_asan_preload.py now does the same for a command of the caller's choosing. An ASan-instrumented extension imported by an ordinary interpreter fails two ways: ASan's __cxa_throw interceptor resolves the real __cxa_throw from libstdc++, which a plain python never links, so the first C++ exception thrown inside the extension -- cytnx_error_msg surfacing as cytnx.CytnxError, which error-path tests hit constantly -- kills the process with no traceback; and LeakSanitizer reports CPython's own deliberate shutdown non-cleanup as hundreds of leaks. Preloading libasan.so together with libstdc++.so fixes the first, ASAN_OPTIONS detect_leaks=0 the second. The compiler comes from the build's own CMakeCache.txt rather than a bare `gcc` guess, because preloading one GCC's libasan.so into a binary built by another, or by Clang, is an ASan runtime/ABI mismatch; -print-file-name echoes its argument back unresolved, so both paths have to come back absolute before anything is preloaded. A build with USE_DEBUG off, a non-Linux platform, or a non-GCC compiler runs the command completely unchanged. Only test-python goes through it. test-cpp stays a plain ctest, so the GoogleTest suite keeps leak detection on, which is where a real cytnx leak would surface. Windows keeps the release presets: the sanitizer arrangement here is GCC's, and MSVC's own /fsanitize=address is not part of the toolchain validated for cytnx. Verified on linux-64: against build/debug-openblas-cpu (USE_DEBUG=ON) the shim exports LD_PRELOAD with the GCC 13 libasan.so and libstdc++.so plus ASAN_OPTIONS=detect_leaks=0; against a USE_DEBUG=OFF build tree and against a directory with no CMakeCache.txt it changes nothing and the command still runs. Co-Authored-By: Claude <noreply@anthropic.com>
The development section still claimed the tasks avoid the debug-* presets because the Python tests cannot import a sanitized extension. They no longer avoid them: tools/run_with_asan_preload.py preloads what the import needs, so both suites run instrumented on Linux and macOS, and only Windows takes the release presets. Co-Authored-By: Claude <noreply@anthropic.com>
Two corrections to how the sanitizer is handled. tools/run_with_asan_preload.py resolved libasan.so and libstdc++.so through the compiler's own -print-file-name, copying what .claude/skills/build-test-workflow/scripts/build_preset.sh does. That care is warranted there, because that script runs outside any managed environment and the compiler on PATH could be anything. Inside a Pixi environment the provenance question is already settled -- the compiler and the runtime come from the same prefix -- and going through the compiler is not merely redundant but wrong for libstdc++: conda-forge points RPATH at $PREFIX/lib, which carries libstdc++.so.6.0.35, while the compiler's private directory still has 6.0.33. Preloading the compiler's copy would satisfy the libstdc++.so.6 soname with a library missing the newer GLIBCXX_3.4 symbol versions. (For libasan.so the two paths resolve to one file, so that half was only redundant.) The script is deleted; test-python preloads $CONDA_PREFIX/lib/libasan.so and $CONDA_PREFIX/lib/libstdc++.so directly. The presets also flip: release is now the default and Linux the exception that takes debug-*. LD_PRELOAD is a Linux mechanism, so a debug-* preset on macOS produced a Clang-instrumented extension with nothing arranged to load the ASan dylib ahead of the interpreter -- the advertised test-python task would have aborted there. macOS would need DYLD_INSERT_LIBRARIES pointed at Clang's runtime and MSVC has its own arrangement again; neither is part of the toolchain validated for cytnx, and neither can be exercised from here, so both platforms build release and leave the sanitizer to a direct `cmake --preset debug-openblas-cpu`. Making Linux the exception keeps the instrumented configuration on the one platform where it is verified. RUN_TESTS moves onto the tasks, since only the debug-* presets set it and CYTNX_PRESET is a release preset everywhere else. Verified on linux-64: CYTNX_PRESET resolves to debug-openblas-cpu under default and debug-mkl-cpu under mkl, and a task with the LD_PRELOAD prefix expands $CONDA_PREFIX to the environment's own libasan.so and libstdc++.so. Co-Authored-By: Claude <noreply@anthropic.com>
libboost-devel was unconstrained and resolved to 1.91, which is inside the range 470d387 pinned the wheel build away from: Boost 1.89 marks boost::intrusive_ptr's members constexpr, and nvcc's cudafe++ frontend crashes on that in every CUDA translation unit reaching cytnx::intrusive_ptr_base through backend/Storage.hpp. A Pixi CUDA build would have hit exactly the crash the wheel build already works around. Constrained to <1.89, with a comment naming tools/cibuildwheel_before_all.sh as the constraint it has to match; the environment now resolves Boost 1.88. custatevec-cu13 was missing from the Linux CUDA dependencies. FindCUQUANTUM.cmake names both CUQUANTUM_TENSORNET_LIB and CUQUANTUM_CUSTATEVEC_LIB in its REQUIRED_VARS, and cutensornet-cu13 pulls only cutensor-cu13, so a configure under USE_CUQUANTUM would have failed looking for cuStateVec. It is in CUDA_RUNTIME_DEPENDENCIES in tools/prepare_cuda_release.py and should have come across with the rest of that list. cuquantum-cu13 stays absent, as it is there: it drags in cudensitymat, cupauliprop and custabilizer, none of which cytnx links. scikit-build-core is declared in pyproject.toml's [build-system].requires, so declaring it here too was the same duplication the Python packages were. Dropping it means the editable install can no longer use --no-build-isolation, which is the better arrangement anyway: PEP 517 now provisions scikit-build-core and pybind11 from pyproject.toml, exactly as ci-cmake_tests.yml does. pybind11 stays in [pypi-dependencies] because find_package(pybind11) has to succeed for the configure task, before any pip install has run. Co-Authored-By: Claude <noreply@anthropic.com>
Linux takes the debug-* presets for the CPU tasks, so that both suites run under AddressSanitizer, but the CUDA tasks stayed on the release mkl-cuda preset and so covered none of the GPU code that way. Point CYTNX_CUDA_PRESET at debug-mkl-cuda on linux-64, and set the wider ASAN_OPTIONS a CUDA build needs alongside it: protect_shadow_gap=0, because the CUDA runtime maps into ASan's shadow gap, and replace_intrin=0, because ASan's intrinsic replacements break CUDA's own memory routines. detect_leaks=0 comes along for the same reason it does elsewhere. This mirrors what .claude/skills/build-test-workflow/scripts/build_preset.sh exports for a debug-*-cuda preset, and being an activation variable it covers the build as well as anything run afterwards. Windows is unchanged: its CUDA tasks stay on mkl-cuda-windows, since the sanitizer arrangement here is GCC's and MSVC's is not part of the toolchain validated for cytnx. Co-Authored-By: Claude <noreply@anthropic.com>
`pixi run -e cuda install-python-cuda` goes through `cmake --install`,
which discards the build tree's RPATH and applies INSTALL_RPATH instead.
CMakeLists.txt sets that to
`${CMAKE_INSTALL_RPATH};${CMAKE_INSTALL_PREFIX}/lib`, and with
CMAKE_INSTALL_RPATH unset it reduces to scikit-build-core's staging
prefix -- a tempfile.TemporaryDirectory that is gone by the time the
install finishes. Nothing else puts the CUDA libraries on the loader's
search path, so importing the resulting extension fails on
libcudart.so.13, libcutensor.so.2 and their siblings.
Pass the same $ORIGIN-relative list the release build bakes in
(CUDA_INSTALL_RPATH in tools/prepare_cuda_release.py). It transfers
unchanged: CMakeLists.txt installs pycytnx into
${CMAKE_INSTALL_PREFIX}/cytnx and scikit-build-core maps that prefix onto
the wheel's platlib root, so the editable install lands the extension at
site-packages/cytnx/ with the NVIDIA, cuTENSOR and cuQuantum wheels as
siblings one level up -- the layout a released cytnx-cuda wheel also
sees. Single quotes keep the task shell from expanding $ORIGIN and from
reading the CMake list separators as command separators.
Move configure-cuda, build-cuda and install-python-cuda from
[feature.cuda.tasks] into [feature.cuda.target.linux-64.tasks] beside
prepare-cuda. All three were already overridden in
[feature.cuda.target.win-64.tasks], so no behavior changes, and it keeps
an ELF-only mechanism out of a section that reads as platform-neutral.
Verified by construction only -- no CUDA build was run for this change.
Co-Authored-By: Claude <noreply@anthropic.com>
Since Python 3.8 the Windows extension-module loader searches only the system directories, the directory holding the extension, and directories registered with os.add_dll_directory(). PATH is not consulted. That is why `pixi run -e mkl test-cpp` works -- test_main.exe is a plain executable and does resolve its imports through PATH -- while `test-python` cannot import the extension at all: cytnx/__init__.py imports .cytnx immediately, and libarpack.dll sits in Library\mingw-w64\bin, which nothing registers. tools/activate_windows.bat now exports CYTNX_WINDOWS_DLL_DIRS alongside the PATH it already composes, listing the subset of those directories that holds dependency DLLs rather than executables: the prefix itself, Library\mingw-w64\bin for libarpack.dll and the mingw runtime, Library\bin for MKL, and the NVIDIA wheels' bin/lib directories when the CUDA layout is present. Keeping it beside the PATH assignment is what stops the two lists from drifting. tools/run_with_dll_dirs.py reads that one variable, registers every entry that exists, and then runs the command that follows in the same process; missing entries are skipped so a CPU environment can share the variable with a CUDA one. The Windows test-python task invokes pytest through it. Stdlib only, per the tools/ convention. No shipped code changes: cytnx publishes no Windows wheels (release_pypi.yml builds Linux and macOS, and pyproject.toml has no [tool.cibuildwheel.windows]), so this is confined to the Pixi development environment and cytnx/__init__.py is untouched. Verified on linux-64 that the launcher skips absent and empty entries, registers the rest in order, forwards argv for both `-m module` and a script path, and propagates the exit code. The Windows path itself is unverified -- no Windows runner in this environment. Co-Authored-By: Claude <noreply@anthropic.com>
…onment
The environments encoded the preset: `default` meant OpenBLAS, `mkl` meant
MKL, `cuda` meant MKL plus CUDA, and Linux additionally forced the
debug-* presets. Three consequences, all of which this commit removes.
Switching preset meant switching environment, and some presets were
unreachable through a task at all -- there was no way to ask for
debug-openblas-cpu on macOS, for openblas-cpu on Linux, or for any CUDA
preset without a second multi-gigabyte environment. Two consecutive
commands could never use different presets.
The presets the tasks drove were not the presets that are released.
pyproject.toml pins openblas-cpu for the PyPI wheel,
tools/prepare_cuda_release.py switches to openblas-cuda for cytnx-cuda,
and conda_build/meta.yaml uses mkl-cpu on x86 and openblas-cpu
elsewhere -- all four are release presets, while Linux development ran
under USE_DEBUG and the CUDA tasks under debug-mkl-cuda, the wrong
vendor as well as the wrong build type.
A contributor without a GPU could not compile-check a CUDA build without
installing a separate environment, even though compiling CUDA needs no
GPU -- only running it does.
Every task now takes the preset as its first argument, defaulting to
openblas-cpu (mkl-cpu on Windows, which has no PyPI wheel and whose
conda package is built with MKL):
pixi run test-cpp
pixi run test-cpp debug-mkl-cpu
pixi run build openblas-cuda
One environment carries both BLAS vendors and the CUDA toolkit.
libblas/liblapack/liblapacke are single-vendor by construction in
conda-forge, but pinning them to OpenBLAS is enough: cytnx's USE_MKL path
resolves Intel's own libmkl_* and mkl_lapacke.h and needs none of the
shims. Verified by configuring both presets in the same prefix --
openblas-cpu resolves libopenblas.so and LAPACKE from it, mkl-cpu
resolves libmkl_intel_lp64/thread/core from it.
Platform differences are expressed as [target.<platform>...] tables
rather than features. A feature that declares `platforms` narrows every
environment containing it to that intersection, which would have dropped
linux-aarch64 and osx-arm64 entirely once MKL and CUDA joined the single
environment; a target table adds dependencies on one platform without
restricting anything.
MKLROOT is now set from CONDA_PREFIX on linux-64 and osx-64, as
tools/activate_windows.bat already did for Windows. CMake's MKL lookup
reads it, so an MKLROOT already exported for a system-wide MKL would
otherwise win over this environment's.
New tasks cover what had no entry point: `gate` runs the two debug CPU
presets CONTRIBUTING.md asks for before a pull request, `bench` builds
and runs benchmarks_main, `stubs`/`stubtest` regenerate and check the
committed type stubs, and `clean` reclaims one preset's build tree.
configure turns RUN_TESTS and RUN_BENCHMARKS on whatever the preset says,
so a tree never needs reconfiguring to build a different target, and
`benchmark` joins the dependencies for the find_package(benchmark
REQUIRED) that RUN_BENCHMARKS triggers.
tools/run_with_dll_dirs.py becomes tools/run_pytest.py: with the preset
now an argument, one launcher can arrange everything importing the
extension needs. It keeps the Windows DLL directory registration and adds
the AddressSanitizer preload for a debug-* preset on Linux, re-executing
once with LD_PRELOAD -- which cannot be set after the process starts --
and appending detect_leaks=0 to ASAN_OPTIONS. A debug-* preset on macOS
now fails with an explanation instead of importing an instrumented
extension into an unprepared interpreter.
Co-Authored-By: Claude <noreply@anthropic.com>
RUN_TESTS=ON is what makes a build tree able to build test_main, so every Pixi task turns it on -- and CMakeLists.txt reads it as a second signal, adding --coverage to every cytnx translation unit for GNU and Clang. A development tree therefore carried .gcno files, instrumented code and slower test runs for a coverage report nobody reads locally; Codecov collects coverage in CI, from CI's own build. configure and install-python now wire tools/strip_coverage_launcher.sh in as CMAKE_CXX_COMPILER_LAUNCHER and CMAKE_CXX_LINKER_LAUNCHER, which removes the flag before it reaches the compiler and the linker. Stripping the token is what works: --coverage is a driver shorthand that a trailing -fno-profile-arcs -fno-test-coverage does not cancel. The launcher moves from .claude/skills/build-test-workflow/scripts/ to tools/, so the Pixi tasks and build_preset.sh share one copy instead of diverging; build_preset.sh resolves it from the repository root and needs only its path updated. Windows wires in nothing, because CMakeLists.txt applies coverage only for GNU and Clang and the launcher is a shell script MSVC could not run. Verified on linux-64: a fresh `pixi run configure openblas-cpu` puts the launcher into build.ninja, and compiling CMakeFiles/cytnx.dir/src/Network_base.cpp.o produces the object with no .gcno beside it. Co-Authored-By: Claude <noreply@anthropic.com>
790189e to
d209a05
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This is good enough. Minor improvements can be left for the future. |
There was a problem hiding this comment.
Do we really need to commit .lock file?
There was a problem hiding this comment.
Yes, we should. pixi.lock will not be consumed by downstream. It reproduces the development environment.
The branch was retargeted onto #1149's merge commit when that PR landed, and master has advanced 41 commits since. Two conflicts, both from the same cause: master moved the agent skills from .claude/skills/ to .agents/skills/ (leaving .claude/skills as a symlink), while this branch had moved one file out of that tree. - strip-coverage-launcher.sh was renamed by both sides -- to tools/strip_coverage_launcher.sh here, and to .agents/skills/build-test-workflow/scripts/ on master. Resolved to the tools/ copy alone. The script is a build tool the skill invokes rather than a skill itself, and pixi.toml's configure task needs it too; keeping master's copy as well would restore the two-copies-that-drift situation moving it was meant to end. - build_preset.sh's coverage_launcher assignment conflicted because each side pointed it at its own new path. Resolved to the tools/ path at the file's new .agents/ location. Verified against the merged tree: every preset pixi.toml names still exists in CMakePresets.json, CMakeLists.txt still applies --coverage under RUN_TESTS so the launcher is still needed, benchmarks_main is still the benchmark target, and the three pins pixi.toml documents as having to match elsewhere still agree -- pybind11 ==3.0.4 with pyproject.toml, clang-format 14.* with .pre-commit-config.yaml, and boost <1.89 with tools/cibuildwheel_before_all.sh. Co-Authored-By: Claude <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Rebase of #1117 onto #1149, with the Pixi workspace extended from
win-64only to every platform cytnx is developed on, and the Windows guide dissolved intopixi.tomlcomments andCONTRIBUTING.mdrather than added as a page of its own.master → #1149 → this PR. The previous base,codex/windows-msvc-cuda-stack, combined the already-merged #1110 with #1116; #1116 is independently open againstmasterand nothing here depends on it. #1118, #1119 and #1113 were stacked on the old head of this branch and need retargeting.Problem
Setting up a cytnx build means installing a compiler, CMake, Ninja, a BLAS/LAPACK pair, Boost, ARPACK, GoogleTest, Python and the contributor tooling — and every platform spelled it differently: mamba on Linux CI, Homebrew or conda on macOS, and on Windows nothing at all, because MSVC needs an activation step no package manager was performing.
#1117 solved that for Windows. Its workspace declared
platforms = ["win-64"], so Linux and macOS contributors got none of it.Fix
One workspace, five platforms —
linux-64,linux-aarch64,osx-64,osx-arm64,win-64— resolved together into a singlepixi.lock.One environment. It carries both BLAS vendors and the CUDA toolkit, so no
pixi runever needs-e.libblas/liblapack/liblapackeare single-vendor by construction in conda-forge, but pinning them to OpenBLAS is enough for both: cytnx'sUSE_MKLpath resolves Intel's ownlibmkl_*andmkl_lapacke.hand needs none of the shims. CUDA is installed rather than split off because compiling a CUDA preset needs no GPU — only running one does — so a contributor without a GPU can still compile-check before pushing.Platform differences are
[target.<platform>...]tables, not features. A feature that declaresplatformsnarrows every environment containing it to that intersection, which would silently droplinux-aarch64andosx-arm64once MKL and CUDA joined the single environment.The preset is a task argument, not a property of the environment. Every task takes it first, so consecutive commands can build different presets:
The default is a release preset on purpose.
pyproject.tomlpinsopenblas-cpufor the PyPI wheel,tools/prepare_cuda_release.pyswitches toopenblas-cudaforcytnx-cuda, andconda_build/meta.yamlusesmkl-cpuon x86 — all four released artifacts are release presets, so the everyday build now matches one. Windows defaults tomkl-cpu, since there is no Windows wheel and the conda package uses MKL.pixi run gateruns the twodebug-*CPU presetsCONTRIBUTING.mdasks for before a pull request.Everything a build tree needs is decided once, at configure.
RUN_TESTSandRUN_BENCHMARKSgo on whatever the preset says, so a tree never reconfigures to build a different target, andbuild/<preset>/is shared by the C++ suite and the editable install.RUN_TESTSalso makesCMakeLists.txtadd--coverageto every cytnx translation unit;tools/strip_coverage_launcher.shremoves it before it reaches the compiler and linker, so a development tree does not pay for a report only Codecov reads. That launcher moves out of.claude/skills/intotools/, so the Pixi tasks andbuild_preset.shshare one copy.tools/run_pytest.pyarranges whatever the preset needs before the extension is imported: the AddressSanitizer preload for adebug-*preset on Linux (re-executing once, sinceLD_PRELOADcannot be set after start), and on Windows the DLL directory registration Python's extension loader has required since 3.8 —PATHreachestest_main.exebut never animport cytnx, which is whylibarpack.dllunderLibrary\mingw-w64\binwas unreachable.CUDA comes from the same NVIDIA PyPI wheels the release build installs, with the version ranges tracking
tools/prepare_cuda_release.py.tools/prepare_cuda_layout.shis extracted fromtools/cibuildwheel_before_all_cuda.sh, which now delegates to it, so the release path and the Pixi path do the same thing.custatevec-cu13is named explicitly becauseFindCUQUANTUM.cmakerequires it and nothing else pulls it in. The editable install passes the release's$ORIGIN-relativeCMAKE_INSTALL_RPATH, without which the extension's only RUNPATH entry is scikit-build-core's temporary staging directory.MKLROOTis set fromCONDA_PREFIXonlinux-64andosx-64, astools/activate_windows.batalready did for Windows. CMake's MKL lookup reads it, so anMKLROOTexported for a system-wide MKL would otherwise win.Boost is pinned
<1.89, matchingtools/cibuildwheel_before_all.sh. Boost 1.89 makesboost::intrusive_ptr's members constexpr, which crashescudafe++in every CUDA translation unit reachingcytnx::intrusive_ptr_base; unconstrained it resolved to 1.91.Docs.
CONTRIBUTING.mdgains a "Development environment" section — install commands, how to choose a preset, the task table. It enumerates no dependencies:pixi.tomlis that list.docs/dev/windows.mdis not added; its content is split between those two places.One workaround, documented rather than hidden
A Linux or macOS build needs
pixi config set --local detached-environments true. Pixi's default layout puts the environment in.pixi/inside the checkout, and cytnx adds its LAPACKE include directory to thecytnxtarget'sPUBLICusage requirements, so CMake refuses to generate:This is the same export-contract failure #1114 records for cuTENSOR on Windows, reached from the CPU build on Linux. #1120 owns the fix; the note in
CONTRIBUTING.mdnames it, so the workaround comes back out when that lands. (It bites the OpenBLAS path only — the MKL path exports no such directory and generates cleanly either way.)Testing
No C++ or Python source changes; the diff is
pixi.toml,pixi.lock, docs, andtools/. The C++ suites for the tree this sits on are reported in #1149.On
linux-64:pixi lock --checkpasses against the committed manifest, andpixi installmaterializes the environment.openblas-cpufindslibopenblas.soand LAPACKE in the Pixi environment,mkl-cpufindslibmkl_intel_lp64/thread/corein the same one. Configure and generate complete for both with detached environments.depends-onforwards the preset argument throughconfigure → build → test-cpp.build.ninja, and compilingCMakeFiles/cytnx.dir/src/Network_base.cpp.oproduces the object with no.gcnobeside it.tools/run_pytest.pycomposesLD_PRELOADfrom$CONDA_PREFIX/libandASAN_OPTIONS=protect_shadow_gap=0:replace_intrin=0:detect_leaks=0, matching whatbuild_preset.shexports for adebug-*-cudapreset.Not verified here: no macOS runner, no Windows runner, and no GPU-capable toolchain, so the Windows and CUDA halves rest on the lock resolving and on matching the release build's own configuration. No full build or
ctestof the new task graph was run — verification stopped at configure plus a single translation unit.Nothing in CI exercises
pixi.toml, so these tasks can rot silently; #1158 tracks adding a smoke job, deliberately left out of this PR to keep the diff scoped to the development environment.The sandbox's
conda-mapping.prefix.devis blocked by network policy, sopixi lockwas run against the same mapping data served fromraw.githubusercontent.com/prefix-dev/parselmouth. The committedpixi.tomlcarries no such override andpixi lock --checkvalidates the lock against it unchanged.