Update Reaction constructor

[TorkelE]

• Permits [] and nothing to be used interchangeably (previously there were some cases which unintentionally would not work).
• Ensures that the Reaction structure is concretely typed.

[TorkelE]

We should know that species are BasicSymbolic{Real}.

Technically, someone could declare a constantspecies parameter, and set it to a non-default type (i.e. BasicSymbolic{Int64}). However, we probably should not permit this?

[isaacsas]

There was a reason this was left untyped I believe but I'm not remembering it. The case you mention is one possibility for someone making a jump model though. If we want to type these maybe they should be Any. Let me think about it a bit.

[isaacsas]

Let me know when you've reverted this and updated the PR as we discussed.

[TorkelE]

The decision is to change to

substrates::Vector{S}
products::Vector{R}

for now

(sorry, was a bit unsure what the final conclusion was if there was one, more than happy to make this switch)

[isaacsas]

No, it was to just leave them untyped as they currently are.

[TorkelE]

No longer needed as we need to check both S and T (as S might now need to be converted from Num to BasicSymbolic{Real}.

[isaacsas]

Actually, now I think this is a bad idea. Someone has made the confusing choice to define get_variables(e::Equation) completely inconsistently with get_variables! in Symbolics, where the latter gets dependent variables (so rhs only).

I think if we want such a function for Reactions we should come up with a different name to avoid confusion (since we already have get_variables! defined on it).

[TorkelE]

Do you mean this definition:

# determine which unknowns a reaction depends on
function MT.get_variables!(deps::Set, rx::Reaction, variables)
    (rx.rate isa Symbolic) && get_variables!(deps, rx.rate, variables)
    for s in rx.substrates
        # parametric stoichiometry means may have a parameter as a substrate
        any(isequal(s), variables) && push!(deps, s)
    end
    deps
end

and this PR #855?

To do get_variables(x) where you return a vector of all the symbolic variables in x (whatever it is) is (to me) the established way to use the function, so get_variables(rx::Reaction) makes sense. Practically, I think we should have some function that does this, as it seems like something genuinely useful.

[TorkelE]

helper function to ensure something have the best type