tests: fix stale steady-state default left behind by #28 by wshlavacek · Pull Request #36 · lanl/bngsim

wshlavacek · 2026-07-22T22:12:02Z

What

8a0609a ("steady_state: default to method=\"integration\" (#28)") flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods in tests/test_bngsim.cpp kept asserting the old default, so the native suite has reported 49/50 on every branch since — verified on a clean tree at 52c9022, unrelated to any in-flight work.

Two assertions in the function's first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Also reworded the comment block above the function, which described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default.

Why it matters beyond the count

A permanently-red suite is worse than a slightly smaller one — a genuine regression in this file arrives looking exactly like the failure everyone has learned to wave off.

Notes for review

The test's intent is preserved; only the expected default changed. Two things worth confirming:

Newton keeps direct coverage without the default block. The "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects "newton" explicitly to compare equilibrium concentrations against integration. The other three blocks are correct as written and are untouched.
The residual assertion still bites. It runs at the default tol = 1e-9, not the looser 1e-8 the explicit-integration block uses — the parity early-stop clears the tighter bound on the reversible model on its own, so this wasn't relaxed to make it pass.

Checks

tests/test_bngsim (Ninja, Release): 50/50, up from 49/50.
pytest -q python/tests via the pre-push hook: passed.

🤖 Generated with Claude Code

8a0609a flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods kept asserting the old default, so the native suite has reported 49/50 on every branch since -- a standing failure that makes a real regression easy to wave off as "the known one". Two assertions in the first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Only the expected default changes here; the rest of the function is correct as written and is left alone. Newton keeps its direct coverage without the default block: the "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects it explicitly to compare equilibrium concentrations against integration. The default's residual assertion still runs at the default tol = 1e-9 rather than the looser 1e-8 the explicit-integration block uses -- the parity early-stop clears the tighter bound on the reversible model on its own. The comment block above the function described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default. Checks: 50/50 in tests/test_bngsim (Ninja, Release).

…37f0c

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge onward, and no CI run could have said so. The failure's longevity was a consequence of its invisibility: a permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default because find_package(LAPACK QUIET) degrades to the reference dense solver when absent, so Linux builds either way. Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step then asserts the DENOMINATOR against a floor, which is the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled -- there is no #if around any of them -- so the total is a real invariant rather than a platform-dependent count, and the floor only ever moves up. Verified by building both sides locally with the exact flags: current main exits 1 at 49/50 naming the stale assertion, and the #36 fix exits 0 at 50/50. The guard was exercised against four captured logs -- pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash -- failing on the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt (test_validation, test_c_api, test_lapack_dense_linsol, test_seed_count_rounding, the NFsim pair) stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that actually drew blood.

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge until #36 fixed it, and no CI run could have said so. A permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default: find_package(LAPACK QUIET) degrades to the reference dense solver when absent, which the Linux CI run exercised (built-in dense LU, 50/50). Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step asserts the DENOMINATOR against a floor -- the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled (no #if around any of them), so the total is a real invariant, and the floor only ever moves up. Verified both directions: current main exits 1 at 49/50 naming the stale assertion, the #36 fix exits 0 at 50/50, and the guard was exercised against four captured logs (pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash), failing the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that drew blood.

…#39) (#47) The native-tests gate added in #38 built and ran test_bngsim only; every other binary in tests/CMakeLists.txt was built by nothing in CI and run by nothing anywhere. Driving the suite with `ctest` instead of a single named target picks up any add_test() binary automatically, and surfaced two latent breakages that had gone undetected precisely because nothing built them. tests/CMakeLists.txt: - Fix test_c_api registration. Its TEST_DATA_DIR + add_test lines had been copy-paste-slipped onto test_validation, so test_c_api compiled and was then dropped on the floor with no add_test(). test_c_api reads its model from argv[1] (not a TEST_DATA_DIR define), so register it as c_api_test with simple_decay.net; it is model-agnostic and exercises every branch. - Fix test_seed_count_rounding compile break (present since initial release). It includes bngsim/platform_compat.hpp (in include/) but only put src/ on its search path and does not link bngsim, so the standalone target never compiled. Add include/ to its include directories. .github/workflows/native-tests.yml: - Build all targets, then gate with `ctest --test-dir build --output-on-failure --no-tests=error` so new tests run without a workflow edit and a zero-test (mis-scoped) run fails. - Keep the false-green guard in two layers: a ctest binary-count floor for a binary silently losing its add_test(), plus test_bngsim's own case-count floor, which ctest cannot see since it treats the binary as one test — the exact #28/#36 failure mode. - Document the NFsim tradeoff explicitly: the NFsim tests exist only with BNGSIM_BUILD_NFSIM=ON, stay off this lean Linux leg, and are covered by windows-nfsim.yml.

wshlavacek · 2026-07-22T22:12:02Z

What

8a0609a ("steady_state: default to method=\"integration\" (#28)") flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods in tests/test_bngsim.cpp kept asserting the old default, so the native suite has reported 49/50 on every branch since — verified on a clean tree at 52c9022, unrelated to any in-flight work.

