Merge develop to main

.github/workflows/ci.yaml

@@ -3,37 +3,40 @@ name: CI
 on: [pull_request, workflow_dispatch]
 
 jobs:
+  pre-commit:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Clone repo
+      uses: actions/checkout@v6
+    - name: Set up uv
+      uses: astral-sh/setup-uv@v8.3.2
+    - name: Run pre-commit
+      run: uvx pre-commit run --all-files
 
   symmetrix:
     runs-on: ubuntu-latest
     steps:
     - name: Clone repo
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
       with:
         submodules: recursive
     - name: Install dependencies
       run: |
-        sudo apt-get install -y build-essential cmake git libblas-dev liblapack-dev
-    - name: Set up python
-      uses: actions/setup-python@v5
+        sudo apt-get update
+        sudo apt-get install -y build-essential cmake git libblas-dev liblapack-dev ninja-build
+    - name: Set up uv
+      uses: astral-sh/setup-uv@v8.3.2
       with:
         python-version: '3.12'
-    - name: Create python venv
+    - name: Create python venv and install
       run: |
-        python -m venv venv
-        source venv/bin/activate
-        pip install ase cmake-build-extension[all] numpy pytest setuptools scipy wheel cmake-build-extension[all]
-        deactivate
-    - name: Build and package symmetrix
-      run: |
-        source venv/bin/activate
-        cd symmetrix
-        pip install .
-        cd ..
+        uv venv --clear
+        source .venv/bin/activate
+        uv pip install ./symmetrix[test]
         deactivate
     - name: Run tests
       run: |
-        source venv/bin/activate
+        source .venv/bin/activate
         python -c "import symmetrix; print(symmetrix.__version__)"
         cd symmetrix/test
         pytest
@@ -44,25 +47,26 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - name: Clone repo
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
       with:
         submodules: recursive
     - name: Install dependencies
       run: |
+        sudo apt-get update
         sudo apt-get install -y build-essential cmake git libfftw3-dev libopenmpi-dev mpi-default-bin mpi-default-dev libblas-dev liblapack-dev
-    - name: Set up python
-      uses: actions/setup-python@v5
+    - name: Set up uv
+      uses: astral-sh/setup-uv@v8.3.2
       with:
         python-version: '3.12'
-    - name: Create python venv
+    - name: Create python venv and install
       run: |
-        python -m venv venv
-        source venv/bin/activate
-        pip install numpy pytest
+        uv venv --clear
+        source .venv/bin/activate
+        uv pip install pip numpy pytest
         deactivate
     - name: Clone and build LAMMPS
       run: |
-        source venv/bin/activate
+        source .venv/bin/activate
         git clone -b release --depth 1 https://github.com/lammps/lammps.git
         cd pair_symmetrix
         chmod +x install.sh
@@ -82,13 +86,13 @@ jobs:
             -D Kokkos_ENABLE_AGGRESSIVE_VECTORIZATION=ON \
             -D SYMMETRIX_KOKKOS=ON \
             cmake
-        cmake --build build -j 2
+        cmake --build build -j 4
         cd build
         make install-python
         cd ../../..
     - name: Run tests
       run: |
-        source venv/bin/activate
+        source .venv/bin/activate
         cd pair_symmetrix/test/
         python -m pytest
 
@@ -97,42 +101,39 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        mace-torch: ["mace-torch==0.3.10", "mace-torch"]
+        include:
+        - mace-torch: "mace-torch==0.3.10"
+          torch: "torch==2.5.*"
+        - mace-torch: "mace-torch"
+          torch: "torch"
     steps:
     - name: Clone repo
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
       with:
         submodules: recursive
     - name: Install dependencies
       run: |
-        sudo apt-get install -y build-essential cmake git libblas-dev liblapack-dev
-    - name: Set up python
-      uses: actions/setup-python@v5
+        sudo apt-get update
+        sudo apt-get install -y build-essential cmake git libblas-dev liblapack-dev ninja-build
+    - name: Set up uv
+      uses: astral-sh/setup-uv@v8.3.2
       with:
         python-version: '3.12'
