Enhance Windows compatibility and update CI configurations#212
Enhance Windows compatibility and update CI configurations#212hkmoon wants to merge 176 commits intobertiniteam:developfrom
Conversation
There are some compiling errors while being compiled in windows * `size_t` is translated into `unsigned long` in linux, mac while `unsigned long long` in windows 10: `core/include/bertini2/eigen_extensions.hpp` and `core/test/classes/start_system_test.cpp` are modified * use `clang` of LLVM in Windows since MSVC has different compiling way for `template` * use `--no-isolation` for `scikit-build` in Windows For linux wheel naming convention, we cannot use x86_64, x86_i386 anymore for pypi repository. https://peps.python.org/pep-0600/ * use `auditwheel` for it Co-authored-by: HongKee Moon <moon@mpi-cbg.de>
the default branch is changed to 'develop'
additionally, fix ctest folder
additionally, fix ctest folder
* chore: test for win32 * chore: use Release libraries for linking in windows * chore: use 1e-14 instead of 1e-15 * 1e-15 doesn't work in windows * chore: remove docker-compose.yml * chore: revert test list --------- Co-authored-by: HongKee Moon <moon@mpi-cbg.de>
There was a problem hiding this comment.
Pull request overview
This PR updates the project’s packaging/build system to improve cross-platform (especially Windows) builds and adds automation for building and publishing Python wheels/releases.
Changes:
- Added a GitHub Actions workflow to build wheels on Linux/macOS/Windows and publish to TestPyPI/PyPI + GitHub Releases.
- Updated CMake configuration and custom Find modules to improve Windows builds/linking and modernize dependency discovery.
- Updated Python packaging metadata/versioning and added Windows DLL search-path handling for runtime imports.
Reviewed changes
Copilot reviewed 24 out of 28 changed files in this pull request and generated 30 comments.
Show a summary per file
| File | Description |
|---|---|
python_bindings/include/mpfr_export.hpp |
Whitespace/formatting-only adjustments. |
python_bindings/CMakeLists.txt |
Windows-oriented build/link changes; Boost/Python/Eigen handling updates. |
python/examples/solve_system.py |
Fixes indentation/formatting in example. |
python/bertini/windows_dll_manager.py |
New helper for locating/adding DLL directories on Windows. |
python/bertini/_version.py |
Bumps package version to 1.0.9. |
python/bertini/__init__.py |
Adds Windows DLL directory setup during import. |
pyproject.toml |
Renames project to pybertini, updates version and Python version range, adjusts scikit-build settings. |
environment.yml |
Adds a conda environment spec (Linux). |
environment-win.yml |
Adds a conda environment spec (Windows). |
core/test/classic/classic_test.cpp |
Disables BOOST_TEST_DYN_LINK define. |
core/test/classes/start_system_test.cpp |
Adjusts tests for Windows size_t/literal portability; formatting changes. |
core/test/classes/class_test.cpp |
Disables BOOST_TEST_DYN_LINK define; adjusts tolerance. |
core/test/blackbox/blackbox.cpp |
Disables BOOST_TEST_DYN_LINK define. |
core/include/bertini2/trackers/base_tracker.hpp |
Whitespace/formatting-only adjustments. |
core/include/bertini2/system/system.hpp |
Exposes EIGEN_MAKE_ALIGNED_OPERATOR_NEW under public. |
core/include/bertini2/system/straight_line_program.hpp |
Attempts to gate precision checks for Windows; formatting tweaks. |
core/include/bertini2/mpfr_extensions.hpp |
Removes Eigen 3.2.x workaround comment/code and documents Eigen>=3.3 requirement. |
core/include/bertini2/logging.hpp |
Disables BOOST_LOG_DYN_LINK define. |
core/include/bertini2/endgames/cauchy.hpp |
Large comment/whitespace formatting adjustments. |
core/include/bertini2/eigen_extensions.hpp |
Replaces uint with unsigned int in APIs for portability. |
core/CMakeLists.txt |
Substantial build system updates: MSVC flags, Boost/Eigen discovery, install rules, test definitions. |
cmake/FindMPFR.cmake |
Updates Windows library name search. |
cmake/FindMPC.cmake |
Updates Windows library name search. |
cmake/FindGMP.cmake |
Updates Windows library name search. |
README.md |
Updates installation instructions focusing on pybertini wheels and platforms. |
CMakeLists.txt |
Refactors top-level CMake to use jrl-cmakemodules and related configuration. |
CHANGELOG.md |
Adds a changelog documenting release preparations and platform notes. |
.github/workflows/build-and-publish-to-pypi.yml |
New CI workflow for building/publishing wheels and creating signed GitHub releases. |
Comments suppressed due to low confidence (2)
core/include/bertini2/system/straight_line_program.hpp:512
#ifndef BERTINI_DISABLE_PRECISION_CHECKS && _WIN32is invalid preprocessor syntax (#ifndefonly accepts a single macro name). This will not compile; use#if !defined(BERTINI_DISABLE_PRECISION_CHECKS) && defined(_WIN32)(or equivalent).
#ifndef BERTINI_DISABLE_PRECISION_CHECKS && _WIN32
if (!std::is_same<NumT,dbl_complex>::value && Precision(variable_values)!=this->precision_){
std::stringstream err_msg;
err_msg << "variable_values and SLP must be of same precision. respective precisions: " << Precision(variable_values) << " " << this->precision_ << std::endl;
throw std::runtime_error(err_msg.str());
core/include/bertini2/system/straight_line_program.hpp:542
- Same issue as above:
#ifndef BERTINI_DISABLE_PRECISION_CHECKS && _WIN32is not valid. Replace with an#if !defined(...) && defined(_WIN32)style conditional so the file compiles.
#ifndef BERTINI_DISABLE_PRECISION_CHECKS && _WIN32
if (Precision(time)!= DoublePrecision() && Precision(time)!=this->precision_){
std::stringstream err_msg;
err_msg << "time value and SLP must be of same precision. respective precisions: " << Precision(time) << " " << this->precision_ << std::endl;
throw std::runtime_error(err_msg.str());
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS TRUE) | ||
| set(BUILD_SHARED_LIBS TRUE) | ||
| target_compile_options(bertini2 | ||
| PRIVATE $<$<CXX_COMPILER_ID:MSVC>:-bigobj -MP>) |
There was a problem hiding this comment.
MSVC compile options are spelled with / (e.g. /bigobj, /MP). Using -bigobj -MP under CXX_COMPILER_ID:MSVC can fail with cl.exe. Update these flags to the MSVC forms (and if you also support clang-cl, consider branching on CXX_SIMULATE_ID/CMAKE_CXX_COMPILER_FRONTEND_VARIANT).
| PRIVATE $<$<CXX_COMPILER_ID:MSVC>:-bigobj -MP>) | |
| PRIVATE | |
| $<$<CXX_COMPILER_ID:MSVC>:/bigobj /MP> | |
| $<$<AND:$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_FRONTEND_VARIANT:MSVC>>:-bigobj -MP>) |
| add_test(NAME test_classic COMMAND ${CMAKE_BINARY_DIR}/core/test_classic) | ||
|
|
There was a problem hiding this comment.
Hardcoding ${CMAKE_BINARY_DIR}/core/test_classic in add_test can fail on multi-config builds (Debug/Release subdirs) and if runtime output dirs are customized. Use $<TARGET_FILE:test_classic> (or the target name) instead.
| add_test(NAME test_nag_datatypes COMMAND ${CMAKE_BINARY_DIR}/core/test_nag_datatypes) | ||
|
|
There was a problem hiding this comment.
add_test(... COMMAND ${CMAKE_BINARY_DIR}/core/test_nag_datatypes) hardcodes the executable path; this will fail in multi-config generators (Debug/Release subdirs). Use $<TARGET_FILE:test_nag_datatypes> or the target name.
| void SetVariableValues(Eigen::MatrixBase<Derived> const& variable_values) const{ | ||
| using NumT = typename Derived::Scalar; | ||
|
|
There was a problem hiding this comment.
using NumT = typename Derived::Scalar; is declared twice in this function (here and again a few lines below). Re-declaring the same alias in the same scope is a compile error; keep a single NumT alias and remove the duplicate.
| install( | ||
| TARGETS bertini2 | ||
| EXPORT ${TARGETS_EXPORT_NAME} | ||
| PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} | ||
| INCLUDES | ||
| DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} | ||
| LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
| ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
| RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}) |
There was a problem hiding this comment.
bertini2 is installed via install(TARGETS bertini2 ...) here, and then again a few lines later in another install(TARGETS bertini2 ...) call. Installing the same target twice with different destinations/components is error-prone and can lead to duplicate/contradictory install rules; consolidate into a single install(TARGETS ...) block (using GNUInstallDirs variables).
| add_test(NAME test_blackbox COMMAND ${CMAKE_BINARY_DIR}/core/test_blackbox) | ||
|
|
There was a problem hiding this comment.
This test command hardcodes ${CMAKE_BINARY_DIR}/core/test_blackbox, which is fragile across multi-config generators and custom output dirs. Use $<TARGET_FILE:test_blackbox> (or the target name) as the command for add_test.
|
|
||
| find_library(GMP_LIBRARIES gmp PATHS $ENV{GMP_LIB} ${LIB_INSTALL_DIR}) | ||
| if(WIN32) | ||
| find_library(GMP_LIBRARIES libgmp.dll.a PATHS $ENV{GMP_LIB} ${LIB_INSTALL_DIR}) |
There was a problem hiding this comment.
On Windows, hardcoding the import library name to libgmp.dll.a is likely to fail for MSVC/clang-cl toolchains where conda packages typically provide .lib import libs (and CMake can already handle prefixes/suffixes). Consider using find_library(... NAMES gmp libgmp libgmp-10 ...) (or just gmp) rather than a single .dll.a filename to support both MSVC and MinGW layouts.
| find_library(GMP_LIBRARIES libgmp.dll.a PATHS $ENV{GMP_LIB} ${LIB_INSTALL_DIR}) | |
| find_library(GMP_LIBRARIES | |
| NAMES gmp libgmp libgmp-10 | |
| PATHS $ENV{GMP_LIB} ${LIB_INSTALL_DIR} | |
| ) |
| if (MSVC) | ||
| add_compile_options(/bigobj) | ||
| set(CMAKE_BUILD_TYPE Release) | ||
| set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS TRUE) | ||
| set(BUILD_SHARED_LIBS TRUE) |
There was a problem hiding this comment.
set(CMAKE_BUILD_TYPE Release) inside the MSVC branch overrides user choice and breaks multi-config generators (Visual Studio / Ninja Multi-Config), where CMAKE_BUILD_TYPE is ignored. Prefer not forcing the build type here; let the generator/configuration control it (or set defaults only when NOT CMAKE_CONFIGURATION_TYPES AND NOT CMAKE_BUILD_TYPE).
| add_test(NAME test_endgames COMMAND ${CMAKE_BINARY_DIR}/core/test_endgames) | ||
|
|
There was a problem hiding this comment.
add_test(... COMMAND ${CMAKE_BINARY_DIR}/core/test_endgames) hardcodes a build-tree path that isn't portable to multi-config generators. Prefer $<TARGET_FILE:test_endgames> for the test command.
| add_test(NAME test_generating COMMAND ${CMAKE_BINARY_DIR}/core/test_generating) | ||
|
|
There was a problem hiding this comment.
This add_test hardcodes ${CMAKE_BINARY_DIR}/core/test_generating, which can break on multi-config generators (and if output paths change). Use $<TARGET_FILE:test_generating> (or the target name) as the command.
|
I have been playing with compiling at top level (without going through pip). i'm using mamba for package management. it's correctly finding eigenpy (if i remove it via mamba, cmake errors and can't find it; on the other hand, with it installed, I can compile Bertini 2). But, 
|
|
Please, get the latest version of the repo and follow the below steps: |
I was overwhelmed by the warnings I'm not responsible for, coming from Boost, MPFR, etc. I cannot fix those. But, I CAN fix the warnings from my own software. So, by marking the external libraries as SYSTEM, the warnings don't appear. Also, explicitly include mpfr headers, they were missing. also, be target specific.
I was able to compile on my mac with Python 3.14.
|
I'm having test failures on MacOS at this time. Here's the output from running |
|
@ofloveandhate By the way, I use the below packages: The steps are: |
|
We found together that without using care, with both homebrew installed and a mamba environment active, cmake is mixing together packages from both mamba and homebrew. Solution 1: Not using a mamba environment, but just brew, worked like a charm. Outstanding problem: nonetheless, we would like to be able to use mamba on a computer that also has homebrew. |
|
Solution 2: take care when using cmake on a computer with both Homebrew and Mamba active. I'm on MacOS when doing this. Before running cmake, do this Then, when running cmake, let it know where it should install things. It worked like a charm. Unit tests passing. |
|
It would be nice if CMake detected potentially problematic setups -- in particular, those where a conda/mamba environment is active, but pieces are found in other places. This seems difficult. |
|
It's awesome you found the way to do the tests and founded the ground for all the upgrades of dependencies! |
|
On Ubuntu 25.04, using micromamba to install packages, inside virtualbox on my Mac, I was able to compile. Not all tests are passing in the core. |
Fixes crash in unit tests in Ubuntu
|
Fixed the failing test by using explicit types. (Eigen expression templates, lazy evaluation, and |
|
All core tests passing in Ubuntu, if in debug mode. In release mode, still getting a few failures: I think these are due to arithmetic compiler optimization, and could be corrected via slightly relaxing the tolerances. |
several tests were failing on Ubuntu (so gcc) when using compiler optimization, so relaxing the tolerances. In particular `manual_construction_x_pow_lnum1_times_num2l` produces noise in the last 2-3 digits. i added comments for what i got for "exact" values from vpa in Matlab. I also note that both of the tests I modified involve `pow` for complex numbers.
to prevent case fallthrough and evaluation of the system twice
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This pull request introduces significant improvements to the build system and release workflow for the project, especially focusing on cross-platform compatibility and automated Python package publishing. The main highlights include the addition of a comprehensive GitHub Actions workflow for building and publishing Python wheels to PyPI/TestPyPI, enhancements for Windows compatibility, and updates to documentation and changelogs to reflect these changes.
Automated Build and Release Workflow:
.github/workflows/build-and-publish-to-pypi.yml) to automate building Python wheels for Linux, macOS, and Windows, and to publish them to PyPI, TestPyPI, and GitHub Releases. This workflow includes environment setup, dependency installation, platform-specific build steps, artifact uploading, and release signing.Windows Compatibility Improvements:
cmake/FindGMP.cmake,cmake/FindMPFR.cmake,cmake/FindMPC.cmake) to correctly locate and link against Windows-specific library filenames, improving build reliability on Windows. [1] [2] [3]core/CMakeLists.txtto set appropriate build flags and shared library options for MSVC, and improved Boost/Eigen3 detection and linking for cross-platform builds. [1] [2] [3] [4]Documentation and Changelog Updates:
CHANGELOG.mddocumenting recent changes, release preparations, and platform-specific notes, including new contributor acknowledgments.README.mdwith clear installation instructions for Linux, macOS, and Windows, and clarified wheel/platform support.CMake Modernization and Refactoring:
CMakeLists.txtto use modern CMake practices, improved project metadata, integratedjrl-cmakemodulesfor streamlined dependency management, and added options for documentation and testing.pythonsubdirectory in the build to avoid issues during the transition.These changes collectively streamline the build, test, and release process, making it easier to maintain and distribute the package across all major platforms.
References:
[1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]
Check the github workflow: https://github.com/hkmoon/b2/actions/runs/23671693162
Use https://pypi.org/manage/project/pybertini/release/1.0.9/ for testing.