Two assertions in the function's first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Also reworded the comment block above the function, which described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default.

Why it matters beyond the count

A permanently-red suite is worse than a slightly smaller one — a genuine regression in this file arrives looking exactly like the failure everyone has learned to wave off.

Notes for review

The test's intent is preserved; only the expected default changed. Two things worth confirming:

Newton keeps direct coverage without the default block. The "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects "newton" explicitly to compare equilibrium concentrations against integration. The other three blocks are correct as written and are untouched.
The residual assertion still bites. It runs at the default tol = 1e-9, not the looser 1e-8 the explicit-integration block uses — the parity early-stop clears the tighter bound on the reversible model on its own, so this wasn't relaxed to make it pass.

Checks

tests/test_bngsim (Ninja, Release): 50/50, up from 49/50.
pytest -q python/tests via the pre-push hook: passed.

🤖 Generated with Claude Code

8a0609a flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods kept asserting the old default, so the native suite has reported 49/50 on every branch since -- a standing failure that makes a real regression easy to wave off as "the known one". Two assertions in the first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Only the expected default changes here; the rest of the function is correct as written and is left alone. Newton keeps its direct coverage without the default block: the "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects it explicitly to compare equilibrium concentrations against integration. The default's residual assertion still runs at the default tol = 1e-9 rather than the looser 1e-8 the explicit-integration block uses -- the parity early-stop clears the tighter bound on the reversible model on its own. The comment block above the function described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default. Checks: 50/50 in tests/test_bngsim (Ninja, Release).

…37f0c

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge onward, and no CI run could have said so. The failure's longevity was a consequence of its invisibility: a permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default because find_package(LAPACK QUIET) degrades to the reference dense solver when absent, so Linux builds either way. Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step then asserts the DENOMINATOR against a floor, which is the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled -- there is no #if around any of them -- so the total is a real invariant rather than a platform-dependent count, and the floor only ever moves up. Verified by building both sides locally with the exact flags: current main exits 1 at 49/50 naming the stale assertion, and the #36 fix exits 0 at 50/50. The guard was exercised against four captured logs -- pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash -- failing on the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt (test_validation, test_c_api, test_lapack_dense_linsol, test_seed_count_rounding, the NFsim pair) stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that actually drew blood.

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge until #36 fixed it, and no CI run could have said so. A permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default: find_package(LAPACK QUIET) degrades to the reference dense solver when absent, which the Linux CI run exercised (built-in dense LU, 50/50). Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step asserts the DENOMINATOR against a floor -- the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled (no #if around any of them), so the total is a real invariant, and the floor only ever moves up. Verified both directions: current main exits 1 at 49/50 naming the stale assertion, the #36 fix exits 0 at 50/50, and the guard was exercised against four captured logs (pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash), failing the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that drew blood.

…#39) (#47) The native-tests gate added in #38 built and ran test_bngsim only; every other binary in tests/CMakeLists.txt was built by nothing in CI and run by nothing anywhere. Driving the suite with `ctest` instead of a single named target picks up any add_test() binary automatically, and surfaced two latent breakages that had gone undetected precisely because nothing built them. tests/CMakeLists.txt: - Fix test_c_api registration. Its TEST_DATA_DIR + add_test lines had been copy-paste-slipped onto test_validation, so test_c_api compiled and was then dropped on the floor with no add_test(). test_c_api reads its model from argv[1] (not a TEST_DATA_DIR define), so register it as c_api_test with simple_decay.net; it is model-agnostic and exercises every branch. - Fix test_seed_count_rounding compile break (present since initial release). It includes bngsim/platform_compat.hpp (in include/) but only put src/ on its search path and does not link bngsim, so the standalone target never compiled. Add include/ to its include directories. .github/workflows/native-tests.yml: - Build all targets, then gate with `ctest --test-dir build --output-on-failure --no-tests=error` so new tests run without a workflow edit and a zero-test (mis-scoped) run fails. - Keep the false-green guard in two layers: a ctest binary-count floor for a binary silently losing its add_test(), plus test_bngsim's own case-count floor, which ctest cannot see since it treats the binary as one test — the exact #28/#36 failure mode. - Document the NFsim tradeoff explicitly: the NFsim tests exist only with BNGSIM_BUILD_NFSIM=ON, stay off this lean Linux leg, and are covered by windows-nfsim.yml.

wshlavacek · 2026-07-22T22:12:02Z

What

8a0609a ("steady_state: default to method=\"integration\" (#28)") flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods in tests/test_bngsim.cpp kept asserting the old default, so the native suite has reported 49/50 on every branch since — verified on a clean tree at 52c9022, unrelated to any in-flight work.

Two assertions in the function's first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Also reworded the comment block above the function, which described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default.

Why it matters beyond the count

A permanently-red suite is worse than a slightly smaller one — a genuine regression in this file arrives looking exactly like the failure everyone has learned to wave off.

Notes for review

The test's intent is preserved; only the expected default changed. Two things worth confirming:

