PR-[10] - feat: add Windows CI support - #258
Draft
San1357 wants to merge 104 commits into
Draft
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
PR[1]: build: migrate from setuptools to scikit-build-core
PR[2] : feat: add libcint C extension bindings for various integrals
PR[3]: feat: use PyArray_GETPTR instead of PyLong_AsVoidPtr + fix dylib rpath
PR[4]: fix: platform-aware libcint loading + moment integral libcint v6 fix
PR-5 - feat: add C shell loop for all 1-electron integrals
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Windows CI, but also substantially rewrites libcint packaging and APIs.
Changes:
- Expands CI across Windows, macOS, and Ubuntu.
- Migrates builds to CMake/scikit-build-core with native bindings.
- Replaces
CBasisintegral APIs and adds extensive tests.
Reviewed changes
Copilot reviewed 7 out of 19 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/pytest.yaml |
Adds platform-specific CI setup and execution. |
CMakeLists.txt |
Builds and packages libcint bindings. |
pyproject.toml |
Migrates packaging to scikit-build-core. |
gbasis/integrals/src/libcint_wrap.c |
Implements native integral wrappers. |
gbasis/integrals/libcint.py |
Reworks the Python libcint API. |
gbasis/evals/_deriv.py |
Adds explicit Hermite argument casting. |
tests/test_libcint.py |
Expands native-binding tests. |
Suppressed comments (3)
gbasis/integrals/src/libcint_wrap.c:480
- The Cartesian gradient bindings have the same component-loss issue as the spherical variants: libcint emits three components, while the scalar macro exposes only component zero. Switch these to a three-component wrapper and propagate that shape through the Python API.
DEFINE_INT1E_LOOP_FN(ipkin_integral_array, cart, int1e_ipkin, int1e_ipkin)
DEFINE_INT1E_LOOP_FN(ipnuc_integral_array, cart, int1e_ipnuc, int1e_ipnuc)
DEFINE_INT1E_LOOP_FN(iprinv_integral_array, cart, int1e_iprinv, int1e_iprinv)
gbasis/integrals/src/libcint_wrap.c:485
- The Cartesian GIAO bindings also route three-component libcint functions through the scalar macro, silently dropping two components. Use the multi-component wrapper and expose the full result in the Cartesian Python methods.
DEFINE_INT1E_LOOP_FN(ia01p_integral_array, cart, int1e_ia01p, int1e_ia01p)
DEFINE_INT1E_LOOP_FN(ircxp_integral_array, cart, int1e_cg_irxp, int1e_cg_irxp)
DEFINE_INT1E_LOOP_FN(igkin_integral_array, cart, int1e_igkin, int1e_igkin)
DEFINE_INT1E_LOOP_FN(igovlp_integral_array, cart, int1e_igovlp, int1e_igovlp)
DEFINE_INT1E_LOOP_FN(ignuc_integral_array, cart, int1e_ignuc, int1e_ignuc)
.github/workflows/pytest.yaml:64
- MSYS2 also treats
>as shell redirection, so the Windows command does not passnumpy>=2.0to pip and writes to=2.0. Quote the requirement string.
pip install numpy>=2.0 setuptools_scm scikit-build-core cmake ninja
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| DEFINE_INT1E_LOOP_FN_OPT(overlap_integral_array, sph, int1e_ovlp, int1e_ovlp) | ||
| DEFINE_INT1E_LOOP_FN_OPT(kinetic_integral_array, sph, int1e_kin, int1e_kin) | ||
| DEFINE_INT1E_LOOP_FN_OPT(nuclear_integral_array, sph, int1e_nuc, int1e_nuc) | ||
| DEFINE_INT1E_LOOP_FN_MULTICOMP(momentum_integral_array, sph, int1e_ipovlp, int1e_ipovlp, 3) |
Comment on lines
+460
to
+462
| DEFINE_INT1E_LOOP_FN(ipkin_integral_array, sph, int1e_ipkin, int1e_ipkin) | ||
| DEFINE_INT1E_LOOP_FN(ipnuc_integral_array, sph, int1e_ipnuc, int1e_ipnuc) | ||
| DEFINE_INT1E_LOOP_FN(iprinv_integral_array, sph, int1e_iprinv, int1e_iprinv) |
Comment on lines
+463
to
+467
| DEFINE_INT1E_LOOP_FN(ia01p_integral_array, sph, int1e_ia01p, int1e_ia01p) | ||
| DEFINE_INT1E_LOOP_FN(ircxp_integral_array, sph, int1e_cg_irxp, int1e_cg_irxp) | ||
| DEFINE_INT1E_LOOP_FN(igkin_integral_array, sph, int1e_igkin, int1e_igkin) | ||
| DEFINE_INT1E_LOOP_FN(igovlp_integral_array, sph, int1e_igovlp, int1e_igovlp) | ||
| DEFINE_INT1E_LOOP_FN(ignuc_integral_array, sph, int1e_ignuc, int1e_ignuc) |
| self._ovlp_minhalf = None | ||
|
|
||
|
|
||
| def overlap(self, transform=None): |
Comment on lines
+996
to
+999
| # Apply permutation | ||
| out = out[self._permutations, :][:, self._permutations] | ||
| # Apply permutation | ||
| out = out[self._permutations, :][:, self._permutations] |
Comment on lines
+5
to
+9
| # Force MinGW compiler on Windows before project() declaration | ||
| if(WIN32) | ||
| set(CMAKE_C_COMPILER "gcc" CACHE STRING "" FORCE) | ||
| set(CMAKE_CXX_COMPILER "g++" CACHE STRING "" FORCE) | ||
| endif() |
| - name: Install development version (non-Windows) | ||
| if: ${{ runner.os != 'Windows' }} | ||
| run: | | ||
| pip install numpy>=2.0 setuptools_scm scikit-build-core cmake ninja |
Comment on lines
+1
to
+3
| /* | ||
| * libcint_wrap.c — Python/C API bindings for libcint GTO integral library. | ||
| * |
Comment on lines
+604
to
+608
| if integral == "gradient_kinetic": | ||
| # Compare C shell-loop against make_int1e path on same CBasis instance | ||
| lc_int = lc_basis.gradient_kinetic() | ||
| npt.assert_array_equal(lc_int.shape, (lc_basis.nbfn, lc_basis.nbfn)) | ||
| assert np.all(np.isfinite(lc_int)) |
|
|
||
| extern int int3c2e_sph(double *out, int *dims, int *shls, int *atm, int natm, | ||
| int *bas, int nbas, double *env, void *opt, double *cache); | ||
| extern void cint3c2e_sph_optimizer(CINTOpt **, int *, int, int *, int, double *); |
added 22 commits
August 12, 2026 07:49
msricher
reviewed
Aug 12, 2026
Comment on lines
613
to
615
| py_basis = make_contractions(basis_dict, atsyms, atcoords, coord_types="spherical") | ||
| lc_basis = CBasis(py_basis, atsyms, atcoords, coord_type="spherical") | ||
|
|
Collaborator
There was a problem hiding this comment.
Can you make sure the arguments, keywords, etc., match, between the gbasis Python and CBasis implementation? For example, coord_type and coord_types do not match here.
Contributor
Author
There was a problem hiding this comment.
Thanks for catching this, The difference is intentional — make_contractions takes coord_types (plural) as per the standard GBasis API, while CBasis.init takes coord_type (singular) to match the libcint convention. Both are correct for their respective functions.
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.
Summary
Add Windows CI support to the pytest workflow.
Changes
windows-latestto the OS matrix in.github/workflows/pytest.yamlpip install --no-build-isolationwithcmakeandninjapre-installedTesting
CI will verify that the build and tests pass on Windows with Python 3.9, 3.10, 3.11, and 3.12.
Related