-
Notifications
You must be signed in to change notification settings - Fork 45
PR[1]: build: migrate from setuptools to scikit-build-core #252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
San1357
wants to merge
6
commits into
theochem:master
Choose a base branch
from
San1357:pr-1/scikit-build-migration
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
0fb4fce
build: migrate from setuptools to scikit-build-core
8a37c73
build: restore comments and fix numpy version consistency
b7c0938
build: add CMakeLists.txt with FetchContent and platform detection
4b3ac64
style: fix comments
467ae73
build: fix misleading comment about setuptools in pyproject.toml
San1357 334ecc4
build: address Copilot review comments
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| # CMakeLists.txt — Build C extension for GBasis libcint integration (Issue: #229) | ||
|
|
||
| # Usage: | ||
| # Default (all platforms): pip install qc-gbasis | ||
| # qcint (x86+AVX2 only): pip install qc-gbasis -C cmake.args="-DUSE_QCINT=ON" | ||
|
|
||
|
|
||
| cmake_minimum_required(VERSION 3.17) | ||
| project(gbasis_libcint C) | ||
|
|
||
| # Python and NumPy are required to build the C extension module | ||
|
|
||
| find_package(Python COMPONENTS Interpreter Development.Module REQUIRED) | ||
| find_package(Python COMPONENTS NumPy REQUIRED) | ||
|
|
||
| # Get NumPy include path for C extension (numpy/arrayobject.h) | ||
|
|
||
| execute_process( | ||
| COMMAND "${Python_EXECUTABLE}" "-c" | ||
| "import numpy; print(numpy.get_include())" | ||
| OUTPUT_VARIABLE NumPy_INCLUDE_DIRS | ||
| OUTPUT_STRIP_TRAILING_WHITESPACE | ||
| ) | ||
|
|
||
| # ── Platform Detection — x86 vs ARM ────────────────────── | ||
| # Detect platform: use qcint on x86+AVX2, libcint on ARM/non-AVX | ||
| # qcint is an optimized version of libcint for x86 with SIMD support | ||
| include(CheckCCompilerFlag) | ||
|
|
||
| if(MSVC) | ||
| check_c_compiler_flag("/arch:AVX2" HAS_AVX2) | ||
| else() | ||
| check_c_compiler_flag("-mavx2" HAS_AVX2) | ||
| endif() | ||
|
|
||
| # Respect user override via -DUSE_QCINT=ON/OFF; otherwise auto-detect | ||
| if(NOT DEFINED USE_QCINT) | ||
| if(HAS_AVX2 AND NOT CMAKE_SYSTEM_PROCESSOR MATCHES "arm") | ||
| set(USE_QCINT ON CACHE BOOL "Use qcint instead of libcint" FORCE) | ||
| message(STATUS "Architecture: x86 + AVX2 detected — use qcint") | ||
| else() | ||
| set(USE_QCINT OFF CACHE BOOL "Use qcint instead of libcint" FORCE) | ||
| message(STATUS "Architecture: ARM/non-AVX detected — use libcint") | ||
| endif() | ||
| endif() | ||
|
|
||
|
|
||
| # Download libcint or qcint based on platform detection | ||
| # Both are pinned to v6.1.2 for stability | ||
| # qcint: optimized for x86+AVX2 | ||
| # libcint: generic, works on all platforms | ||
|
|
||
| include(FetchContent) | ||
|
|
||
| if(USE_QCINT) | ||
| FetchContent_Declare( | ||
| libcint | ||
| GIT_REPOSITORY https://github.com/sunqm/qcint.git | ||
| GIT_TAG v6.1.2 | ||
| ) | ||
| else() | ||
| FetchContent_Declare( | ||
| libcint | ||
| GIT_REPOSITORY https://github.com/sunqm/libcint.git | ||
| GIT_TAG v6.1.2 | ||
| ) | ||
| endif() | ||
|
|
||
| # Compile libcint — Fortran disabled, range-separated Coulomb enabled | ||
|
|
||
| set(WITH_FORTRAN OFF CACHE BOOL "" FORCE) | ||
| # Enable range-separated Coulomb integrals | ||
| set(WITH_RANGE_COULOMB ON CACHE BOOL "" FORCE) | ||
|
|
||
| # Redirect libcint's install to build directory to avoid permission issues | ||
| set(CMAKE_INSTALL_PREFIX "${CMAKE_BINARY_DIR}/local" CACHE PATH "" FORCE) | ||
| set(SKIP_INSTALL_ALL ON CACHE BOOL "" FORCE) | ||
| FetchContent_MakeAvailable(libcint) | ||
|
|
||
| # Build Python C extension module from libcint_wrap.c | ||
| python_add_library(libcint_bindings MODULE | ||
| gbasis/integrals/src/libcint_wrap.c | ||
| WITH_SOABI | ||
| ) | ||
|
|
||
| # Include NumPy and libcint headers | ||
| target_include_directories(libcint_bindings PRIVATE | ||
| ${NumPy_INCLUDE_DIRS} | ||
| ${libcint_SOURCE_DIR}/include | ||
| ) | ||
|
|
||
| # Link C extension with libcint | ||
| target_link_libraries(libcint_bindings PRIVATE cint) | ||
|
|
||
| # Prevent libcint from installing to system directories | ||
| set(CINT_INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}/cint_install CACHE PATH "" FORCE) | ||
|
|
||
| # Install only the Python extension into the wheel | ||
| install(TARGETS libcint_bindings | ||
| LIBRARY DESTINATION ${SKBUILD_PLATLIB_DIR}/gbasis/integrals/lib | ||
| ) | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
|
|
||
|
|
||
| /* libcint Python/C API bindings. | ||
| * The full bindings will be implemented in a follow-up PR. | ||
| */ | ||
|
|
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,12 @@ | ||
| # Replaced setuptools with scikit-build-core | ||
| # Reason: use a CMake-based build (via scikit-build-core) to compile and bundle libcint/qcint | ||
| # as part of the Python wheel build | ||
|
|
||
| [build-system] | ||
| requires = ["setuptools>=64", "setuptools-scm>=8"] | ||
| build-backend = "setuptools.build_meta" | ||
| requires = ["numpy>=2.0", "scikit-build-core>=0.9","setuptools-scm>=8"] | ||
| build-backend = "scikit_build_core.build" | ||
|
Comment on lines
5
to
+7
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — added setuptools-scm to [build-system].requires. |
||
|
|
||
|
|
||
|
|
||
| [project] | ||
| name = "qc-gbasis" | ||
|
|
@@ -31,18 +37,38 @@ classifiers = [ | |
| "Programming Language :: Python :: 3.12", | ||
| ] | ||
| dependencies = [ | ||
| # Ensure the minimal versions are kept consistent with those in .github/workflows/pytest.yaml | ||
| "numpy >=1.22, <2.0.0; platform_system=='Windows'", | ||
| "numpy >=1.22; platform_system=='Linux'", | ||
| "scipy>=1.11.1", | ||
| # Updated to numpy>=2.0 required for scikit-build-core C extension support | ||
| # Note: Keep consistent with .github/workflows/pytest.yaml | ||
| "numpy>=2.0", | ||
| "scipy>=1.13.0", | ||
| "importlib_resources", | ||
| "sympy", | ||
|
Comment on lines
39
to
45
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Raised scipy minimum to 1.13.0 which added NumPy 2.x support. |
||
| ] | ||
| dynamic = ["version"] | ||
|
|
||
| [tool.scikit-build] | ||
| # Build type: Release for optimized production build (no debug symbols) | ||
| cmake.build-type = "Release" | ||
|
|
||
| # Replaces [tool.setuptools.packages.find] from old setuptools config | ||
| # Only pack gbasis folder in the wheel | ||
| wheel.packages = ["gbasis"] | ||
|
|
||
|
|
||
|
|
||
| [tool.scikit-build.metadata.version] | ||
| # Automatically get version from git tags via setuptools_scm | ||
| provider = "scikit_build_core.metadata.setuptools_scm" | ||
|
|
||
| # Required to activate setuptools_scm for automatic versioning from git tags | ||
| [tool.setuptools_scm] | ||
|
|
||
|
|
||
|
|
||
| [project.optional-dependencies] | ||
|
|
||
| # Install with: pip install qc-gbasis[dev] | ||
|
|
||
| dev = [ | ||
| "tox", | ||
| "pre-commit", | ||
|
|
@@ -56,29 +82,36 @@ dev = [ | |
| "sphinx_autodoc_typehints", | ||
| "sphinx-copybutton", | ||
| ] | ||
|
|
||
| # Install with: pip install qc-gbasis[doc] | ||
|
|
||
| doc = [ | ||
| "numpydoc", | ||
| "sphinx_copybutton", | ||
| "sphinx-autoapi", | ||
| "nbsphinx", | ||
| "sphinx_rtd_theme", | ||
| "sphinx_autodoc_typehints", | ||
| "docutils==0.16", # Needed to show bullet points in sphinx_rtd_theme | ||
| "docutils==0.16", # Needed to show bullet points in sphinx_rtd_theme | ||
| "nbsphinx-link" | ||
| ] | ||
|
|
||
| # iodata = [ | ||
| # "qc-iodata@git+https://github.com/theochem/iodata.git@main" | ||
| # ] | ||
|
|
||
| # Install with: pip install qc-gbasis[iodata] | ||
|
|
||
| iodata = [ | ||
| "qc-iodata>=1.0.0a5" | ||
| ] | ||
|
|
||
| # Install with: pip install qc-gbasis[pyscf] | ||
|
|
||
| pyscf = [ | ||
| "pyscf>=1.6.1" | ||
| ] | ||
|
|
||
| [tool.setuptools.packages.find] | ||
| where = ["."] # list of folders that contain the packages (["."] by default) | ||
| include = ["gbasis"] # package names should match these glob patterns (["*"] by default) | ||
|
|
||
| [project.urls] | ||
| Documentation = "https://gbasis.qcdevs.org" | ||
|
|
@@ -112,7 +145,4 @@ addopts = "-v" | |
| #[tool.setuptools_scm] | ||
| #write_to = "src/gbasis/_version.py" | ||
| #version_scheme = "post-release" | ||
| #local_scheme = "no-local-version" | ||
|
|
||
| [tool.setuptools.package-data] | ||
| gbasis = ["integrals/include/cint*.h", "integrals/lib/libcint.so*"] | ||
| #local_scheme = "no-local-version" | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be addressed in the next PR which adds the platform-aware libcint shared library copying alongside the extension.