Skip to content

fix isdetailedbalanced tol issue#1376

Merged
isaacsas merged 1 commit intoSciML:version_16_newfrom
isaacsas:isDB_test_tol_investigation
Feb 5, 2026
Merged

fix isdetailedbalanced tol issue#1376
isaacsas merged 1 commit intoSciML:version_16_newfrom
isaacsas:isDB_test_tol_investigation

Conversation

@isaacsas
Copy link
Copy Markdown
Member

@isaacsas isaacsas commented Feb 4, 2026

Fix substitutevals to return numeric types instead of Num

Problem

isdetailedbalanced was failing with abstol=0 for systems that mathematically satisfy detailed balance, requiring abstol=1e-12 as a workaround.

Root Cause

substitutevals returned Symbolics Num-wrapped values even when substituting concrete numbers. This caused adjacencymat to return Matrix{Num} instead of Matrix{Float64}, triggering a bug in Symbolics.jl's isapprox for Num types (it compares err to 0.0 instead of comparing original values, breaking relative tolerance handling).

Fix

Unwrap substitution results with value() so adjacencymat, fluxmat, and massactionvector correctly return numeric types when parameter maps are provided (as their docstrings already promised).

Testing

All 168 tests in crn_theory.jl and network_properties.jl pass.

substitutevals was returning Symbolics Num-wrapped values even when
all parameters were substituted with concrete numeric values. This
caused adjacencymat, fluxmat, and massactionvector to return Matrix{Num}
instead of Matrix{Float64} when parameter maps were provided.

This triggered a bug in Symbolics.jl isapprox for Num types, which
incorrectly handles relative tolerance. This caused isdetailedbalanced
to fail with abstol=0 even for systems that satisfy detailed balance.

The fix unwraps the substitution result with value(), so these functions
now correctly return numeric types as documented. This also restores
isdetailedbalanced to use abstol=0 with proper rtol behavior.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@isaacsas isaacsas mentioned this pull request Feb 4, 2026
43 tasks
@isaacsas isaacsas merged commit c197eca into SciML:version_16_new Feb 5, 2026
16 of 19 checks passed
@isaacsas isaacsas deleted the isDB_test_tol_investigation branch February 5, 2026 00:55
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.

1 participant