Skip to content

Stabilize new build in separate repository, and get CI/CD working - #8

Open
CodeByDrescher wants to merge 54 commits into
masterfrom
stabilize-new-build
Open

Stabilize new build in separate repository, and get CI/CD working#8
CodeByDrescher wants to merge 54 commits into
masterfrom
stabilize-new-build

Conversation

@CodeByDrescher

@CodeByDrescher CodeByDrescher commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Stabilize the ODE solver against sanitizers, modernize the build, and ship a
Python distribution alongside the native CLI.

  • Build: switch to Conan-managed dependencies (argparse, spdlog,
    libcurl); CMake minimum 3.13 → 3.16; consolidate platform detection on
    WIN32; new options OPTION_TARGET_PYTHON_BINDING,
    OPTION_TEST_WITH_LOCALHOST, OPTION_STATICALLY_LINK; warning cleanup
    pass and sprintf removal so the C++20 build compiles clean.
  • Solver architecture: lift input parsing out of the solver into
    VCellSolverInput + VCellSolverFactory; introduce abstract VCellSolver
    base; replace the 273-line hand-rolled argv parser in
    SundialsSolverStandalone with argparse; expose a reusable
    solve(input, output, tid) entry in SundialsSolverInterface (used by
    both the CLI and the Python module).
  • Messaging stabilization: decompose SimulationMessaging into
    MessageEventManager (worker thread + queue, with documented lock
    ordering), CurlProxyClasses (AbstractCurlProxy / NullCurlProxy /
    CurlProxy — replaces #ifdef USE_MESSAGING forests with polymorphism),
    JobEventStatus (now a JobEvent::Status namespaced enum), and
    WorkerEvent. Fixes from TSAN/UBSAN/leak runs: data race,
    mutex-access-before-constructor-finished, undefined behavior, memory
    leaks, int overflow in a test. std::jthreadstd::thread for
    compiler portability. Net: SimulationMessaging.cpp 755 → 194 lines.
  • Python bindings (pyvcell_odesolver): scikit-build-core + vendored
    pybind11; src/main.cpp exposes version() and
    solve(cvode_input_file_path, output_file_path, tid=-1).
  • CI/CD: matrix over macOS-15 (arm + intel), Windows, Linux x86_64 +
    arm64, Python 3.10–3.13. Per-platform Conan profiles, cibuildwheel
    Python wheels, lipo macOS Universal build, GHCR manylinux images,
    mold linker on Linux, sanitizer-enabled CMake.
  • Tests: replace IDAWin/tests/smoke/smoke.py with a top-level
    tests/ dir of GoogleTest C++ tests (smoke, hello, message-processing)
    plus a test_basic.py that exercises the wheel.

Test plan

  • CI green across all 5 platforms × 4 Python versions
  • ctest -VV green locally (Linux, macOS); Windows via CI
  • SundialsSolverStandalone_x64 runs the smoke .cvodeInput and
    matches *.ida.expected / *.cvode.expected
  • pip install of the built wheel exposes
    pyvcell_odesolver.solve(...) and produces output matching the CLI
  • Run with -DOPTION_TARGET_MESSAGING=ON against a localhost broker
    (-DOPTION_TEST_WITH_LOCALHOST=ON) — no TSAN/leak warnings
  • macOS Universal binary verified with lipo -archs

@CodeByDrescher
CodeByDrescher requested a review from jcschaff April 14, 2026 14:35
@CodeByDrescher

Copy link
Copy Markdown
Collaborator Author

closes #5
closes #7

CodeByDrescher and others added 11 commits July 10, 2026 12:52
Also: adding windows on arm (woa) support(?)
Both libraries are shared with vcell-stochastic, which already consumes them as
submodules. Vendoring them here let the copies drift apart.

vcell-messaging: the public API is byte-identical to the copy it replaces (the
apparent diff was CRLF vs LF), so no call sites change. The submodule additionally
carries a deadlock fix and a shutdown fix: the lock order is now queue-before-stop
rather than the reverse, and shutdown joins the worker instead of waiting for the
queue to drain -- an empty queue is not "all work finished", since the worker pops
under the lock but sends after releasing it.

vcell-expressionparser: the in-tree copy here was the *newer* of the two -- the
std::format/nullptr/warning cleanup from f3c3107 and 096ba48 existed only in
this repo. Rather than lose it, that work was pushed up to the submodule
(virtualcell/vcell-expressionparser @ 2fd45ae) and is pinned here, so the two
repos converge on the better copy instead of the older one. The target rename
ExpressionParser -> vcellexpressionparser is reflected in IDAWin.

ExpressionParserTest/ is left alone; it was already wired into nothing.

Verified: clean configure, full build, and all 4 gtest cases pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OdeResultSet.cpp and VCellSundialsSolver.cpp call memset/memcpy without
including <cstring>. Older libstdc++ pulled it in transitively through other
headers; GCC 13 no longer does, so both fail to compile with "'memset' was not
declared in this scope".

Pre-existing and unrelated to the submodule migration -- it reproduces on a
pristine checkout of this branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Added support for recursive submodule checkout in CI workflow.
The windows-latest runner moved to the windows-2025-vs2026 image, which
ships only Visual Studio 18 (MSVC 14.51). The x86_64 Conan profile pinned
compiler.runtime_version=v143, and for compiler=clang Conan derives the
Visual Studio version from that setting rather than from
tools.microsoft.msbuild:vs_version (see _vcvars_versions() in
conan/tools/microsoft/visual.py, which maps v143/v144 -> VS 17 and
v145 -> VS 18). VCVars therefore looked for a VS 17 install that no longer
exists and the job failed while building fmt from source:

    ConanException: VS non-existing installation: Visual Studio 17

Bump the x86_64 profile to v145. Windows-ARM64 stays on v144 because the
windows-11-arm64 image still carries VS 17; it will need the same bump when
that image rolls forward.

Also record the runtime_version -> Visual Studio mapping in CLAUDE.md and in
the profile, since the setting's name does not suggest it selects a Visual
Studio installation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five issues introduced by this branch and left behind when it replaced the
pybind11/scikit-build scaffold with ctypes + uv_build:

- version() was annotated `-> str` but returned the ReturnValue pydantic model
  from call_version() unchanged, so callers got `success=True message='...'`
  instead of the version string. Unwrap `.message`, matching how solve()
  already unwraps `.success`. The existing test only asserted `is not None`,
  so it caught nothing either way.
- .gitignore listed __pycache__ per directory and missed
  pyvcell_odesolver/_internal/, which appeared as untracked .pyc files.
  Replace the three entries with a single __pycache__/ rule.
- [tool.ruff] src pointed at "src", a directory that has never existed in this
  layout (the package sits at the repo root), leaving first-party import
  resolution misconfigured.
- argparse was fetched via FetchContent with no GIT_TAG, so every clean
  configure cloned whatever was on the default branch. Pin v3.2 to match the
  ">=3.2 <4.0" range conanfile.py already declares; googletest alongside it
  was already pinned.
- extern/pybind11 (3.4 MB) had no remaining references anywhere -- no
  add_subdirectory, no find_package -- once the bindings became ctypes.

CLAUDE.md documented the first four as known defects; updated accordingly.

Verified: cmake configure/build clean with argparse pinned at v3.2, ctest 4/4,
pytest 2/2, and version() now returns a str.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants