SKopecz / PositiveIntegrators.jl

A Julia library of positivity-preserving time integration methods
https://skopecz.github.io/PositiveIntegrators.jl/
MIT License
13 stars 4 forks source link

Performance: Allow evaluating the production and destruction terms in one function #112

Open ranocha opened 1 month ago

ranocha commented 1 month ago

Right now, all algorithms compute first the production terms in one function call and the destruction terms (given the same states) in another function call directly afterward. However, it would be more efficient for some problems to have an API where the production and destruction terms are computed in a single function call.

For backward compatibility, we could wrap the two functions in one struct and call them one after the other. However, that would mean that we have to touch the out-of-place version a bit more (since we use some if statements in them). The in-place versions are fine since conservative and non-conservative systems are already separated.

What do you think about this, @SKopecz?

SKopecz commented 1 month ago

I'd suppose that at such an early stage of this package, we don't need to care too much about backward compatibility. And users of the old version can adapt their code to the new version with little effort.

ranocha commented 1 month ago

Well, the other argument in favor of the wrapping could be that is may look more clear for simple ODE problems. And if we make breaking changes, we can indicate it in the version number and we're fine.

SKopecz commented 1 month ago

Okay, let's try wrapping.

ranocha commented 1 month ago

This is not as straightforward as hoped: We get problems with constructors like

PDSProblem(P, D, u0, tspan, p = NullParameters())
PDSProblem(prod_dest, u0, tspan, p = NullParameters())

since it is not clear how to differentiate the first form with the default argument for p and the second form with the given p - both have the same number of arguments and I don't see any obvious type restrictions we could use for dispatch (since we want to keep everything quite generic). We could consider imposing P::Function, D::Function, but that makes it impossible to use custom callable structs that are no Functions.

Thus, it looks like this has to be a breaking change. I see the following options:

Which version do you prefer?

SKopecz commented 1 month ago

I'd opt for the first case: single function in all cases.