-    - name: Create python venv
-      run: |
-        python -m venv venv
-        source venv/bin/activate
-        pip install ${{ matrix.mace-torch }}
-        deactivate
-    - name: Build and package symmetrix
+    - name: Create python venv and install
       run: |
-        source venv/bin/activate
-        cd symmetrix
-        pip install .
-        cd ..
+        uv venv --clear
+        source .venv/bin/activate
+        uv pip install "${{ matrix.torch }}" "${{ matrix.mace-torch }}" "./symmetrix[mace]"
         deactivate
     - name: Test model extraction
       run: |
-        source venv/bin/activate
-        pip install mace-torch
+        source .venv/bin/activate
         # check for valid symmetrix json from atomic numbers and default filename
-        wget https://github.com/ACEsuit/mace-off/raw/refs/heads/main/mace_off23/MACE-OFF23_small.model 
+        wget https://github.com/ACEsuit/mace-off/raw/refs/heads/main/mace_off23/MACE-OFF23_small.model
         symmetrix_extract_mace --model MACE-OFF23_small.model --atomic-numbers 1 8
         python3 -c "from symmetrix import Symmetrix; calc = Symmetrix('MACE-OFF23_small-1-8.json', species=[1, 8])"
         # check for valid symmetrix json from checmical symbols and specified filename
-        wget https://github.com/ACEsuit/mace-off/raw/refs/heads/main/mace_off23/MACE-OFF23_medium.model 
+        wget https://github.com/ACEsuit/mace-off/raw/refs/heads/main/mace_off23/MACE-OFF23_medium.model
         symmetrix_extract_mace --model MACE-OFF23_medium.model --chemical-symbols H O -o extract_test.json
         python3 -c "from symmetrix import Symmetrix; calc = Symmetrix('extract_test.json', species=[1, 8])"
         deactivate

.pre-commit-config.yaml

@@ -0,0 +1,15 @@
+repos:
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.5.0
+    hooks:
+    -   id: trailing-whitespace
+    -   id: end-of-file-fixer
+    -   id: check-yaml
+    -   id: check-added-large-files
+
+-   repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.3.0
+    hooks:
+    -   id: ruff
+        args: [ --fix ]
+    -   id: ruff-format

README.md

@@ -15,6 +15,18 @@ See the `symmetrix` [README](symmetrix/README.md) to build and use the Python pa
 
 See the `pair_symmetrix` [README](pair_symmetrix/README.md) for use from LAMMPS.
 
+### Development Setup
+
+Use `uv` to create a virtual environment and install the package with its test dependencies:
+
+```bash
+uv venv
+source .venv/bin/activate
+uv pip install -e "./symmetrix[test]"
+```
+
+Run Python formatting and lint checks with `uvx pre-commit run --all-files`.
+
 ### Citing Symmetrix
 
 The earliest `symmetrix` results are reported in:

libsymmetrix/CMakeLists.txt

@@ -74,4 +74,8 @@ if (SYMMETRIX_SPHERICART_CUDA)
     message(STATUS "Symmetrix: Will use CUDA version of sphericart.")
     target_compile_definitions(symmetrix PRIVATE SYMMETRIX_SPHERICART_CUDA)
 endif()
-
+option(SYMMETRIX_SPHERICART_SYCL OFF)
+if (SYMMETRIX_SPHERICART_SYCL)
+    message(STATUS "Symmetrix: Will use SYCL version of sphericart.")
+    target_compile_definitions(symmetrix PRIVATE SYMMETRIX_SPHERICART_SYCL)
+endif()

libsymmetrix/source/cubic_spline.cpp

@@ -1,5 +1,8 @@
 #include <stdexcept>
 #include <vector>
+#include <string>
+#include <algorithm>
+#include <cmath>
 
 #include "cubic_spline.hpp"
 
