Merge pull request #270 from loriab/windowsforeal

adapt Libint library for Windows
evaleev · Oct 12, 2023 · ca32e0c · ca32e0c
2 parents 5d5dd0b + e4af31f
commit ca32e0c
Show file tree

Hide file tree

Showing 18 changed files with 237 additions and 31 deletions.
diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml
@@ -1,9 +1,7 @@
-name: Linux/MacOS Build
+name: Build
 
 on: [push, pull_request]
 
-#env:
-
 jobs:
   build_repo:
     strategy:
@@ -17,7 +15,7 @@ jobs:
           - os: macos-latest
             cxx: clang++
 
-    name: "${{ matrix.os }}: ${{ matrix.cxx }} ${{ matrix.build_type }}"
+    name: "Repo • ${{ matrix.os }}: ${{ matrix.cxx }} ${{ matrix.build_type }}"
     runs-on: ${{ matrix.os }}
     env:
       CXX : ${{ matrix.cxx }}
@@ -37,11 +35,11 @@ jobs:
       should_skip: ${{ steps.skip_check.outputs.should_skip }}
 
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
 
     - id: skip_check
       name: Check if can skip
-      uses: fkirc/skip-duplicate-actions@v3.4.0
+      uses: fkirc/skip-duplicate-actions@v5
       with:
         cancel_others: 'true'
 
@@ -75,11 +73,11 @@ jobs:
       shell: cmake -P {0}
       run: |
         string(TIMESTAMP current_date "%Y-%m-%d-%H;%M;%S" UTC)
-        message("::set-output name=timestamp::${current_date}")
+        message("\"timestamp=${current_date}\" >> $GITHUB_OUTPUT")
 
     - name: Setup ccache cache files
       if: ${{ steps.skip_check.outputs.should_skip != 'true' }}
-      uses: actions/cache@v1.1.0
+      uses: actions/cache@v3
       with:
         path: ${{github.workspace}}/build/.ccache
         key: ${{ matrix.config.name }}-ccache-${{ steps.ccache_cache_timestamp.outputs.timestamp }}
@@ -162,7 +160,7 @@ jobs:
     # * turn off generator testing (comment out "make check" and "stdtests.pl" lines)
     # * suppress last two (test library and bindings) steps ("if: false")
     needs: build_repo
-    if: always() && (needs.build_repo.steps.skip_check.outputs.should_skip != 'true')
+    if: always() && (needs.build_repo.outputs.should_skip != 'true')
     strategy:
       fail-fast: false
       matrix:
@@ -175,6 +173,20 @@ jobs:
             # note full paths depend on setup-miniconda:
             # * Miniforge is miniconda3 vs. Miniconda is miniconda
 
+          - runs-on: windows-latest
+            lane: windows-clang-cl
+            libargs: >
+              -GNinja
+              -DCMAKE_BUILD_TYPE=Release
+              -DBUILD_SHARED_LIBS=OFF
+              -DCMAKE_CXX_COMPILER=clang-cl
+              -DCMAKE_C_COMPILER=clang-cl
+            testargs: >
+              -GNinja
+              -DCMAKE_BUILD_TYPE=Release
+              -DCMAKE_CXX_COMPILER=clang-cl
+              -DCMAKE_C_COMPILER=clang-cl
+
           - runs-on: macos-latest
             lane: macos-clang
             libargs: >
@@ -185,14 +197,12 @@ jobs:
             lane: ubuntu-intel
             libargs: >
               -DCMAKE_CXX_COMPILER=icpx
-              -DCMAKE_CXX_FLAGS="--gcc-toolchain=/usr/share/miniconda3/envs/test --sysroot=/usr/share/miniconda3/envs/test/x86_64-conda-linux-gnu/sysroot -target x86_64-conda-linux-gnu"
-              # flags are long-form of below (envvars not available at spinup)
-              # -DCMAKE_CXX_FLAGS="--gcc-toolchain=${CONDA_PREFIX} --sysroot=${CONDA_PREFIX}/${HOST}/sysroot -target ${HOST}"
+              -DCMAKE_CXX_FLAGS="--gcc-toolchain=${CONDA_PREFIX} --sysroot=${CONDA_PREFIX}/${HOST}/sysroot -target ${HOST}"
             testargs: >
               -DCMAKE_CXX_COMPILER=icpx