Newton keeps direct coverage without the default block. The "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects "newton" explicitly to compare equilibrium concentrations against integration. The other three blocks are correct as written and are untouched.
The residual assertion still bites. It runs at the default tol = 1e-9, not the looser 1e-8 the explicit-integration block uses — the parity early-stop clears the tighter bound on the reversible model on its own, so this wasn't relaxed to make it pass.

Checks

tests/test_bngsim (Ninja, Release): 50/50, up from 49/50.
pytest -q python/tests via the pre-push hook: passed.

🤖 Generated with Claude Code

8a0609a flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods kept asserting the old default, so the native suite has reported 49/50 on every branch since -- a standing failure that makes a real regression easy to wave off as "the known one". Two assertions in the first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Only the expected default changes here; the rest of the function is correct as written and is left alone. Newton keeps its direct coverage without the default block: the "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects it explicitly to compare equilibrium concentrations against integration. The default's residual assertion still runs at the default tol = 1e-9 rather than the looser 1e-8 the explicit-integration block uses -- the parity early-stop clears the tighter bound on the reversible model on its own. The comment block above the function described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default. Checks: 50/50 in tests/test_bngsim (Ninja, Release).

…37f0c

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge onward, and no CI run could have said so. The failure's longevity was a consequence of its invisibility: a permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default because find_package(LAPACK QUIET) degrades to the reference dense solver when absent, so Linux builds either way. Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step then asserts the DENOMINATOR against a floor, which is the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled -- there is no #if around any of them -- so the total is a real invariant rather than a platform-dependent count, and the floor only ever moves up. Verified by building both sides locally with the exact flags: current main exits 1 at 49/50 naming the stale assertion, and the #36 fix exits 0 at 50/50. The guard was exercised against four captured logs -- pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash -- failing on the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt (test_validation, test_c_api, test_lapack_dense_linsol, test_seed_count_rounding, the NFsim pair) stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that actually drew blood.

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge until #36 fixed it, and no CI run could have said so. A permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default: find_package(LAPACK QUIET) degrades to the reference dense solver when absent, which the Linux CI run exercised (built-in dense LU, 50/50). Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step asserts the DENOMINATOR against a floor -- the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled (no #if around any of them), so the total is a real invariant, and the floor only ever moves up. Verified both directions: current main exits 1 at 49/50 naming the stale assertion, the #36 fix exits 0 at 50/50, and the guard was exercised against four captured logs (pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash), failing the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that drew blood.

…#39) (#47) The native-tests gate added in #38 built and ran test_bngsim only; every other binary in tests/CMakeLists.txt was built by nothing in CI and run by nothing anywhere. Driving the suite with `ctest` instead of a single named target picks up any add_test() binary automatically, and surfaced two latent breakages that had gone undetected precisely because nothing built them. tests/CMakeLists.txt: - Fix test_c_api registration. Its TEST_DATA_DIR + add_test lines had been copy-paste-slipped onto test_validation, so test_c_api compiled and was then dropped on the floor with no add_test(). test_c_api reads its model from argv[1] (not a TEST_DATA_DIR define), so register it as c_api_test with simple_decay.net; it is model-agnostic and exercises every branch. - Fix test_seed_count_rounding compile break (present since initial release). It includes bngsim/platform_compat.hpp (in include/) but only put src/ on its search path and does not link bngsim, so the standalone target never compiled. Add include/ to its include directories. .github/workflows/native-tests.yml: - Build all targets, then gate with `ctest --test-dir build --output-on-failure --no-tests=error` so new tests run without a workflow edit and a zero-test (mis-scoped) run fails. - Keep the false-green guard in two layers: a ctest binary-count floor for a binary silently losing its add_test(), plus test_bngsim's own case-count floor, which ctest cannot see since it treats the binary as one test — the exact #28/#36 failure mode. - Document the NFsim tradeoff explicitly: the NFsim tests exist only with BNGSIM_BUILD_NFSIM=ON, stay off this lean Linux leg, and are covered by windows-nfsim.yml.

wshlavacek · 2026-07-22T22:12:02Z

What

8a0609a ("steady_state: default to method=\"integration\" (#28)") flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods in tests/test_bngsim.cpp kept asserting the old default, so the native suite has reported 49/50 on every branch since — verified on a clean tree at 52c9022, unrelated to any in-flight work.

Two assertions in the function's first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Also reworded the comment block above the function, which described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default.

Why it matters beyond the count

A permanently-red suite is worse than a slightly smaller one — a genuine regression in this file arrives looking exactly like the failure everyone has learned to wave off.

Notes for review

The test's intent is preserved; only the expected default changed. Two things worth confirming:

Newton keeps direct coverage without the default block. The "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects "newton" explicitly to compare equilibrium concentrations against integration. The other three blocks are correct as written and are untouched.
The residual assertion still bites. It runs at the default tol = 1e-9, not the looser 1e-8 the explicit-integration block uses — the parity early-stop clears the tighter bound on the reversible model on its own, so this wasn't relaxed to make it pass.

Checks