@@ -14,10 +17,9 @@ CubicSpline::CubicSpline(
 
 double CubicSpline::evaluate(double r)
 {
-    const int i = static_cast<int>(r / h);
-    // TODO: something better with this bounds checking
-    if (i<0 or i>=c.size()/4)
-        throw std::invalid_argument("Out of bounds in CubicSpline::evaluate.");
+    if (r<0 or r>h*c.size()/4 or std::isnan(r))
+        throw std::invalid_argument("Out of bounds in CubicSpline::evaluate. r=" + std::to_string(r));
+    const int i = std::clamp(static_cast<int>(r / h), 0, static_cast<int>(c.size()/4 - 1));
     const double x = r - h*i;
     const double xx = x*x;
     const double xxx = xx*x;
@@ -28,10 +30,9 @@ double CubicSpline::evaluate(double r)
 
 std::tuple<double,double> CubicSpline::evaluate_deriv(double r)
 {
-    const int i = static_cast<int>(r / h);
-    // TODO: something better with this bounds checking
-    if (i<0 or i>=c.size()/4)
-        throw std::invalid_argument("Out of bounds in CubicSpline::evaluate_deriv.");
+    if (r<0 or r>h*c.size()/4 or std::isnan(r))
+        throw std::invalid_argument("Out of bounds in CubicSpline::evaluate_deriv. r=" + std::to_string(r));
+    const int i = std::clamp(static_cast<int>(r / h), 0, static_cast<int>(c.size()/4 - 1));
     const double x = r - h*i;
     const double xx = x*x;
     const double xxx = xx*x;
@@ -42,10 +43,9 @@ std::tuple<double,double> CubicSpline::evaluate_deriv(double r)
 
 std::tuple<double,double> CubicSpline::evaluate_deriv_divided(double r)
 {
-    const int i = static_cast<int>(r / h);
-    // TODO: something better with this bounds checking
-    if (i<0 or i>=c.size()/4)
-        throw std::invalid_argument("Out of bounds in CubicSpline::evaluate_deriv.");
+    if (r<=0 or r>h*c.size()/4 or std::isnan(r))
+        throw std::invalid_argument("Out of bounds in CubicSpline::evaluate_deriv_divided. r=" + std::to_string(r));
+    const int i = std::clamp(static_cast<int>(r / h), 0, static_cast<int>(c.size()/4 - 1));
     const double x = r - h*i;
     const double xx = x*x;
     const double xxx = xx*x;
@@ -60,6 +60,11 @@ auto CubicSpline::generate_coefficients(
     std::vector<double> nodal_derivs)
     -> std::vector<double>
 {
+    if (h<=0 or not std::isfinite(h))
+        throw std::invalid_argument("CubicSpline requires positive finite spacing.");
+    if (nodal_values.size()<2 or nodal_values.size()!=nodal_derivs.size())
+        throw std::invalid_argument("CubicSpline requires at least two values and matching derivatives.");
+
     auto c = std::vector<double>(4*(nodal_values.size()-1), 0.0);
     for (int i=0; i<nodal_values.size()-1; ++i) {
         c[4*i] = nodal_values[i];

libsymmetrix/source/cubic_spline_kokkos.cpp

@@ -1,14 +1,21 @@
 #include <stdexcept>
 #include<iostream>
 #include<cmath>
+#include<string>
+#include<algorithm>
 #include "cubic_spline_kokkos.hpp"
 
 CubicSplineKokkos::CubicSplineKokkos(
     double h,
     std::vector<double> nodal_values,
     std::vector<double> nodal_derivs)
-    : h(h), num_coeffs(4*(nodal_values.size() - 1))
+    : h(h)
 {
+    if (h<=0 or not std::isfinite(h))
+        throw std::invalid_argument("CubicSplineKokkos requires positive finite spacing.");
+    if (nodal_values.size()<2 or nodal_values.size()!=nodal_derivs.size())
+        throw std::invalid_argument("CubicSplineKokkos requires at least two values and matching derivatives.");
+    num_coeffs = 4*(nodal_values.size() - 1);
     c = Kokkos::View<double*>("coeffs",num_coeffs);
     generate_coefficients(h, nodal_values, nodal_derivs);
 }
@@ -17,26 +24,30 @@ CubicSplineKokkos::CubicSplineKokkos(
     double h,
     Kokkos::View<double*> nodal_values,
     Kokkos::View<double*> nodal_derivs)
-    : h(h), num_coeffs(4*(nodal_values.size() - 1))
+    : h(h)
 {
+    if (h<=0 or not std::isfinite(h))
+        throw std::invalid_argument("CubicSplineKokkos requires positive finite spacing.");
+    if (nodal_values.size()<2 or nodal_values.size()!=nodal_derivs.size())
+        throw std::invalid_argument("CubicSplineKokkos requires at least two values and matching derivatives.");
+    num_coeffs = 4*(nodal_values.size() - 1);
     c = Kokkos::View<double*>("coeffs",num_coeffs);
     generate_coefficients(h, nodal_values, nodal_derivs);
 }
 
 double CubicSplineKokkos::evaluate(double r)
 {
-    const int i = static_cast<int>(r / h);
-    // TODO: something better with this bounds checking
-    if (i < 0 || i >= num_coeffs / 4)
-        throw std::invalid_argument("Out of bounds in CubicSplineKokkos::evaluate.");
-
+    if (r<0 or r>h*num_coeffs/4 or std::isnan(r))
+        throw std::invalid_argument("Out of bounds in CubicSplineKokkos::evaluate. r=" + std::to_string(r));
+    const int i = std::clamp(static_cast<int>(r / h), 0, static_cast<int>(num_coeffs/4 - 1));
+
     const double x = r - h * i;
     const double xx = x * x;
     const double xxx = xx * x;
     const int i4 = 4 * i;
 
     auto h_c = Kokkos::create_mirror_view(c);
-    
+
     double ret = 0;
     const double c0 = h_c(i4);
     const double c1 = h_c(i4 + 1);
@@ -49,10 +60,9 @@ double CubicSplineKokkos::evaluate(double r)
 
 std::tuple<double, double> CubicSplineKokkos::evaluate_deriv(double r)
 {
-    const int i = static_cast<int>(r / h);
-    // TODO: something better with this bounds checking
-    if (i < 0 || i > num_coeffs / 4)
-        throw std::invalid_argument("Out of bounds in CubicSplineKokkos::evaluate_deriv.");
+    if (r<0 or r>h*num_coeffs/4 or std::isnan(r))
+        throw std::invalid_argument("Out of bounds in CubicSplineKokkos::evaluate_deriv. r=" + std::to_string(r));
+    const int i = std::clamp(static_cast<int>(r / h), 0, static_cast<int>(num_coeffs/4 - 1));
 
     const double x = r - h * i;
     const double xx = x * x;
@@ -74,10 +84,9 @@ std::tuple<double, double> CubicSplineKokkos::evaluate_deriv(double r)
 
 std::tuple<double,double> CubicSplineKokkos::evaluate_deriv_divided(double r)
 {
-    const int i = static_cast<int>(r / h);
-    // TODO: something better with this bounds checking
-    if (i<0 or i> num_coeffs)
-        throw std::invalid_argument("Out of bounds in CubicSplineKokkos::evaluate_deriv.");
+    if (r<=0 or r>h*num_coeffs/4 or std::isnan(r))
+        throw std::invalid_argument("Out of bounds in CubicSplineKokkos::evaluate_deriv_divided. r=" + std::to_string(r));
+    const int i = std::clamp(static_cast<int>(r / h), 0, static_cast<int>(num_coeffs/4 - 1));
 
     const double x = r - h*i;
     const double xx = x*x;
@@ -90,7 +99,7 @@ std::tuple<double,double> CubicSplineKokkos::evaluate_deriv_divided(double r)
     const double c1 = h_c(i4+1);
     const double c2=h_c(i4+2);
     const double c3=h_c(i4+3);
-    
+
     return {c0 + c1*x + c2*xx + c3*xxx, (c1 + 2*c2*x + 3*c3*xx) / r};
 }
 

libsymmetrix/source/cubic_spline_kokkos.hpp

@@ -15,7 +15,7 @@ CubicSplineKokkos(double h,
 CubicSplineKokkos(double h,
                   Kokkos::View<double*> nodal_values,
                   Kokkos::View<double*> nodal_derivs);
-            
+
 double evaluate(double r);
 std::tuple<double,double> evaluate_deriv(double r);
 std::tuple<double,double> evaluate_deriv_divided(double r);