expanding equations when passed to @reaction_network

[vyudu]

Closes #1022.

[isaacsas]

Can you add some tests in the dsl_options.jl file?

[TorkelE]

It seems like this extracts parameters from DSL expressions (checking on my phone, so have not gone through properly)? Previously we decided quite strongly against doing any such extractions, and generally keep it to a minimum with the exception of fir reactions (we could change this of course, but it would be a rather dramatic shift)

[vyudu]

Ah, I can revert that. I wanted to make

rn = @reaction_network
    @equations D(A) ~ hill(A, v, K, n)
end

work without throwing an error that v is undefined, but I see that it could be complicated to allow this inference generally. What was the main reason for not letting this, out of curiosity?

[TorkelE]

The reason has to do that it can be ambiguous what is a species, a variable, and a parameter, from just looking at the expression. And if there is some misunderstanding, there could be nasty errors.

Given that equations in ReactionSystems is relatively niche, we figured it would be better to have the user manually declare in these situations.

[isaacsas]

@vyudu just as a general comment; it is always good to try to keep a PR focused on one thing. That helps keep it shorter and easier to review. I'd keep this one focused on the external function bug and we can discuss what to do about variable/parameter discovery separately.

[vyudu]

@TorkelE Why isn't it default behavior to add_default_diff? Since they are not allowed to use D for their own differentials it should not create any issues right? This would make the expressions in #1044 work I think.

[TorkelE]

This does a lot of stuff with how equations in the DSL are handled which I don't think we should touch here (e.g. permits f(d(X)) LHS). We did have some reasons to be restrictive with how we infer variables, but I really don't mind expanding this either. However, I think we should let this just handle the function errors, and then we can discuss separately which additional cases we wish to support and not, and handle those in a separate PR.

[isaacsas]

This looks good to me assuming docs build ok. @TorkelE can you confirm it is ok with you before we merge?

[isaacsas]

@vyudu if you put "Closes issue#" like I edited above that will actually close the issue upon merging.

[TorkelE]

I will have a look later today, there is a thing I want to check on my computer.

[TorkelE]

If you ctrl + f search for "### " in the dsl_options.jl test file you will find a section with tests for adding coupled equations to the DSL, I'd move the tests there. Otherwise looks good.
(we have been meaning to have a short dev doc page somewhere with stuff like this, without it is is quite hard to know about stuff likes that!)