tests/test_bngsim (Ninja, Release): 50/50, up from 49/50.
pytest -q python/tests via the pre-push hook: passed.

🤖 Generated with Claude Code

8a0609a flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods kept asserting the old default, so the native suite has reported 49/50 on every branch since -- a standing failure that makes a real regression easy to wave off as "the known one". Two assertions in the first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Only the expected default changes here; the rest of the function is correct as written and is left alone. Newton keeps its direct coverage without the default block: the "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects it explicitly to compare equilibrium concentrations against integration. The default's residual assertion still runs at the default tol = 1e-9 rather than the looser 1e-8 the explicit-integration block uses -- the parity early-stop clears the tighter bound on the reversible model on its own. The comment block above the function described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default. Checks: 50/50 in tests/test_bngsim (Ninja, Release).

…37f0c

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge onward, and no CI run could have said so. The failure's longevity was a consequence of its invisibility: a permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default because find_package(LAPACK QUIET) degrades to the reference dense solver when absent, so Linux builds either way. Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step then asserts the DENOMINATOR against a floor, which is the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled -- there is no #if around any of them -- so the total is a real invariant rather than a platform-dependent count, and the floor only ever moves up. Verified by building both sides locally with the exact flags: current main exits 1 at 49/50 naming the stale assertion, and the #36 fix exits 0 at 50/50. The guard was exercised against four captured logs -- pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash -- failing on the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt (test_validation, test_c_api, test_lapack_dense_linsol, test_seed_count_rounding, the NFsim pair) stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that actually drew blood.

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge until #36 fixed it, and no CI run could have said so. A permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default: find_package(LAPACK QUIET) degrades to the reference dense solver when absent, which the Linux CI run exercised (built-in dense LU, 50/50). Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step asserts the DENOMINATOR against a floor -- the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled (no #if around any of them), so the total is a real invariant, and the floor only ever moves up. Verified both directions: current main exits 1 at 49/50 naming the stale assertion, the #36 fix exits 0 at 50/50, and the guard was exercised against four captured logs (pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash), failing the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that drew blood.

…#39) (#47) The native-tests gate added in #38 built and ran test_bngsim only; every other binary in tests/CMakeLists.txt was built by nothing in CI and run by nothing anywhere. Driving the suite with `ctest` instead of a single named target picks up any add_test() binary automatically, and surfaced two latent breakages that had gone undetected precisely because nothing built them. tests/CMakeLists.txt: - Fix test_c_api registration. Its TEST_DATA_DIR + add_test lines had been copy-paste-slipped onto test_validation, so test_c_api compiled and was then dropped on the floor with no add_test(). test_c_api reads its model from argv[1] (not a TEST_DATA_DIR define), so register it as c_api_test with simple_decay.net; it is model-agnostic and exercises every branch. - Fix test_seed_count_rounding compile break (present since initial release). It includes bngsim/platform_compat.hpp (in include/) but only put src/ on its search path and does not link bngsim, so the standalone target never compiled. Add include/ to its include directories. .github/workflows/native-tests.yml: - Build all targets, then gate with `ctest --test-dir build --output-on-failure --no-tests=error` so new tests run without a workflow edit and a zero-test (mis-scoped) run fails. - Keep the false-green guard in two layers: a ctest binary-count floor for a binary silently losing its add_test(), plus test_bngsim's own case-count floor, which ctest cannot see since it treats the binary as one test — the exact #28/#36 failure mode. - Document the NFsim tradeoff explicitly: the NFsim tests exist only with BNGSIM_BUILD_NFSIM=ON, stay off this lean Linux leg, and are covered by windows-nfsim.yml.

wshlavacek · 2026-07-22T22:12:02Z

What

8a0609a ("steady_state: default to method=\"integration\" (#28)") flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods in tests/test_bngsim.cpp kept asserting the old default, so the native suite has reported 49/50 on every branch since — verified on a clean tree at 52c9022, unrelated to any in-flight work.

Two assertions in the function's first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Also reworded the comment block above the function, which described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default.

Why it matters beyond the count

A permanently-red suite is worse than a slightly smaller one — a genuine regression in this file arrives looking exactly like the failure everyone has learned to wave off.

Notes for review

The test's intent is preserved; only the expected default changed. Two things worth confirming:

Newton keeps direct coverage without the default block. The "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects "newton" explicitly to compare equilibrium concentrations against integration. The other three blocks are correct as written and are untouched.
The residual assertion still bites. It runs at the default tol = 1e-9, not the looser 1e-8 the explicit-integration block uses — the parity early-stop clears the tighter bound on the reversible model on its own, so this wasn't relaxed to make it pass.

Checks

tests/test_bngsim (Ninja, Release): 50/50, up from 49/50.
pytest -q python/tests via the pre-push hook: passed.

🤖 Generated with Claude Code

