chore(bngsim_expr): refresh vendored expression evaluator to PyBNF-Private 1a1d49da

scripts/vendor_exprtk.py

@@ -35,7 +35,6 @@
 from pathlib import Path
 from typing import Any
 
-
 RM_ROOT = Path(__file__).resolve().parents[1]
 VENDOR_DIR = RM_ROOT / "third_party" / "bngsim_expr"
 VENDOR_NOTE = VENDOR_DIR / "VENDOR"
@@ -55,6 +54,11 @@
         "source_path": "include/bngsim/expression.hpp",
         "description": "BNGsim ExprTkEvaluator public wrapper header",
     },
+    "expr_compat_header": {
+        "path": "third_party/bngsim_expr/include/bngsim/expr_compat.hpp",
+        "source_path": "include/bngsim/expr_compat.hpp",
+        "description": "BNGsim BNG<->ExprTk compatibility primitives (GH #49)",
+    },
     "expression_source": {
         "path": "third_party/bngsim_expr/src/expression.cpp",
         "source_path": "src/expression.cpp",
@@ -67,8 +71,7 @@ def run(cmd: list[str], *, text: bool = True) -> subprocess.CompletedProcess:
     return subprocess.run(
         cmd,
         check=True,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
+        capture_output=True,
         text=text,
     )
 
@@ -224,10 +227,7 @@ def build_metadata(
 
 
 def source_bytes_at_commit(bngsim: Path, commit: str) -> dict[str, bytes]:
-    return {
-        key: git_show(bngsim, commit, spec["source_path"])
-        for key, spec in FILES.items()
-    }
+    return {key: git_show(bngsim, commit, spec["source_path"]) for key, spec in FILES.items()}
 
 
 def write_vendor_note(metadata: dict[str, Any]) -> None:
@@ -311,8 +311,7 @@ def local_metadata_drift(metadata: dict[str, Any] | None) -> list[str]:
             continue
         if source_path != spec["source_path"]:
             drift.append(
-                f"files.{key}.source_path: expected {spec['source_path']!r}, "
-                f"found {source_path!r}"
+                f"files.{key}.source_path: expected {spec['source_path']!r}, found {source_path!r}"
             )
 
         path = RM_ROOT / spec["path"]
@@ -324,13 +323,11 @@ def local_metadata_drift(metadata: dict[str, Any] | None) -> list[str]:
         actual_bytes = len(data)
         if record.get("sha256") != actual_sha:
             drift.append(
-                f"files.{key}.sha256: expected {record.get('sha256')!r}, "
-                f"found {actual_sha!r}"
+                f"files.{key}.sha256: expected {record.get('sha256')!r}, found {actual_sha!r}"
             )
         if record.get("bytes") != actual_bytes:
             drift.append(
-                f"files.{key}.bytes: expected {record.get('bytes')!r}, "
-                f"found {actual_bytes!r}"
+                f"files.{key}.bytes: expected {record.get('bytes')!r}, found {actual_bytes!r}"
             )
 
     if not metadata_commit(metadata):
@@ -344,9 +341,7 @@ def source_drift(bngsim: Path, commit: str) -> list[str]:
         source_data = git_show(bngsim, commit, spec["source_path"])
         local_data = (RM_ROOT / spec["path"]).read_bytes()
         if source_data != local_data:
-            drift.append(
-                f"{spec['path']} differs from BNGsim {commit}:./{spec['source_path']}"
-            )
+            drift.append(f"{spec['path']} differs from BNGsim {commit}:./{spec['source_path']}")
     return drift
 
 
@@ -446,7 +441,7 @@ def main() -> int:
     metadata = build_metadata(bngsim, checkout_info, ref, commit, source_bytes)
     write_refresh(source_bytes, metadata)
     print(
-        "vendor_exprtk: refreshed 3 files plus VENDOR.json/VENDOR "
+        f"vendor_exprtk: refreshed {len(FILES)} files plus VENDOR.json/VENDOR "
         f"from BNGsim {ref} ({commit})"
     )
     return 0

third_party/bngsim_expr/VENDOR

@@ -9,12 +9,13 @@ target instead.
 
 Source repo  : git@github.com:wshlavacek/PyBNF-Private.git
 Source subdir: bngsim
-Pinned commit: 330cfcc4913a98bd45f2dc80da047621a2ed8092
+Pinned commit: 1a1d49da13f8cd723be213774c2a81f0da538c84
 Metadata     : third_party/bngsim_expr/VENDOR.json
 
 Vendored files (copied verbatim, no edits):
   third_party/exprtk/exprtk.hpp <- bngsim third_party/exprtk/exprtk.hpp
   third_party/bngsim_expr/include/bngsim/expression.hpp <- bngsim include/bngsim/expression.hpp
+  third_party/bngsim_expr/include/bngsim/expr_compat.hpp <- bngsim include/bngsim/expr_compat.hpp
   third_party/bngsim_expr/src/expression.cpp <- bngsim src/expression.cpp
 
 Refresh / drift-check

third_party/bngsim_expr/VENDOR.json

@@ -5,11 +5,11 @@
     "name": "BNGsim",
     "repo_remote": "git@github.com:wshlavacek/PyBNF-Private.git",
     "repo_subdir": "bngsim",
-    "branch_or_ref": "330cfcc4913a98bd45f2dc80da047621a2ed8092",
+    "branch_or_ref": "1a1d49da13f8cd723be213774c2a81f0da538c84",
     "current_branch": "feature/bngsim",
-    "commit": "330cfcc4913a98bd45f2dc80da047621a2ed8092"
+    "commit": "1a1d49da13f8cd723be213774c2a81f0da538c84"
   },
-  "imported_at_utc": "2026-05-23T19:17:39+00:00",
+  "imported_at_utc": "2026-06-20T02:10:39+00:00",
   "files": {
     "exprtk_header": {
       "path": "third_party/exprtk/exprtk.hpp",
@@ -25,11 +25,18 @@
       "bytes": 5032,
       "description": "BNGsim ExprTkEvaluator public wrapper header"
     },
+    "expr_compat_header": {
+      "path": "third_party/bngsim_expr/include/bngsim/expr_compat.hpp",
+      "source_path": "include/bngsim/expr_compat.hpp",
+      "sha256": "00c6944497d5405c03a11d92c6295452963d70f304378e5be9561af58762ed7b",
+      "bytes": 2778,
+      "description": "BNGsim BNG<->ExprTk compatibility primitives (GH #49)"
+    },
     "expression_source": {
       "path": "third_party/bngsim_expr/src/expression.cpp",
       "source_path": "src/expression.cpp",
-      "sha256": "4a7bf502cdd6be1e381e754140e35f0bf57a3fc9e778a5723db9b7909ca03a64",
-      "bytes": 33050,
+      "sha256": "9b54169c75e672e74fe6ca3825e5dcd027f7dee463821fc767991ccf775962fd",
+      "bytes": 36878,
       "description": "BNGsim ExprTkEvaluator implementation"
     }
   },

third_party/bngsim_expr/include/bngsim/expr_compat.hpp

@@ -0,0 +1,54 @@
+// bngsim/include/bngsim/expr_compat.hpp — BNG ↔ ExprTk compatibility primitives
+//
+// Single source of truth for the BNG-compatibility logic shared by the two
+// ExprTk-backed expression layers in BNGsim:
+//
+//   * the host evaluator  — bngsim::ExprTkEvaluator (src/expression.cpp), used
+//     by the ODE/SSA engines and NfsimSimulator::evaluate_expression; and
+//   * the vendored NFsim mu::Parser shim (third_party/nfsim/src/NFfunction/
+//     nfsim_funcparser.h), which evaluates NFsim function columns in-process.
+//
+// Both layers must agree on (a) the BNGL built-in mratio(), and (b) how a model
+// identifier is mapped onto an ExprTk-legal symbol name (the unconditional
+// leading-underscore remap and the conditional reserved-symbol mangle). Before
+// issue #49 each feature was hand-ported into both layers and kept in sync by
+// guard tests; the shim now forwards to these definitions instead, so the logic
+// lives in exactly one place (the bngsim::expression static archive that the
+// vendored NFsim target links). See issue #49 and bngsim ADR-005
+// (bngsim/dev/adr/ADR-005-nfsim-exprtk-shim-single-source.md).
+//
+// The declarations here are deliberately free of any ExprTk dependency: the
+// reserved-symbol set is built from exprtk.hpp inside the single translation
+// unit that defines is_exprtk_reserved(), so neither the host call sites nor
+// the NFsim shim need to include the 1.6 MB ExprTk header to call these.
+#pragma once
+
+#include <string>
+
+namespace bngsim {
+namespace expr_compat {
+
+// mratio(a, b, z): the BNGL built-in confluent-hypergeometric ratio
+// M(a+1, b+1, z) / M(a, b, z), evaluated by Gauss's continued fraction with the
+// modified-Lentz method so it stays finite on BNG's large-|z| range (issue #42).
+double mratio(double a, double b, double z);
+
+// Unconditional leading-underscore remap: "_X" → "u_X" (ExprTk rejects an
+// identifier starting with '_'). Identity for any name not starting with '_'.
+// No ExprTk built-in starts with '_', so this rewrite is always safe to apply
+// token-by-token to an expression string.
+std::string remap_name(const std::string &name);
+
+// True if `name`, registered as an ExprTk variable, would collide with a name
+// the symbol table already owns: an ExprTk reserved word/symbol, a bngsim
+// function alias (ln/rint/sign/mratio/time), or one of the "u_*" keys the
+// built-in constants occupy after the underscore remap (GH #90).
+bool is_exprtk_reserved(const std::string &name);
+
+// Symbol-table key for `name` at registration time: the unconditional
+// underscore remap first, then the conditional reserved-symbol mangle
+// ("X" → "r_X"). Identity if neither applies.
+std::string compute_registration_name(const std::string &name);
+
+} // namespace expr_compat
+} // namespace bngsim

third_party/bngsim_expr/src/expression.cpp

@@ -19,6 +19,7 @@
 #define exprtk_disable_caseinsensitivity
 #include "exprtk.hpp"
 
+#include "bngsim/expr_compat.hpp"
 #include "bngsim/expression.hpp"
 
 #include <algorithm>
@@ -62,7 +63,11 @@ namespace bngsim {
 // double's representable range — and the ratio becomes inf/inf = nan. Lentz
 // stays O(1) throughout and converges in a few hundred iterations even on
 // those inputs.
-static double mratio_impl(double a, double b, double z) {
+//
+// This is the single source of truth for mratio across BNGsim: the host
+// MratioFunction adapter below and the vendored NFsim mu::Parser ExprTk shim
+// both call it (the shim via <bngsim/expr_compat.hpp>). See issue #49.
+double expr_compat::mratio(double a, double b, double z) {
     constexpr double eps = 1.0e-16;
     constexpr double tiny = 1.0e-32;
     // Safety cap so a pathological non-converging case fails loud rather
@@ -204,7 +209,7 @@ template <typename T> struct MratioFunction : public exprtk::ifunction<T> {
         const double da = static_cast<double>(a);
         const double db = static_cast<double>(b);
         const double dz = static_cast<double>(z);
-        const double r = mratio_impl(da, db, dz);
+        const double r = expr_compat::mratio(da, db, dz);
         if (warner) {
             warner->warn_if_nonfinite("mratio", {da, db, dz}, r);
         }
@@ -376,7 +381,22 @@ static const std::unordered_set<std::string> &bngsim_exprtk_aliases() {
     return aliases;
 }
 
-static bool is_exprtk_reserved(const std::string &name) {
+// Registration keys occupied by bngsim's built-in constants. init_builtins()
+// registers each "_X" constant (Planck's `_h`, Avogadro's `_NA`, …) under the
+// ExprTk key "u_X" because ExprTk rejects a leading '_'. A user parameter
+// named literally "u_h" / "u_pi" / … maps to that same key (it does not start
+// with '_', so compute_registration_name() would leave it unchanged) and
+// would collide with the constant slot at registration (GH #90: Chitnis2012
+// / BIOMD0000000950 has a parameter `u_h`). Treat these keys as reserved so
+// such names take the r_ mangling path like any other reserved-word collision.
+// Must stay in sync with the "_X" constants registered in init_builtins().
+static const std::unordered_set<std::string> &bngsim_remapped_constant_keys() {
+    static const std::unordered_set<std::string> keys = {"u_pi", "u_e", "u_kB", "u_NA",
+                                                         "u_R",  "u_h", "u_F"};
+    return keys;
+}
+
+bool expr_compat::is_exprtk_reserved(const std::string &name) {
     // Names that, if registered as a user variable, would collide with
     // a name already taken by the symbol table. Two sources:
     //
@@ -395,20 +415,36 @@ static bool is_exprtk_reserved(const std::string &name) {
     //      the `sign` collision in practice — but we mangle all five
     //      for symmetry and to handle hand-crafted .net inputs.
     //
+    //   3. The registration keys bngsim's built-in constants occupy after the
+    //      unconditional "_X" → "u_X" remap (`u_pi`, …, `u_h`, `u_F`). A user
+    //      parameter named literally `u_h` would otherwise alias Planck's
+    //      constant slot and fail to register (GH #90).
+    //
     // Comparison is exact (case-sensitive) because we build with
     // exprtk_disable_caseinsensitivity, so e.g. `Const` is not reserved.
     const auto &exprtk_reserved = exprtk_reserved_identifiers();
     const auto &bngsim_aliases = bngsim_exprtk_aliases();
+    const auto &constant_keys = bngsim_remapped_constant_keys();
     return exprtk_reserved.find(name) != exprtk_reserved.end() ||
-           bngsim_aliases.find(name) != bngsim_aliases.end();
+           bngsim_aliases.find(name) != bngsim_aliases.end() ||
+           constant_keys.find(name) != constant_keys.end();
 }
 
-// Compute the symbol-table key for `name` at registration time.
-// Combines the unconditional underscore remap with reserved-word mangling.
-static std::string compute_registration_name(const std::string &name) {
+// Unconditional leading-underscore remap "_X" → "u_X" (see expr_compat.hpp).
+std::string expr_compat::remap_name(const std::string &name) {
     if (!name.empty() && name[0] == '_') {
         return "u_" + name.substr(1);
     }
+    return name;
+}
+
+// Compute the symbol-table key for `name` at registration time.
+// Combines the unconditional underscore remap with reserved-word mangling.
+std::string expr_compat::compute_registration_name(const std::string &name) {
+    std::string underscore_mapped = remap_name(name);
+    if (underscore_mapped != name) {
+        return underscore_mapped;
+    }
     if (is_exprtk_reserved(name)) {
         return "r_" + name;
     }
@@ -466,8 +502,9 @@ struct ExprTkEvaluator::Impl {
     // that were actually registered on this evaluator, so built-in tokens
     // (sin, if, time, ...) pass through unchanged.
     std::string remap_token(const std::string &name) const {
-        if (!name.empty() && name[0] == '_') {
-            return "u_" + name.substr(1);
+        std::string underscore_mapped = expr_compat::remap_name(name);
+        if (underscore_mapped != name) {
+            return underscore_mapped;
         }
         auto it = mangled_user_names.find(name);
         if (it != mangled_user_names.end()) {
@@ -540,7 +577,37 @@ struct ExprTkEvaluator::Impl {
                        (std::isalnum(static_cast<unsigned char>(expr[i])) || expr[i] == '_')) {
                     i++;
                 }
-                result += remap_token(expr.substr(start, i - start));
+                const std::string token = expr.substr(start, i - start);
+                // A declared symbol that collides with an ExprTk reserved name
+                // and is *also* used in call form (`frac(x)`) is genuinely
+                // ambiguous in a single flat namespace — raise the same clear,
+                // deterministic error the NFsim mu::Parser shim does, rather
+                // than rewriting to r_<name> and letting ExprTk emit a cryptic
+                // "not a function" message. strip_empty_parens() has already
+                // removed legitimate zero-arg scalar calls (`obs()`), so any
+                // `name(` left for a mangled symbol is a real call form. Only
+                // reserved-mangled names are tracked in mangled_user_names;
+                // underscore remaps are not, so they fall through untouched.
+                if (mangled_user_names.count(token)) {
+                    size_t j = i;
+                    while (j < expr.size() && std::isspace(static_cast<unsigned char>(expr[j]))) {
+                        j++;
+                    }
+                    if (j < expr.size() && expr[j] == '(') {
+                        throw std::runtime_error(
+                            "identifier '" + token +
+                            "' is both a declared model "
+                            "symbol and used as a function call '" +
+                            token +
+                            "(...)'; "
+                            "ExprTk reserves '" +
+                            token +
+                            "' as a built-in and a single "
+                            "flat namespace cannot hold both meanings — rename the "
+                            "model symbol");
+                    }
+                }
+                result += remap_token(token);
                 continue;
             }
             result += expr[i];
@@ -550,7 +617,7 @@ struct ExprTkEvaluator::Impl {
     }
 
     void add_remapped_constant(const std::string &name, double value) {
-        std::string mapped = compute_registration_name(name);
+        std::string mapped = expr_compat::compute_registration_name(name);
         if (mapped != name && name[0] != '_') {
             mangled_user_names[name] = mapped;
         }
@@ -612,9 +679,9 @@ ExprTkEvaluator::ExprTkEvaluator() : impl_(std::make_unique<Impl>()) {}
 ExprTkEvaluator::~ExprTkEvaluator() = default;
 
 void ExprTkEvaluator::define_variable(const std::string &name, double *addr) {
-    std::string mapped = compute_registration_name(name);
+    std::string mapped = expr_compat::compute_registration_name(name);
     if (!impl_->symbol_table.add_variable(mapped, *addr)) {
-        const bool reserved = is_exprtk_reserved(name);
+        const bool reserved = expr_compat::is_exprtk_reserved(name);
         std::string detail;
         if (reserved) {
             // The mangled form already exists — most often because another