Use scikit build core#5
Open
jonaslb wants to merge 9 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates PSiesta’s build/install workflow from a legacy setup.py/distutils+Cython flow to a modern PEP 517 build using scikit-build-core + CMake, and adds smoke tests plus a Nix-based dev environment to make builds more reproducible.
Changes:
- Replace
setup.py/requirements.txtwithpyproject.toml(scikit-build-core) and a newCMakeLists.txtthat builds the_psiestaextension and (optionally) fetches/builds Siesta as a subproject. - Add a Nix flake development shell (plus lockfile) and update the README with the new build/test workflow.
- Add pytest smoke tests (import checks + simple H/H2 runs) and include a test pseudopotential fixture.
Reviewed changes
Copilot reviewed 10 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
CMakeLists.txt |
New CMake-based build that wires Cython + Fortran bindings and links against Siesta + MPI. |
pyproject.toml |
New PEP 517 build configuration using scikit-build-core; declares runtime/dev dependencies and pytest config. |
README.md |
Updates build + dev instructions for Nix + uv + CMake/scikit-build-core; documents smoke tests. |
flake.nix |
Adds a Nix dev shell with MPI, compilers, CMake/Ninja, and scientific dependencies. |
flake.lock |
Pins the Nix input revisions for the dev shell. |
.gitignore |
Ignores wheel/build artifacts, Python cache files, uv venv/lock, and CMake build outputs. |
tests/test_import.py |
Adds a basic import test for the compiled extension module. |
tests/test_mpi_import.py |
Adds a basic import test under MPI/mpi4py context. |
tests/test_calculations.py |
Adds “technical smoke” calculation tests (H atom, H2, rerun). |
tests/fixtures/H.psml |
Adds a hydrogen PSML pseudopotential fixture used by calculation tests. |
test/run.py |
Adds an ad-hoc performance/smoke runner script intended for manual use. |
setup.py |
Removes the legacy distutils-based build script. |
requirements.txt |
Removes the old requirements list (replaced by pyproject.toml). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,48 @@ | |||
| #!/usr/bin/env mpirun -n 4 python3 -m mpi4py | |||
| from mpi4py import MPI | ||
| import sisl as si | ||
| from psiesta import FilePSiesta | ||
| import numpy as np |
Comment on lines
+25
to
+26
| if rank == 0: | ||
| sp.run(['zstd', '-d', '-f', 'C.psf.zst']) |
| print(f"Time taken: {t1-t0}") | ||
| if "keep" not in sys.argv: | ||
| shutil.rmtree('tenbyten') | ||
| Path('C.psf').unlink() No newline at end of file |
Comment on lines
+15
to
+16
| set(PSIESTA_SIESTA_GIT_REPOSITORY "https://gitlab.com/siesta-project/siesta.git" CACHE STRING "Siesta git repository") | ||
| set(PSIESTA_SIESTA_GIT_TAG "master" CACHE STRING "Siesta git tag, branch, or commit") |
| #!/usr/bin/env mpirun -n 4 python3 -m mpi4py | ||
| """ | ||
| A "slow" test to see if it works and also judge performance a bit. | ||
| On a AMD 1600X this file (see shebang) runs in about : |
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.