8a0609a flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods kept asserting the old default, so the native suite has reported 49/50 on every branch since -- a standing failure that makes a real regression easy to wave off as "the known one". Two assertions in the first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Only the expected default changes here; the rest of the function is correct as written and is left alone. Newton keeps its direct coverage without the default block: the "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects it explicitly to compare equilibrium concentrations against integration. The default's residual assertion still runs at the default tol = 1e-9 rather than the looser 1e-8 the explicit-integration block uses -- the parity early-stop clears the tighter bound on the reversible model on its own. The comment block above the function described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default. Checks: 50/50 in tests/test_bngsim (Ninja, Release).

…37f0c

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge onward, and no CI run could have said so. The failure's longevity was a consequence of its invisibility: a permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default because find_package(LAPACK QUIET) degrades to the reference dense solver when absent, so Linux builds either way. Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step then asserts the DENOMINATOR against a floor, which is the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled -- there is no #if around any of them -- so the total is a real invariant rather than a platform-dependent count, and the floor only ever moves up. Verified by building both sides locally with the exact flags: current main exits 1 at 49/50 naming the stale assertion, and the #36 fix exits 0 at 50/50. The guard was exercised against four captured logs -- pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash -- failing on the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt (test_validation, test_c_api, test_lapack_dense_linsol, test_seed_count_rounding, the NFsim pair) stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that actually drew blood.

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge until #36 fixed it, and no CI run could have said so. A permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default: find_package(LAPACK QUIET) degrades to the reference dense solver when absent, which the Linux CI run exercised (built-in dense LU, 50/50). Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step asserts the DENOMINATOR against a floor -- the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled (no #if around any of them), so the total is a real invariant, and the floor only ever moves up. Verified both directions: current main exits 1 at 49/50 naming the stale assertion, the #36 fix exits 0 at 50/50, and the guard was exercised against four captured logs (pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash), failing the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that drew blood.

…#39) (#47) The native-tests gate added in #38 built and ran test_bngsim only; every other binary in tests/CMakeLists.txt was built by nothing in CI and run by nothing anywhere. Driving the suite with `ctest` instead of a single named target picks up any add_test() binary automatically, and surfaced two latent breakages that had gone undetected precisely because nothing built them. tests/CMakeLists.txt: - Fix test_c_api registration. Its TEST_DATA_DIR + add_test lines had been copy-paste-slipped onto test_validation, so test_c_api compiled and was then dropped on the floor with no add_test(). test_c_api reads its model from argv[1] (not a TEST_DATA_DIR define), so register it as c_api_test with simple_decay.net; it is model-agnostic and exercises every branch. - Fix test_seed_count_rounding compile break (present since initial release). It includes bngsim/platform_compat.hpp (in include/) but only put src/ on its search path and does not link bngsim, so the standalone target never compiled. Add include/ to its include directories. .github/workflows/native-tests.yml: - Build all targets, then gate with `ctest --test-dir build --output-on-failure --no-tests=error` so new tests run without a workflow edit and a zero-test (mis-scoped) run fails. - Keep the false-green guard in two layers: a ctest binary-count floor for a binary silently losing its add_test(), plus test_bngsim's own case-count floor, which ctest cannot see since it treats the binary as one test — the exact #28/#36 failure mode. - Document the NFsim tradeoff explicitly: the NFsim tests exist only with BNGSIM_BUILD_NFSIM=ON, stay off this lean Linux leg, and are covered by windows-nfsim.yml.

wshlavacek · 2026-07-22T22:12:02Z

What

8a0609a ("steady_state: default to method=\"integration\" (#28)") flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods in tests/test_bngsim.cpp kept asserting the old default, so the native suite has reported 49/50 on every branch since — verified on a clean tree at 52c9022, unrelated to any in-flight work.

Two assertions in the function's first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Also reworded the comment block above the function, which described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default.

Why it matters beyond the count

A permanently-red suite is worse than a slightly smaller one — a genuine regression in this file arrives looking exactly like the failure everyone has learned to wave off.

Notes for review

The test's intent is preserved; only the expected default changed. Two things worth confirming:

Newton keeps direct coverage without the default block. The "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects "newton" explicitly to compare equilibrium concentrations against integration. The other three blocks are correct as written and are untouched.
The residual assertion still bites. It runs at the default tol = 1e-9, not the looser 1e-8 the explicit-integration block uses — the parity early-stop clears the tighter bound on the reversible model on its own, so this wasn't relaxed to make it pass.

Checks

tests/test_bngsim (Ninja, Release): 50/50, up from 49/50.
pytest -q python/tests via the pre-push hook: passed.

🤖 Generated with Claude Code

8a0609a flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods kept asserting the old default, so the native suite has reported 49/50 on every branch since -- a standing failure that makes a real regression easy to wave off as "the known one". Two assertions in the first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Only the expected default changes here; the rest of the function is correct as written and is left alone. Newton keeps its direct coverage without the default block: the "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects it explicitly to compare equilibrium concentrations against integration. The default's residual assertion still runs at the default tol = 1e-9 rather than the looser 1e-8 the explicit-integration block uses -- the parity early-stop clears the tighter bound on the reversible model on its own. The comment block above the function described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default. Checks: 50/50 in tests/test_bngsim (Ninja, Release).

