network_analysis cleanup: Updating ratematrix to be more similar to new matrix API functions
#1172
Conversation
…n to be more similar
…twork-analysis-touchup
| @@ -1069,50 +1038,74 @@ function iscomplexbalanced(rs::ReactionSystem, parametermap) | |||
| end | |||
|
|
|||
| """ | |||
| ratematrix(rs::ReactionSystem, parametermap) | |||
| adjacencymat(rs::ReactionSystem, pmap = Dict(); sparse = false) | |||
There was a problem hiding this comment.
Does this require ismassaction, and if so is that being checked anywhere?
There was a problem hiding this comment.
More generally, it would be good if any of these methods that only make sense with fixed rate constants (in the Catalyst sense) return an informative error if they get an invalid system.
There was a problem hiding this comment.
Have added them. I think fluxmat and adjacencymat should work with reactions with fixed rate constants that aren't necessarily mass action (which is the case for generalized mass action systems e.g., the CatalystNetworkAnalysis stuff does some stuff with that) and the massactionvector should work with just mass action reactions
There was a problem hiding this comment.
Ok, if they should work more generally then the checks aren't needed. I just want to make sure we don't have people using them for systems they aren't intended / expected to work with.
LMK how you want to proceed (I'm fine keeping the checks or dropping them if you feel they aren't actually needed here).
There was a problem hiding this comment.
I think the check currently is good, we do still wanna check that stuff like k*A, A --> B isn't allowed in the adjacencymat/fluxmat since you could no longer use it in the ODE system decomposition if it is time-dependent. But in theory something like k, A --> B that doesn't have mass action kinetics (e.g. the rate law is k*A^2 or something) is fine and constructing the adjacencymat/fluxmat for such a reaction is useful in some cases.
|
OK, then feel free to merge if you are happy with this. |
ratematrixtoadjacencymat, change its implementation to be more similar to the new API matrix functions in Add API functions for the other kinds of matrixes that a CRN ODE system can be factored into #1134iscomplexbalanced