Potential issues with isequivalent function

[TorkelE]

I don't really know exactly what is the exact intention of this function, but some notes:

• Events are not checked (but e.g. observables are).
• Should complete be checked?
• For subsystem, issetequal is used. Does this actually use isequivalent though? If not, shouldn't isequivalent somehow be called on the subsystems?

"""
    isequivalent(rn1::ReactionSystem, rn2::ReactionSystem; ignorenames = true)

Tests whether the underlying species, parameters and reactions are the same in
the two [`ReactionSystem`](@ref)s. Ignores the names of the systems in testing
equality.

Notes:
- *Does not* currently simplify rates, so a rate of `A^2+2*A+1` would be
    considered different than `(A+1)^2`.
- Does not include `defaults` in determining equality.
"""
function isequivalent(rn1::ReactionSystem, rn2::ReactionSystem; ignorenames = true)
    if !ignorenames
        (nameof(rn1) == nameof(rn2)) || return false
    end

    (get_combinatoric_ratelaws(rn1) == get_combinatoric_ratelaws(rn2)) || return false
    isequal(get_iv(rn1), get_iv(rn2)) || return false
    issetequal(get_sivs(rn1), get_sivs(rn2)) || return false
    issetequal(get_unknowns(rn1), get_unknowns(rn2)) || return false
    issetequal(get_ps(rn1), get_ps(rn2)) || return false
    issetequal(MT.get_observed(rn1), MT.get_observed(rn2)) || return false
    issetequal(get_eqs(rn1), get_eqs(rn2)) || return false

    # subsystems
    (length(get_systems(rn1)) == length(get_systems(rn2))) || return false
    issetequal(get_systems(rn1), get_systems(rn2)) || return false

    true
end

[isaacsas]

isequivalent should be the same as == modulo that it ignores the names of the systems. So anything we think is necessary to decide two systems are the same should be included. (In fact, ==, and hence isequal, just calls isequivalent.)

Probably we need to make a list at some point of everything that needs updating when adding new fields to ReactionSystem. Thinks like flattening, extending, and isequivalent. (There are probably others though.) It is otherwise hard to not miss something...

[TorkelE]

Yeah, I think I entirely forgot about this when I added e.g. the metadata field. It should be noted that MTK also have related problems: SciML/ModelingToolkit.jl#2638 which we might need to consider.

[isaacsas]

A checklist of what functions should be updated in the comments before the ReactionSystem structure def would probably help with this.

[TorkelE]

Maybe something we can add to the dev docs (once we write those)? Just as a natural place to keep that. Alternatively we can just create an internal .txt doc with stuff like that (that we don't necessarily display in the docs, but link to).

[ChrisRackauckas]

Why isn't this isequal?

[isaacsas]

We dispatch == to call isequivalent with the kwarg to require the same system name. Is there a reason to dispatch isequal instead of ==? I thought isequal falls back to == anyways?

[ChrisRackauckas]

Either should be fine.