…37f0c

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge onward, and no CI run could have said so. The failure's longevity was a consequence of its invisibility: a permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default because find_package(LAPACK QUIET) degrades to the reference dense solver when absent, so Linux builds either way. Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step then asserts the DENOMINATOR against a floor, which is the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled -- there is no #if around any of them -- so the total is a real invariant rather than a platform-dependent count, and the floor only ever moves up. Verified by building both sides locally with the exact flags: current main exits 1 at 49/50 naming the stale assertion, and the #36 fix exits 0 at 50/50. The guard was exercised against four captured logs -- pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash -- failing on the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt (test_validation, test_c_api, test_lapack_dense_linsol, test_seed_count_rounding, the NFsim pair) stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that actually drew blood.

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge until #36 fixed it, and no CI run could have said so. A permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default: find_package(LAPACK QUIET) degrades to the reference dense solver when absent, which the Linux CI run exercised (built-in dense LU, 50/50). Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step asserts the DENOMINATOR against a floor -- the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled (no #if around any of them), so the total is a real invariant, and the floor only ever moves up. Verified both directions: current main exits 1 at 49/50 naming the stale assertion, the #36 fix exits 0 at 50/50, and the guard was exercised against four captured logs (pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash), failing the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that drew blood.

…#39) (#47) The native-tests gate added in #38 built and ran test_bngsim only; every other binary in tests/CMakeLists.txt was built by nothing in CI and run by nothing anywhere. Driving the suite with `ctest` instead of a single named target picks up any add_test() binary automatically, and surfaced two latent breakages that had gone undetected precisely because nothing built them. tests/CMakeLists.txt: - Fix test_c_api registration. Its TEST_DATA_DIR + add_test lines had been copy-paste-slipped onto test_validation, so test_c_api compiled and was then dropped on the floor with no add_test(). test_c_api reads its model from argv[1] (not a TEST_DATA_DIR define), so register it as c_api_test with simple_decay.net; it is model-agnostic and exercises every branch. - Fix test_seed_count_rounding compile break (present since initial release). It includes bngsim/platform_compat.hpp (in include/) but only put src/ on its search path and does not link bngsim, so the standalone target never compiled. Add include/ to its include directories. .github/workflows/native-tests.yml: - Build all targets, then gate with `ctest --test-dir build --output-on-failure --no-tests=error` so new tests run without a workflow edit and a zero-test (mis-scoped) run fails. - Keep the false-green guard in two layers: a ctest binary-count floor for a binary silently losing its add_test(), plus test_bngsim's own case-count floor, which ctest cannot see since it treats the binary as one test — the exact #28/#36 failure mode. - Document the NFsim tradeoff explicitly: the NFsim tests exist only with BNGSIM_BUILD_NFSIM=ON, stay off this lean Linux leg, and are covered by windows-nfsim.yml.

wshlavacek · 2026-07-22T22:12:02Z

What

8a0609a ("steady_state: default to method=\"integration\" (#28)") flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods in tests/test_bngsim.cpp kept asserting the old default, so the native suite has reported 49/50 on every branch since — verified on a clean tree at 52c9022, unrelated to any in-flight work.

Two assertions in the function's first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Also reworded the comment block above the function, which described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default.

Why it matters beyond the count

A permanently-red suite is worse than a slightly smaller one — a genuine regression in this file arrives looking exactly like the failure everyone has learned to wave off.

Notes for review

The test's intent is preserved; only the expected default changed. Two things worth confirming:

Newton keeps direct coverage without the default block. The "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects "newton" explicitly to compare equilibrium concentrations against integration. The other three blocks are correct as written and are untouched.
The residual assertion still bites. It runs at the default tol = 1e-9, not the looser 1e-8 the explicit-integration block uses — the parity early-stop clears the tighter bound on the reversible model on its own, so this wasn't relaxed to make it pass.

Checks

tests/test_bngsim (Ninja, Release): 50/50, up from 49/50.
pytest -q python/tests via the pre-push hook: passed.

🤖 Generated with Claude Code

8a0609a flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods kept asserting the old default, so the native suite has reported 49/50 on every branch since -- a standing failure that makes a real regression easy to wave off as "the known one". Two assertions in the first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Only the expected default changes here; the rest of the function is correct as written and is left alone. Newton keeps its direct coverage without the default block: the "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects it explicitly to compare equilibrium concentrations against integration. The default's residual assertion still runs at the default tol = 1e-9 rather than the looser 1e-8 the explicit-integration block uses -- the parity early-stop clears the tighter bound on the reversible model on its own. The comment block above the function described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default. Checks: 50/50 in tests/test_bngsim (Ninja, Release).

