Skip to content

Bk/improve error for non integer stoichiometry#1389

Merged
isaacsas merged 5 commits intoSciML:masterfrom
calebjubal:bk/improve-error-for-non-integer-stoichiometry
Feb 14, 2026
Merged

Bk/improve error for non integer stoichiometry#1389
isaacsas merged 5 commits intoSciML:masterfrom
calebjubal:bk/improve-error-for-non-integer-stoichiometry

Conversation

@calebjubal
Copy link
Copy Markdown
Contributor

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

Added helper function:

image

Added error check:

image

Added tests

image

closes #1286

@TorkelE
Copy link
Copy Markdown
Member

TorkelE commented Feb 7, 2026

Hello, thanks a lot for the PR. In this case, however, non-integer stoichiometries for bifurcation diagrams are something that we do want to support.

@TorkelE TorkelE closed this Feb 7, 2026
@isaacsas
Copy link
Copy Markdown
Member

isaacsas commented Feb 7, 2026

@TorkelE this is only in the context of building the reaction complex mapping though? In that context, it makes sense to have non-integer coefficients, and at some point we should support them in the network analysis as much as possible, but it is true they probably won't really work throughout right now.

@TorkelE
Copy link
Copy Markdown
Member

TorkelE commented Feb 7, 2026

sorry, got confussed by reading the title and presumed this was targetting bifurcation kit!

@isaacsas
Copy link
Copy Markdown
Member

isaacsas commented Feb 7, 2026

I'm going to reopen this, and will review later today or tomorrow once tests pass.

@isaacsas isaacsas reopened this Feb 7, 2026
@isaacsas
Copy link
Copy Markdown
Member

isaacsas commented Feb 8, 2026

Seems like tests are failing, so something else needs updating.

@calebjubal
Copy link
Copy Markdown
Contributor Author

One of the test is deprecated, I'm trying to create a fix for it.

…at, linkageclasses, and deficiency functions
@calebjubal
Copy link
Copy Markdown
Contributor Author

Hey @isaacsas, the test worked and now there is no issue with this PR!

isaacsas and others added 2 commits February 13, 2026 18:15
…laws

Move incidencematgraph and linkageclasses calls before conservationlaws
in deficiency so that the integer stoichiometry check in
reactioncomplexmap fires before conservationlaws attempts to compute
an integer nullspace on a non-integer stoichiometry matrix.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@isaacsas isaacsas merged commit 7f725e0 into SciML:master Feb 14, 2026
19 checks passed
@isaacsas
Copy link
Copy Markdown
Member

@calebjubal thanks for the PR! All merged now.

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.

Print helpful error if non-integer stoichiometry detected in network analysis functions

3 participants