Skip to content

tests: pin the steady state the two unmended quotients reach from a jump; warn on another SUNDIALS (#1335) - #1352

Open
Montana wants to merge 3 commits into
lanl:mainfrom
Montana:test/1335-dq-on-a-jump
Open

Montana wants to merge 3 commits into
lanl:mainfrom
Montana:test/1335-dq-on-a-jump

Conversation

@Montana

@Montana Montana commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Closes #1335.

#1335 lists three paths that #1302 (#1002) left without a test or a decision. No wrong answer was known on any of them, and this PR finds none. It pins each one and documents the third.

1 and 2: the two steady-state quotients with no jump mend

#1302 mends a difference-quotient row whose step crosses a jump for runs (dense and colored) and for the dense steady-state march. Two quotients were left as they were:

  • KINSOL's own quotient: method="newton" with jacobian="fd". A declined law sends newton to the march instead, so that path is reached only with "fd".
  • The steady-state march's colored sparse quotient: KLU.

New tests in test_difference_quotient_jump.py run each path, plus KINSOL on KLU, from:

Each is checked against its closed form (< 1e-4 relative, the file's OK), and the held species against exactly 0.

Each test also asserts the path it took: method_used, linear_solver (dense/klu) and solver_jacobian_source == "finite-difference". Prototyping showed why that matters: with a declined law, method="newton" silently converges by the march instead, and would have "tested" KINSOL without touching it.

cases on main
threshold starts × 2 sources × 3 paths 18 all pass
held species × 2 models × 3 paths 6 all pass

The KLU paths skip on a KLU-off build, with the reason conftest.py already declares.

These pin the current behaviour rather than fix a defect. They are the "nothing pins that" #1335 asks for, and they agree with #1302's 72 and 24 variants that found no wrong answer.

3: SUNDIALS other than the vendored 7.2

include/bngsim/dq_jacobian.hpp reproduces cvLsDenseDQJac of the vendored SUNDIALS (v7.2.1) bit for bit. With -DBNGSIM_USE_SYSTEM_SUNDIALS=ON and another release, it is still a valid quotient, but not necessarily that release's own. #1335 asks for a CI leg or a documented refusal. This does the documented half:

  • CMakeLists.txt: in the system-SUNDIALS branch, compare SUNDIALS_VERSION's major.minor with VENDOR.json's authoritative_release_tag, and warn on a mismatch:
    CMake Warning: System SUNDIALS 7.3.0 is not the vendored v7.2.1.  bngsim's dense
    difference quotient (jacobian="fd", or a law the closed form declines) reproduces
    CVODE 7.2's.  With this release it is still a valid quotient, but can differ from
    this CVODE's own in the last digits, and no CI leg tests the combination (issue #1335).
    
  • docs/installation.md: the same note beside BNGSIM_USE_SYSTEM_SUNDIALS.

Verified by running the block in cmake -P with SUNDIALS_VERSION set to each of these:

version warns
7.2.1 no
7.2.0 no
7.3.0 yes
6.7.0 yes
unset yes

The default (vendored) configure does not enter the branch. Every CI leg uses the vendored path, so the full configure is exercised there.

A warning, not a refusal: a system SUNDIALS of another 7.x is supported (find_package(SUNDIALS 7.0 …)), and refusing it would drop HPC builds that link a site SUNDIALS.

Measured

test_difference_quotient_jump.py with the new cases, plus test_skip_audit.py, test_ci_run_list_coverage.py and test_docs_api_examples.py: 120 passed.

…ump; warn on another SUNDIALS (lanl#1335)

KINSOL's difference quotient (method="newton", jacobian="fd") and the
steady-state march's colored sparse one (forced KLU) have no mend for a
step across a jump, and nothing pinned that they need none. They now
start on a threshold and 1e-9 either side of it, and beside a species
held at 0, and are checked against closed forms on each path, each test
asserting the path it took. All pass on main: no wrong answer, as lanl#1302
found.

dq_jacobian.hpp reproduces the vendored CVODE 7.2's dense quotient. A
configure against a system SUNDIALS of another release now says so, and
docs/installation.md documents it, in place of a CI leg.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests: KINSOL's and the steady-state march's difference quotients on a jump, and SUNDIALS other than 7.2, are not covered

1 participant