…37f0c

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge onward, and no CI run could have said so. The failure's longevity was a consequence of its invisibility: a permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default because find_package(LAPACK QUIET) degrades to the reference dense solver when absent, so Linux builds either way. Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step then asserts the DENOMINATOR against a floor, which is the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled -- there is no #if around any of them -- so the total is a real invariant rather than a platform-dependent count, and the floor only ever moves up. Verified by building both sides locally with the exact flags: current main exits 1 at 49/50 naming the stale assertion, and the #36 fix exits 0 at 50/50. The guard was exercised against four captured logs -- pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash -- failing on the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt (test_validation, test_c_api, test_lapack_dense_linsol, test_seed_count_rounding, the NFsim pair) stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that actually drew blood.

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge until #36 fixed it, and no CI run could have said so. A permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default: find_package(LAPACK QUIET) degrades to the reference dense solver when absent, which the Linux CI run exercised (built-in dense LU, 50/50). Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step asserts the DENOMINATOR against a floor -- the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled (no #if around any of them), so the total is a real invariant, and the floor only ever moves up. Verified both directions: current main exits 1 at 49/50 naming the stale assertion, the #36 fix exits 0 at 50/50, and the guard was exercised against four captured logs (pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash), failing the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that drew blood.

…#39) (#47) The native-tests gate added in #38 built and ran test_bngsim only; every other binary in tests/CMakeLists.txt was built by nothing in CI and run by nothing anywhere. Driving the suite with `ctest` instead of a single named target picks up any add_test() binary automatically, and surfaced two latent breakages that had gone undetected precisely because nothing built them. tests/CMakeLists.txt: - Fix test_c_api registration. Its TEST_DATA_DIR + add_test lines had been copy-paste-slipped onto test_validation, so test_c_api compiled and was then dropped on the floor with no add_test(). test_c_api reads its model from argv[1] (not a TEST_DATA_DIR define), so register it as c_api_test with simple_decay.net; it is model-agnostic and exercises every branch. - Fix test_seed_count_rounding compile break (present since initial release). It includes bngsim/platform_compat.hpp (in include/) but only put src/ on its search path and does not link bngsim, so the standalone target never compiled. Add include/ to its include directories. .github/workflows/native-tests.yml: - Build all targets, then gate with `ctest --test-dir build --output-on-failure --no-tests=error` so new tests run without a workflow edit and a zero-test (mis-scoped) run fails. - Keep the false-green guard in two layers: a ctest binary-count floor for a binary silently losing its add_test(), plus test_bngsim's own case-count floor, which ctest cannot see since it treats the binary as one test — the exact #28/#36 failure mode. - Document the NFsim tradeoff explicitly: the NFsim tests exist only with BNGSIM_BUILD_NFSIM=ON, stay off this lean Linux leg, and are covered by windows-nfsim.yml.

wshlavacek · 2026-07-22T22:12:02Z

What

8a0609a ("steady_state: default to method=\"integration\" (#28)") flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods in tests/test_bngsim.cpp kept asserting the old default, so the native suite has reported 49/50 on every branch since — verified on a clean tree at 52c9022, unrelated to any in-flight work.

Two assertions in the function's first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Also reworded the comment block above the function, which described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default.

Why it matters beyond the count

A permanently-red suite is worse than a slightly smaller one — a genuine regression in this file arrives looking exactly like the failure everyone has learned to wave off.

Notes for review

The test's intent is preserved; only the expected default changed. Two things worth confirming:

Newton keeps direct coverage without the default block. The "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects "newton" explicitly to compare equilibrium concentrations against integration. The other three blocks are correct as written and are untouched.
The residual assertion still bites. It runs at the default tol = 1e-9, not the looser 1e-8 the explicit-integration block uses — the parity early-stop clears the tighter bound on the reversible model on its own, so this wasn't relaxed to make it pass.

Checks

tests/test_bngsim (Ninja, Release): 50/50, up from 49/50.
pytest -q python/tests via the pre-push hook: passed.

🤖 Generated with Claude Code

8a0609a flipped SteadyStateOptions::method from "newton" to "integration" and updated the Python side, but not the C++ one. test_find_steady_state_methods kept asserting the old default, so the native suite has reported 49/50 on every branch since -- a standing failure that makes a real regression easy to wave off as "the known one". Two assertions in the first block were stale, both for the same reason: the default's method string, and the method_used it echoes back. Only the expected default changes here; the rest of the function is correct as written and is left alone. Newton keeps its direct coverage without the default block: the "kinsol" block asserts it converges and echoes the canonical method_used == "newton", and the parity block selects it explicitly to compare equilibrium concentrations against integration. The default's residual assertion still runs at the default tol = 1e-9 rather than the looser 1e-8 the explicit-integration block uses -- the parity early-stop clears the tighter bound on the reversible model on its own. The comment block above the function described the two-tier integrate-then-polish path as the default; it now describes the parity early-stop and cites the 1.4-3.9x net cost #28 measured as why the polish stopped being the default. Checks: 50/50 in tests/test_bngsim (Ninja, Release).

…37f0c

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge onward, and no CI run could have said so. The failure's longevity was a consequence of its invisibility: a permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default because find_package(LAPACK QUIET) degrades to the reference dense solver when absent, so Linux builds either way. Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step then asserts the DENOMINATOR against a floor, which is the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled -- there is no #if around any of them -- so the total is a real invariant rather than a platform-dependent count, and the floor only ever moves up. Verified by building both sides locally with the exact flags: current main exits 1 at 49/50 naming the stale assertion, and the #36 fix exits 0 at 50/50. The guard was exercised against four captured logs -- pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash -- failing on the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt (test_validation, test_c_api, test_lapack_dense_linsol, test_seed_count_rounding, the NFsim pair) stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that actually drew blood.

