From 1f40766237ff8ce42ddc50ad8b250fd663fffe5e Mon Sep 17 00:00:00 2001 From: Bill Hlavacek Date: Fri, 10 Jul 2026 21:25:52 -0600 Subject: [PATCH] chore(vendor): re-pin BNGsim expr layer to public lanl/bngsim + CI drift-check (#11) The ExprTk swap (#6) vendored four files from BNGsim under third_party/, pinned by third_party/bngsim_expr/VENDOR{,.json}. That pin referenced a commit on the retired private wshlavacek/PyBNF-Private monorepo, where BNGsim lived under a bngsim/ subdir. BNGsim is now published standalone at the public lanl/bngsim, whose history does not carry the old private commit, so scripts/vendor_exprtk.py --check --bngsim-repo could never point at a live public commit. Re-pin to lanl/bngsim@5ce19a4: - Two of the four files advanced upstream and were refreshed to match the new pin: expression.hpp/.cpp gain a tgamma builtin (SBML factorial) and a referenced_variable_addresses accessor (BNGsim forward-sensitivity, GH #212). Neither is used by RM; the rest is clang-format only. Behavioral no-op for RM, confirmed by the full 80-model feature-coverage suite (80 PASS / 0 FAIL at --reps 5) and the 26-test unit suite. - vendor_exprtk.py records repo_subdir "." (lanl/bngsim is the repo root, not a bngsim/ subdir); VENDOR/VENDOR.json now cite the public remote. Wire the guard into CI: a new vendor_check job clones lanl/bngsim (public, no credential) with full history and runs the source drift-check, failing the build on any skew between RM's standalone copy and BNGsim's expression layer (an ODR hazard). This was the only thing blocked on BNGsim going public; the private source repo would have needed a stored deploy key. Patch bump 3.6.0 -> 3.6.1. --- .github/workflows/ci.yml | 35 +++++++++++ CHANGELOG.md | 35 +++++++++++ CMakeLists.txt | 2 +- pyproject.toml | 2 +- scripts/vendor_exprtk.py | 5 +- third_party/bngsim_expr/VENDOR | 6 +- third_party/bngsim_expr/VENDOR.json | 20 +++---- .../bngsim_expr/include/bngsim/expression.hpp | 11 ++++ third_party/bngsim_expr/src/expression.cpp | 58 +++++++++++++++++-- 9 files changed, 153 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20e4a49..aa493db 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -134,3 +134,38 @@ jobs: build/feature_coverage_report.md build/feature_coverage_console.log if-no-files-found: warn + + vendor_check: + # Guards the vendored BNGsim expression layer (issues #6 / #11) against + # drift. scripts/vendor_exprtk.py --check byte-compares the four vendored + # files (exprtk.hpp + bngsim expression.hpp / expr_compat.hpp / + # expression.cpp) against their pinned lanl/bngsim commit. Version skew + # between RuleMonkey's standalone copy and BNGsim's expression layer is an + # ODR violation, so any drift here is a build failure. + # + # This job was blocked until BNGsim went public: --check has to clone the + # source repo, and the old wshlavacek/PyBNF-Private monorepo was private + # (would have needed a stored deploy key / PAT). lanl/bngsim is public, so + # the clone needs no credential. fetch-depth: 0 pulls full history, so the + # pinned commit stays reachable even after BNGsim's HEAD advances past it. + # Purely additive: no build, no NFsim/BNG2, ~stdlib-only Python. + runs-on: ubuntu-latest + + steps: + - name: Checkout RuleMonkey + uses: actions/checkout@v4 + + - name: Checkout lanl/bngsim (full history for the pinned commit) + uses: actions/checkout@v4 + with: + repository: lanl/bngsim + path: bngsim + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Drift-check vendored BNGsim expression layer + run: python3 scripts/vendor_exprtk.py --check --bngsim-repo bngsim diff --git a/CHANGELOG.md b/CHANGELOG.md index 868379e..c564e26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,41 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [3.6.1] — 2026-07-10 + +### Changed + +- **Re-pinned the vendored BNGsim expression layer to public `lanl/bngsim` + (issue #11).** The ExprTk swap (issue #6) vendored four files under + `third_party/` (`exprtk.hpp` + `bngsim_expr/{include,src}`), pinned by + `third_party/bngsim_expr/VENDOR{,.json}`. That pin referenced a commit on + the now-retired private `wshlavacek/PyBNF-Private` monorepo, where BNGsim + lived under a `bngsim/` subdir. BNGsim is now published standalone at the + public `lanl/bngsim`, whose history does not carry the old private commit, + so the pin has been moved to `lanl/bngsim@5ce19a4`. Provenance in + `VENDOR`/`VENDOR.json` now records the public remote and drops the stale + `bngsim` subdir. + + Two of the four files advanced in the public tree and were refreshed to + match the new pin: `expression.hpp`/`expression.cpp` gain a `tgamma` + built-in (SBML factorial support) and a `referenced_variable_addresses` + accessor (BNGsim forward-sensitivity work, GH #212). Neither is used by + RuleMonkey, and the remainder is clang-format only — the change is a + behavioral no-op for RM, confirmed by the full 80-model feature-coverage + suite (80 PASS / 0 FAIL at `--reps 5`) and the 26-test unit suite. + +### Added + +- **CI drift-guard for the vendored BNGsim expression layer (issue #11).** A + new `vendor_check` job in `.github/workflows/ci.yml` clones `lanl/bngsim` + (public, no credential) with full history and runs + `scripts/vendor_exprtk.py --check --bngsim-repo bngsim`, byte-comparing the + vendored files against their pinned commit. Version skew between + RuleMonkey's standalone copy and BNGsim's expression layer is an ODR + violation, so drift now fails the build. This was blocked until BNGsim went + public: the previous private source repo would have required a stored + deploy key to clone. + ## [3.6.0] — 2026-07-03 ### Added diff --git a/CMakeLists.txt b/CMakeLists.txt index 949da6b..9fdd26d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 3.20) -project(RuleMonkey VERSION 3.6.0 LANGUAGES CXX) +project(RuleMonkey VERSION 3.6.1 LANGUAGES CXX) if(CMAKE_VERSION VERSION_LESS 3.21) if(CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR) diff --git a/pyproject.toml b/pyproject.toml index 1555657..e9972b4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "rulemonkey-harness" -version = "3.6.0" +version = "3.6.1" description = "Test and benchmarking harness for the RuleMonkey C++ engine" readme = "README.md" requires-python = ">=3.10" diff --git a/scripts/vendor_exprtk.py b/scripts/vendor_exprtk.py index 1ca516b..6fccd3b 100755 --- a/scripts/vendor_exprtk.py +++ b/scripts/vendor_exprtk.py @@ -208,7 +208,10 @@ def build_metadata( "source": { "name": "BNGsim", "repo_remote": checkout_info["origin_remote"], - "repo_subdir": "bngsim", + # BNGsim is the repository root of lanl/bngsim (no subdir). The + # legacy "bngsim" value described BNGsim living under a bngsim/ + # subdir of the retired wshlavacek/PyBNF-Private monorepo. + "repo_subdir": ".", "branch_or_ref": ref, "current_branch": checkout_info["current_branch"], "commit": commit, diff --git a/third_party/bngsim_expr/VENDOR b/third_party/bngsim_expr/VENDOR index 500a865..c763892 100644 --- a/third_party/bngsim_expr/VENDOR +++ b/third_party/bngsim_expr/VENDOR @@ -7,9 +7,9 @@ from BNGsim and are compiled only in standalone RuleMonkey builds; inside a BNGsim build, RuleMonkey links the host bngsim::expression target instead. -Source repo : git@github.com:wshlavacek/PyBNF-Private.git -Source subdir: bngsim -Pinned commit: 1a1d49da13f8cd723be213774c2a81f0da538c84 +Source repo : git@github.com:lanl/bngsim.git +Source subdir: . +Pinned commit: 5ce19a4f76f236030566b49ee5afa47733b9ba39 Metadata : third_party/bngsim_expr/VENDOR.json Vendored files (copied verbatim, no edits): diff --git a/third_party/bngsim_expr/VENDOR.json b/third_party/bngsim_expr/VENDOR.json index 2fb6610..ccdc760 100644 --- a/third_party/bngsim_expr/VENDOR.json +++ b/third_party/bngsim_expr/VENDOR.json @@ -3,13 +3,13 @@ "vendored_path": "third_party/bngsim_expr", "source": { "name": "BNGsim", - "repo_remote": "git@github.com:wshlavacek/PyBNF-Private.git", - "repo_subdir": "bngsim", - "branch_or_ref": "1a1d49da13f8cd723be213774c2a81f0da538c84", - "current_branch": "feature/bngsim", - "commit": "1a1d49da13f8cd723be213774c2a81f0da538c84" + "repo_remote": "git@github.com:lanl/bngsim.git", + "repo_subdir": ".", + "branch_or_ref": "5ce19a4f76f236030566b49ee5afa47733b9ba39", + "current_branch": "main", + "commit": "5ce19a4f76f236030566b49ee5afa47733b9ba39" }, - "imported_at_utc": "2026-06-20T02:10:39+00:00", + "imported_at_utc": "2026-07-11T03:18:22+00:00", "files": { "exprtk_header": { "path": "third_party/exprtk/exprtk.hpp", @@ -21,8 +21,8 @@ "expression_header": { "path": "third_party/bngsim_expr/include/bngsim/expression.hpp", "source_path": "include/bngsim/expression.hpp", - "sha256": "d1ab141722d4073cc33bad09fa00b22bc1e5d47543c4e639470c3d4a75087a6a", - "bytes": 5032, + "sha256": "ddbc1f8cba3050f1da4d85415b7b3e9104127b3a2bef30475bd205832bfb1fb7", + "bytes": 5823, "description": "BNGsim ExprTkEvaluator public wrapper header" }, "expr_compat_header": { @@ -35,8 +35,8 @@ "expression_source": { "path": "third_party/bngsim_expr/src/expression.cpp", "source_path": "src/expression.cpp", - "sha256": "9b54169c75e672e74fe6ca3825e5dcd027f7dee463821fc767991ccf775962fd", - "bytes": 36878, + "sha256": "d72a2b77b155282e551ccb57d3fb8e68dc7e2ecf0987ea50058ff7aa3e27f668", + "bytes": 39062, "description": "BNGsim ExprTkEvaluator implementation" } }, diff --git a/third_party/bngsim_expr/include/bngsim/expression.hpp b/third_party/bngsim_expr/include/bngsim/expression.hpp index 638ae2b..3ed61e0 100644 --- a/third_party/bngsim_expr/include/bngsim/expression.hpp +++ b/third_party/bngsim_expr/include/bngsim/expression.hpp @@ -40,6 +40,16 @@ class ExpressionEvaluator { // Evaluate a previously compiled expression by ID. virtual double evaluate(int expr_id) = 0; + + // Return the addresses of the bound model variables (species + // concentrations / parameter values registered via define_variable) that a + // compiled expression references. Used to classify event triggers for + // forward-sensitivity support (GH #212): a trigger referencing a species is + // state-dependent, and one referencing a requested sensitivity parameter + // has a parameter-dependent crossing time — both are beyond Phase-1 + // (fixed-time) event sensitivity and keep raising. Constants and built-in + // functions (e.g. time()) are not variables and are not reported. + virtual std::vector referenced_variable_addresses(int expr_id) const = 0; }; // ─── ExprTk-based implementation ───────────────────────────────────────────── @@ -73,6 +83,7 @@ class ExprTkEvaluator : public ExpressionEvaluator { int compile(const std::string &expr) override; double evaluate(int expr_id) override; + std::vector referenced_variable_addresses(int expr_id) const override; // Bind the time() and t() functions to a double* that tracks simulation time. // Must be called after construction, before compiling time-dependent expressions. diff --git a/third_party/bngsim_expr/src/expression.cpp b/third_party/bngsim_expr/src/expression.cpp index 77697d1..53437d4 100644 --- a/third_party/bngsim_expr/src/expression.cpp +++ b/third_party/bngsim_expr/src/expression.cpp @@ -257,6 +257,22 @@ template struct SignFunction : public exprtk::ifunction { } }; +// tgamma(x) = Γ(x); the SBML loader emits ``tgamma((n)+1)`` for MathML +// factorial(n) (ExprTk has no gamma builtin). std::tgamma matches the C +// library form native codegen emits, so interpreted and codegen agree. +template struct TgammaFunction : public exprtk::ifunction { + NonFiniteWarningSet *warner = nullptr; + TgammaFunction() : exprtk::ifunction(1) {} + T operator()(const T &x) override { + const double dx = static_cast(x); + const double r = std::tgamma(dx); + if (warner) { + warner->warn_if_nonfinite("tgamma", {dx}, r); + } + return static_cast(r); + } +}; + // 0-arg: time() — reads from a bound double*. No warner: the simulator // owns the time pointer and a non-finite t would be a higher-level bug // flagged by the integrator, not by this layer. @@ -471,6 +487,7 @@ struct ExprTkEvaluator::Impl { LnFunction ln_func; RintFunction rint_func; SignFunction sign_func; + TgammaFunction tgamma_func; TimeFunction time_func; // User-registered custom functions (owned, heap-allocated) @@ -642,11 +659,13 @@ struct ExprTkEvaluator::Impl { ln_func.warner = &nonfinite_warner; rint_func.warner = &nonfinite_warner; sign_func.warner = &nonfinite_warner; + tgamma_func.warner = &nonfinite_warner; // Register backward-compatible aliases symbol_table.add_function("ln", ln_func); symbol_table.add_function("rint", rint_func); symbol_table.add_function("sign", sign_func); + symbol_table.add_function("tgamma", tgamma_func); // Register built-in functions // Note: ExprTk has a built-in `if` keyword (grammar-level) that handles @@ -806,6 +825,35 @@ const std::string &ExprTkEvaluator::preprocessed_expr(int expr_id) const { return impl_->preprocessed_strings[expr_id]; } +std::vector ExprTkEvaluator::referenced_variable_addresses(int expr_id) const { + std::vector out; + if (expr_id < 0 || expr_id >= static_cast(impl_->preprocessed_strings.size())) { + return out; + } + // Collect the variable identifiers referenced by the (already remapped) + // preprocessed string, then resolve each through this evaluator's symbol + // table to the address it was bound to via define_variable. Names not + // registered as variables (constants, functions) resolve to null and are + // skipped, so the result contains only model-variable addresses. + std::vector names; + // Pass the symbol table so the collector resolves built-in/user functions + // (e.g. time()) instead of bailing out when it meets an unknown token — the + // symbol-table-less overload returns nothing for a trigger like + // `time() >= t_dose`, which would silently hide a parameter reference. + if (!exprtk::collect_variables(impl_->preprocessed_strings[expr_id], impl_->symbol_table, + names)) { + return out; + } + out.reserve(names.size()); + for (const std::string &nm : names) { + auto *var = impl_->symbol_table.get_variable(nm); + if (var != nullptr) { + out.push_back(&var->ref()); + } + } + return out; +} + int ExprTkEvaluator::n_expressions() const { return static_cast(impl_->expressions.size()); } double ExprTkEvaluator::evaluate(int expr_id) { @@ -836,11 +884,11 @@ std::unique_ptr ExprTkEvaluator::clone_empty() const { ReservedNames reserved_names() { ReservedNames names; names.constants = {"_pi", "_e", "_kB", "_NA", "_R", "_h", "_F"}; - names.functions = {"time", "sin", "cos", "tan", "asin", "acos", "atan", "sinh", - "cosh", "tanh", "asinh", "acosh", "atanh", "exp", "log", "ln", - "log2", "log10", "sqrt", "abs", "floor", "ceil", "round", "rint", - "trunc", "min", "max", "clamp", "avg", "sum", "erf", "erfc", - "sign", "sgn", "if", "mratio"}; + names.functions = {"time", "sin", "cos", "tan", "asin", "acos", "atan", "sinh", + "cosh", "tanh", "asinh", "acosh", "atanh", "exp", "log", "ln", + "log2", "log10", "sqrt", "abs", "floor", "ceil", "round", "rint", + "trunc", "min", "max", "clamp", "avg", "sum", "erf", "erfc", + "sign", "sgn", "if", "mratio", "tgamma"}; return names; }