-              -DCMAKE_CXX_FLAGS="--gcc-toolchain=/usr/share/miniconda3/envs/test --sysroot=/usr/share/miniconda3/envs/test/x86_64-conda-linux-gnu/sysroot -target x86_64-conda-linux-gnu"
+              -DCMAKE_CXX_FLAGS="--gcc-toolchain=${CONDA_PREFIX} --sysroot=${CONDA_PREFIX}/${HOST}/sysroot -target ${HOST}"
 
-    name: "Export • ${{ matrix.cfg.lane }} • ${{ matrix.cfg.libargs }}"
+    name: "Export • ${{ matrix.cfg.lane }}"
     runs-on: ${{ matrix.cfg.runs-on }}
 
     steps:
@@ -217,6 +227,9 @@ jobs:
           - pybind11
           #- dpcpp_linux-64
         EOF
+        if [[ "${{ runner.os }}" == "Windows" ]]; then
+          sed -i "s/- cxx/#- cxx/g" export.yaml
+        fi
         if [[ "${{ matrix.cfg.lane }}" == "ubuntu-intel" ]]; then
           sed -i "s/#- dpcpp_linux-64/- dpcpp_linux-64/g" export.yaml
         fi
@@ -239,6 +252,12 @@ jobs:
         conda info
         conda list
 
+    - name: Prepare compiler environment for Windows
+      if: ${{ runner.os == 'Windows' }}
+      uses: ilammy/msvc-dev-cmd@v1
+      with:
+        arch: x64
+
     - uses: actions/download-artifact@v3
       with:
         name: Linux-g++-10
@@ -251,21 +270,28 @@ jobs:
         cmake \
           -S. \
           -Bbuild \
+          -GNinja \
           -DCMAKE_INSTALL_PREFIX="${{github.workspace}}/installed" \
           -DCMAKE_CXX_COMPILER=${CXX} \
           -DLIBINT2_PYTHON=ON \
           -DCMAKE_PREFIX_PATH="${CONDA_PREFIX}" \
           ${{ matrix.cfg.libargs }}
         cmake --build build --target install libint2-python-test
 
-    - name: Test Installed Libint library
+    - name: Test Libint library - unit tests
+      shell: bash -l {0}
+      working-directory: ${{github.workspace}}/libint/build
+      run: |
+        cmake --build . --target check
+
+    - name: Test Libint library - consume installation for SCF
       shell: bash -l {0}
       run: |
         mkdir test_installed_library && cd test_installed_library
         cat > CMakeLists.txt <<EOF
-        cmake_minimum_required(VERSION 3.16)
+        cmake_minimum_required(VERSION 3.8)
         project(hf++)
-        find_package(Libint2 2.7.2 REQUIRED COMPONENTS eri_c4_d0_l2 CXX_ho)
+        find_package(Libint2 2.8.0 REQUIRED)
         get_target_property(_l2_maxam Libint2::int2 Libint2_MAX_AM_ERI)
         message("Libint2_MAX_AM_ERI \${_l2_maxam}")
         find_package(Threads)  # clang does not autolink threads even though we are using std::thread
@@ -276,11 +302,11 @@ jobs:
         cmake --build build --target hf++
         ./build/hf++ ../libint/tests/hartree-fock/h2o_rotated.xyz | python ../libint/tests/hartree-fock/hartree-fock++-validate.py ../libint/MakeVars.features
 
-    - name: Build (again) & Test Python bindings
+    - name: Build & Test Python bindings (again, independently)
       shell: bash -l {0}
       working-directory: ${{github.workspace}}/libint/python
       run: |
-        cmake . -D LIBINT2_PYTHON=ON -D CMAKE_PREFIX_PATH="${{github.workspace}}/installed" ${{ matrix.cfg.testargs }}
+        cmake . -D CMAKE_PREFIX_PATH="${{github.workspace}}/installed" ${{ matrix.cfg.testargs }}
         cmake --build . --target libint2-python
         cmake --build . --target libint2-python-test
 
diff --git a/CHANGES b/CHANGES
@@ -4,6 +4,21 @@
 Following is a brief summary of changes made in each release of Libint.
 
 - 2022-xx-yy: 2.8.0-beta.1
+  - UNMERGED PR #270: For Windows, basis sets with a star have been renamed to "s" on the filesystem,
+    so 6-31g**.g94 -> 6-31gss.g94. In code, the basis can be accessed through "6-31g**" (longstanding)
+    or "6-31gss" (new) for all operating systems.
+  - UNMERGED PR #270: Adapt build system and header imports so that library and Python bindings can build on
+    Windows (at least with clang-cl compiler atop MSVC). Note that a Linux- or Mac-generated export
+    builds on Windows; one cannot generate an export on Windows. Note also that only a static library
+    build, not a shared one, works on Windows (see #237).
+  - PR #269: minimum CMake bumped to 3.16
+  - PR #269: Solid harmonics ordering is runtime switchable in the library. Issue
+    `libint2::set_solid_harmonics_ordering(libint2::SHGShellOrdering_Gaussian)` or `_Standard` after
+    initialization. Similarly, prefer new `INT_SOLIDHARMINDEX(sho, l, m)` to usual `(l, m)` version.
+  - PR #268: Python detection (relevant to Python bindings, Fortran, and some tests) now uses modern
+    `find_package(Python)`. Specify with `Python_EXECUTABLE` (note change in case) or
+    https://cmake.org/cmake/help/latest/module/FindPython.html for details .
+  - PR #268: Python bindings can now use Pybind11 >=2.6 and can use detected installation.
   - PR #249: Python bindings (HT @asadchev)
   - PR #246: BasisSet is no longer derived from vector<Shell>
   - PR #232: introduced new primitive screening methods

diff --git a/export/cmake/CMakeLists.txt.export b/export/cmake/CMakeLists.txt.export
@@ -205,12 +205,29 @@ if(BUILD_SHARED_LIBS OR LIBINT2_BUILD_SHARED_AND_STATIC_LIBS OR LIBINT2_PYTHON)
   set_target_properties(libint2_obj PROPERTIES POSITION_INDEPENDENT_CODE ON)
 endif()
 
+if (MSVC)
+    set_target_properties(
+      libint2_obj
+      PROPERTIES
+        # Increase stack size from 1 MB to 4 MB
+        LINK_FLAGS "/STACK:4194304"
+      )
+endif()
+
 # shared and static libraries built from the same object files
 if (LIBINT2_BUILD_SHARED_AND_STATIC_LIBS)
   add_library(libint2 SHARED $<TARGET_OBJECTS:libint2_obj>)
   set_target_properties(libint2 PROPERTIES LIBRARY_OUTPUT_NAME int2)
+  if(MSVC)
+    target_compile_definitions(libint2 PUBLIC _USE_MATH_DEFINES)
+    target_compile_options(libint2 PUBLIC "/EHsc")
+  endif()
   add_library(libint2-static STATIC $<TARGET_OBJECTS:libint2_obj>)
   set_target_properties(libint2-static PROPERTIES ARCHIVE_OUTPUT_NAME int2)
+  if(MSVC)
+    target_compile_definitions(libint2-static PUBLIC _USE_MATH_DEFINES)
+    target_compile_options(libint2-static PUBLIC "/EHsc")
+  endif()
   target_include_directories(libint2-static INTERFACE
         $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include> $<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/include>
         $<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/include/libint2>
@@ -265,10 +282,18 @@ else()
 endif()
 
 # install basis set library
-install(DIRECTORY ${PROJECT_SOURCE_DIR}/lib/basis
+if(MSVC)
+    install(DIRECTORY ${PROJECT_SOURCE_DIR}/lib/basis
+        COMPONENT libint2
+        DESTINATION "${LIBINT2_INSTALL_DATADIR}"
+        FILES_MATCHING REGEX "[A-Za-z0-9_\\(\\)-]+\.g94"
+        )
+else()
+    install(DIRECTORY ${PROJECT_SOURCE_DIR}/lib/basis
         COMPONENT libint2
         DESTINATION "${LIBINT2_INSTALL_DATADIR}"
         )
+endif()
 
 # LibintCXX library ====================================================================================================
 
@@ -279,10 +304,40 @@ if (LIBINT_HAS_CXX_API)
   if (LIBINT_HAS_SYSTEM_BOOST_PREPROCESSOR_VARIADICS)
       target_link_libraries(libint2_cxx INTERFACE Boost::boost)
   endif(LIBINT_HAS_SYSTEM_BOOST_PREPROCESSOR_VARIADICS)
+    if(MSVC)
+        # MSVC does not include <cmath> constants, unless _USE_MATH_DEFINES is defined.
+        # _CRT_* to squash some getenv, strdup, strncpy, ctime, fopen warnings
+        target_compile_definitions(
+          libint2_cxx
+          INTERFACE
+            _USE_MATH_DEFINES
+            _CRT_NONSTDC_NO_DEPRECATE
+            _CRT_NONSTDC_NO_WARNINGS
+            _CRT_SECURE_NO_WARNINGS
+          )
+        # Set the exception handling model
+        target_compile_options(
+          libint2_cxx
+          INTERFACE
+            "/EHsc"
+          )
+    endif()
   get_filename_component(DATADIR_ABSOLUTE "${CMAKE_INSTALL_PREFIX}/${LIBINT2_INSTALL_DATADIR}" ABSOLUTE)
   target_compile_definitions(libint2_cxx INTERFACE
           $<BUILD_INTERFACE:SRCDATADIR="${PROJECT_SOURCE_DIR}/lib/basis">
-          $<INSTALL_INTERFACE:DATADIR="${DATADIR_ABSOLUTE}">)
+    )
+  if (NOT MSVC)
+      # TODO fix the DATADIR define escaping on Windows
+      # * below works fine in tests
+      # * but fails in Psi4 compile
+      # * prefix replacement in conda used instead on Windows
+      # * LIBINT2_INSTALL_DATADIR -> LIBINT2_INSTALL_BASISDIR
+      target_compile_definitions(
+        libint2_cxx
+        INTERFACE
+          $<INSTALL_INTERFACE:DATADIR="\$\{_IMPORT_PREFIX\}/${LIBINT2_INSTALL_DATADIR}">
+        )
+  endif()
   # Add library to the list of installed components
   install(TARGETS libint2_cxx EXPORT libint2
           COMPONENT cxx
@@ -303,6 +358,11 @@ add_custom_target_subproject(libint2 check USES_TERMINAL COMMAND ${CMAKE_CTEST_C
 add_executable(eritest-libint2 EXCLUDE_FROM_ALL tests/eri/test.cc)
 target_link_libraries(eritest-libint2 ${int2_library})
 target_include_directories(eritest-libint2 PRIVATE ${PROJECT_SOURCE_DIR}/tests/eri)
+if(MSVC)
+    # TODO on future target pass, def should be added to the L2 target, not added to test
+    target_compile_definitions(eritest-libint2 PUBLIC _USE_MATH_DEFINES)
+    target_compile_options(eritest-libint2 PUBLIC "/EHsc")
+endif()
 
 add_test(libint2/eritest/build "${CMAKE_COMMAND}" --build ${CMAKE_BINARY_DIR} --target eritest-libint2)
 set_tests_properties(libint2/eritest/build PROPERTIES FIXTURES_SETUP LIBINT2_ERITEST_EXEC)

diff --git a/include/libint2/basis.h.in b/include/libint2/basis.h.in
@@ -36,7 +36,14 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
+#ifdef _MSC_VER
+#include <io.h>
+#define S_ISDIR(m) (((m)&S_IFMT) == S_IFDIR)
+#define PATH_SEPARATOR "\\"
+#else
 #include <unistd.h>
+#define PATH_SEPARATOR "/"
+#endif
 
 #include <libint2.h>
 #include <libint2/shell.h>
@@ -139,7 +146,7 @@ namespace libint2 {
 
         // read in ALL basis set components
         for(const auto& basis_component_name: basis_component_names) {
-          auto file_dot_g94 = basis_lib_path + "/" + basis_component_name + ".g94";
+          auto file_dot_g94 = basis_lib_path + PATH_SEPARATOR + basis_component_name + ".g94";
 
           // use same cartesian_d convention for all components!
           component_basis_sets.emplace_back(read_g94_basis_library(file_dot_g94, force_cartesian_d, throw_if_no_match));
@@ -161,7 +168,7 @@ namespace libint2 {
             }
             else if (throw_if_no_match) {  // not found? throw, if needed
               std::string errmsg(std::string("did not find the basis for this Z in ") +
-                      basis_lib_path + "/" + basis_component_names[comp_idx] + ".g94");
+                      basis_lib_path + PATH_SEPARATOR + basis_component_names[comp_idx] + ".g94");
               throw std::logic_error(errmsg);
             }
           } // basis component loop
@@ -338,6 +345,9 @@ namespace libint2 {
             char cc = ::tolower(c);
             switch (cc) {
               case '/': cc = 'I'; break;
+#ifdef _MSC_VER
+              case '*': cc = 's'; break;
+#endif
             }
             return cc;
           }
@@ -411,7 +421,7 @@ namespace libint2 {
 #endif
         }
         // validate basis_path = path + "/basis"
-        std::string basis_path = path + std::string("/basis");
+        std::string basis_path = path + PATH_SEPARATOR + std::string("basis");
         bool error = true;
         std::error_code ec;
         auto validate_basis_path = [&basis_path, &error, &ec]() -> void {
@@ -459,7 +469,12 @@ namespace libint2 {
       static std::vector<std::vector<libint2::Shell>> read_g94_basis_library(std::string file_dot_g94,
                                                                              bool force_cartesian_d = false,
                                                                              bool throw_if_missing = true,
-                                                                             std::string locale_name = std::string("POSIX")) {
+#ifdef _MSC_VER
+                                                                             std::string locale_name = std::string("en-US")
+#else
+                                                                             std::string locale_name = std::string("POSIX")  // "en_US"
+#endif
+                                                                            ) {
 
         std::locale locale(locale_name.c_str());  // TODO omit c_str() with up-to-date stdlib
         std::vector<std::vector<libint2::Shell>> ref_shells(118); // 118 = number of chemical elements

diff --git a/include/libint2/boys.h b/include/libint2/boys.h
@@ -36,6 +36,13 @@
 #include <type_traits>
 #include <memory>
 
+#ifdef _MSC_VER
+#define posix_memalign(p, a, s) (((*(p)) = _aligned_malloc((s), (a))), *(p) ?0 :errno)
+#define posix_memfree(p) ((_aligned_free((p))))
+#else
+#define posix_memfree(p) ((free((p))))
+#endif
+
 // from now on at least C++11 is required by default
 #include <libint2/util/cxxstd.h>
 #if LIBINT2_CPLUSPLUS_STD < 2011
@@ -290,7 +297,7 @@ namespace libint2 {
       }
       ~FmEval_Chebyshev7() {
         if (mmax >= 0) {
-          free(c);
+          posix_memfree(c);
         }
       }
 
@@ -881,7 +888,7 @@ namespace libint2 {
 
       ~TennoGmEval() {
         if (c_ != nullptr)
-          free(c_);
+          posix_memfree(c_);
       }
 
       /// Singleton interface allows to manage the lone instance; adjusts max m values as needed in thread-safe fashion
@@ -1313,7 +1320,7 @@ namespace libint2 {
 
         // get memory
         void* result;
-        int status = posix_memalign(&result, std::max(sizeof(Real), 32ul), (mmax_ - mmin_ + 1) * cheb_table_nintervals * ORDERp1 * ORDERp1 * sizeof(Real));
+        int status = posix_memalign(&result, std::max(sizeof(Real), (size_t)32), (mmax_ - mmin_ + 1) * cheb_table_nintervals * ORDERp1 * ORDERp1 * sizeof(Real));
         if (status != 0) {
           if (status == EINVAL)
             throw std::logic_error(