Add Windows CI/CD#552
Open
wavefunction91 wants to merge 10 commits into
Open
Conversation
Adds .github/workflows/build-and-test-windows.yaml: a two-job-per-compiler pipeline (matrix: msvc, clang-cl) on GitHub-hosted windows-latest, wiring the Windows build support from #450 into CI on parity with Linux/macOS. - deps job: vcpkg install + full cmake build, caching vcpkg_installed and the FetchContent build dir (libint2/gauxc/ecpint/blaspp/lapackpp) so the slow deps build once. Split out so the first (uncached) build gets its own 6h budget. - build-test job: restores the cache, rebuilds qdk, runs ctest, then builds the Python package (reusing the installed C++ lib) and runs pytest. - Shared composite action .github/actions/windows-msvc-env imports the VS x64 dev environment and exposes the toolset version for cache keys. Temporary push trigger on session/win_ci_cd for iteration; final triggers to be set before merge. No Windows pip wheel (PR CI only). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The msvc full build (deps + qdk + tests) exceeds the 6h GitHub-hosted job cap (it reached 2246/2384 before being cancelled, so no cache was saved). Build only the slow FetchContent dependency targets (int2/ecpint/gauxc/blaspp/lapackpp) in the deps job to warm the cache well under 6h; qdk, macis, and tests are built in the build-test job (their sources are re-checked-out there regardless). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The libint2 static library target is named `libint2` (its output file is int2.lib); `int2` is not a Ninja target. Caught by the deps-only build's fast target validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MSVC's /O2 optimizer is pathologically slow on libint2's large Unity translation units: building libint2 took 5h17m on a 4-core windows-latest with MSVC (vs ~3 min with clang-cl on the same TUs), exceeding the 6h CI job cap. Disabling Unity for the libint2 object library on MSVC (cl) splits the generated integral code back into many small TUs that compile quickly and parallelize. clang-cl handles the Unity TUs efficiently, so Unity is left enabled there. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MSVC cannot build the dependency stack within the 6h GitHub-hosted job cap even with Unity off (libint2's ~10.5k TUs + gauxc reach ~87% on a 4-vCPU runner). Make the dependency build resumable so progress is never lost: - The deps job restores the furthest prior progress (exact key = a completed build; restore-keys prefix = the newest partial), builds the dependency targets in a time-boxed step (255 min, under the cap), and saves the build dir: a completed build under the stable key, a timed-out build under a per-run partial key. The next run restores that and Ninja continues. Once a complete cache exists the build is skipped. - vcpkg install and configure are skipped when their outputs are already restored. - A best-effort prune keeps the cache store bounded (newest partial for resume; drop all partials once complete). Also stop pinning CMAKE_BUILD_PARALLEL_LEVEL=4 (that was local-dev guidance for a RAM-limited laptop); let Ninja use all cores so the build scales with the runner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The resumable partial-cache save never ran: a step `if:` without a status-check function has an implicit `success()` ANDed in, so `if: steps.build.outcome == 'failure'` became `success() && ...` and was skipped once the build step failed (timed out). Gate both save steps with `always() &&` so they evaluate regardless of the failed build and select on the build outcome. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resume the msvc dependency build from the saved partial cache. No file changes, so the dependency-input hash is unchanged and the partial cache remains valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uncapping parallelism caused MSVC to run out of heap (fatal error C1060) when several qdk TUs that include libint2's engine.impl.h compiled concurrently on the 16 GB runner (~4 heavy TUs x several GB exhausted RAM). Cap the build-test C++ and Python builds at min(cores, RAM/4GB): 4 on the 16 GB / 4-vCPU windows-latest (proven safe), scaling up automatically on a larger runner. The dependency build (small libint2 TUs) stays uncapped for speed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On MSVC, libint2/unit/build (a compile-at-test-time meta-test of the vendored libint2 dependency) exceeds the 400s ctest timeout, failing 3 of 889 tests (libint2/unit/build + its two dependent run tests); all 886 qdk tests pass. These validate libint2 itself (upstream-tested, and run on the Linux job), so exclude them on Windows alongside the existing MACIS_SERIAL_TEST exclusion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Now that the Windows workflow is green for both MSVC and clang-cl, replace the temporary session-branch push trigger with the production triggers matching the Linux/macOS workflow: push to main, pull_request on any branch, merge_group, and workflow_dispatch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wavefunction91
requested review from
ConradJohnston,
RushiGong,
nabbelbabbel and
sobolevnrm
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a dedicated Windows GitHub Actions pipeline for building qdk-chemistry with both MSVC and clang-cl, including a resumable dependency-build cache strategy to cope with long third-party builds on hosted runners.
Changes:
- Disable CMake unity builds for libint2 object target under MSVC (keep unity for clang-cl) to reduce compile time.
- Add a Windows CI workflow that caches and resumes third-party dependency builds, then builds/tests C++ and Python.
- Add a composite action to import the VS dev environment (vcvarsall) and expose compiler path + toolset for cache keys.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
cpp/cmake/third_party.cmake |
Disables unity builds for libint2 object target when using MSVC (non-clang-cl). |
.github/workflows/build-and-test-windows.yaml |
New Windows workflow: resumable deps cache + build/test for C++ and Python on MSVC/clang-cl. |
.github/actions/windows-msvc-env/action.yml |
New composite action to set up MSVC/clang-cl environment and outputs for cache keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+130
to
+132
| # No --parallel / CMAKE_BUILD_PARALLEL_LEVEL: let Ninja use all cores. | ||
| cmake --build "cpp/build-${{ matrix.compiler }}" --target libint2 ecpint gauxc blaspp lapackpp | ||
| if ($LASTEXITCODE -ne 0) { throw "Dependency build failed ($LASTEXITCODE)" } |
| run: | | ||
| # Best-effort: keep the newest partial (for resume) and drop older ones; | ||
| # once a complete build succeeds, drop all partials for this base key. | ||
| $base = "windows-deps-${{ matrix.compiler }}-${{ env.VCPKG_TRIPLET }}-vs${{ steps.env.outputs.toolset }}-${{ env.DEPS_CACHE_VERSION }}-" |
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.