tests/test_bngsim had no gate at any stage. Every workflow that compiles the project passes BNGSIM_BUILD_TESTS=OFF -- mir.yml, windows-nfsim.yml and windows-tail.yml all do -- and no job ever invoked the binary. The pre-push hook covers python/tests; the C++ side had no local or remote equivalent, so the suite's only gate was a developer remembering to build it by hand. #28 is what that costs. It flipped SteadyStateOptions::method from "newton" to "integration", updated the Python side, and left the C++ assertion in test_find_steady_state_methods pinned to the old default. The suite sat at 49/50 on every branch from that merge until #36 fixed it, and no CI run could have said so. A permanently-red suite is worse than a smaller green one, because a genuine regression arrives looking exactly like the one everyone has learned to wave off. The job is Linux-only and lean. KLU, NFsim and RuleMonkey are OFF -- test_bngsim links none of them, and skipping the vendored builds takes the target count from 146 to 81. The wheels/MIR/Windows jobs already cover those subsystems. LAPACK stays at its default: find_package(LAPACK QUIET) degrades to the reference dense solver when absent, which the Linux CI run exercised (built-in dense LU, 50/50). Two things fail the job, not one. test_bngsim exits nonzero unless tests_passed == tests_run, and pipefail carries that through the tee. A second step asserts the DENOMINATOR against a floor -- the failure mode an exit code cannot see: a suite that goes green because tests stopped being registered. Every RUN_TEST is unconditionally compiled (no #if around any of them), so the total is a real invariant, and the floor only ever moves up. Verified both directions: current main exits 1 at 49/50 naming the stale assertion, the #36 fix exits 0 at 50/50, and the guard was exercised against four captured logs (pass, the 49/50 stale run, a doctored 41/41 shrink, and a no-summary crash), failing the last two and passing the first two. Not covered: the other native binaries in tests/CMakeLists.txt stay ungated. Extending this to ctest is a fair follow-up; this closes the gap that drew blood.

…#39) (#47) The native-tests gate added in #38 built and ran test_bngsim only; every other binary in tests/CMakeLists.txt was built by nothing in CI and run by nothing anywhere. Driving the suite with `ctest` instead of a single named target picks up any add_test() binary automatically, and surfaced two latent breakages that had gone undetected precisely because nothing built them. tests/CMakeLists.txt: - Fix test_c_api registration. Its TEST_DATA_DIR + add_test lines had been copy-paste-slipped onto test_validation, so test_c_api compiled and was then dropped on the floor with no add_test(). test_c_api reads its model from argv[1] (not a TEST_DATA_DIR define), so register it as c_api_test with simple_decay.net; it is model-agnostic and exercises every branch. - Fix test_seed_count_rounding compile break (present since initial release). It includes bngsim/platform_compat.hpp (in include/) but only put src/ on its search path and does not link bngsim, so the standalone target never compiled. Add include/ to its include directories. .github/workflows/native-tests.yml: - Build all targets, then gate with `ctest --test-dir build --output-on-failure --no-tests=error` so new tests run without a workflow edit and a zero-test (mis-scoped) run fails. - Keep the false-green guard in two layers: a ctest binary-count floor for a binary silently losing its add_test(), plus test_bngsim's own case-count floor, which ctest cannot see since it treats the binary as one test — the exact #28/#36 failure mode. - Document the NFsim tradeoff explicitly: the NFsim tests exist only with BNGSIM_BUILD_NFSIM=ON, stay off this lean Linux leg, and are covered by windows-nfsim.yml.

wshlavacek added 2 commits July 22, 2026 16:06

Merge remote-tracking branch 'origin/main' into claude/stoic-turing-c…
bf1de01
…37f0c

wshlavacek mentioned this pull request Jul 22, 2026
ci: gate the C++ unit suite, which nothing in CI built #38
Merged

wshlavacek merged commit c34c0b0 into mainJul 22, 2026
1 check passed

wshlavacek mentioned this pull request Jul 22, 2026
CI: extend the native-test gate from test_bngsim to the full ctest suite #39
Closed

wshlavacek deleted the claude/stoic-turing-c37f0c branch July 22, 2026 22:58

This was referenced Jul 22, 2026
ODE: expose force_sparse_linear_solver, the missing counterpart to force_dense_linear_solver #29
Closed
ci: drive the native suite through ctest, fix two rotted test targets (#39) #47
Merged

This was referenced Aug 2, 2026
perf(rulemonkey): drop the XML re-bake, let upstream propagate set_param (#115) #140
Merged
ci(python-tests): run the whole Python suite on Linux and macOS (#169) #175
Merged

wshlavacek mentioned this pull request Aug 10, 2026
test(lapack): run the BLAS dense solver on Linux, and stop counting its skips as passes (#269) #284
Merged