Incomplete implementation of `Piecewise`

[FFroehlich]

Implementing Piecewise through sp.Piecewise is not a good idea if the arguments change dynamically, as this leads to poor integration accurracy (see https://github.com/AMICI-dev/AMICI/blob/master/documentation/implementation_discontinuities.rst). Wondering whether the feature shouldn't be disabled for the time being.

[paulstapor]

Hm... Same problem for having just the time in the piecewise function? Agreeing here that this should be passed to AMICI as a discontinuity, which isn't the case atm...
I totally agree that having parameter-dependent expression in piecewise functions can lead to incorrect sensitivities...

[dweindl]

Wondering whether the feature shouldn't be disabled for the time being.

Personally, I am not using Piecewise or planning to use it in the near future. Okay for me to disable or to include a warning regarding accuracy issues. Not considering investing time on an improved implementation in the near future, as there are more urgent issues for me.

[paulstapor]

Would be good to fix this one together with #1328.

What's to be done here:

• Add an event counter (ne) and make the Python interface of AMICI create a vector of root functions.
• Track piecewise functions during symbolic processing (check for time or parameter or state dependence), add their arguments to the vector of root functions and increment ne for each of those.
• Replace every occurrence of the piecewise function by amici internal heavisides (denoted as h[<index_of_the_argument_in_the_root_vector>] in the cpp code): ideally when processing the symbolic expressions.
• Throw an error for state of parameter dependent arguments for the moment.

[lcontento]

Bit late for the discussion, but in the spline PR Paul and I are using piecewise functions as a fallback in the case someone loads the SBML model with a different software. Having piecewise functions is not really important on the AMICI side at runtime, but we use them for testing that our spline implementation is behaving as expected. So it would be great if the functionality is not removed completely but could be enabled for example by some keyword argument.

By the way, in our case the piecewise functions we use are continuous up to the first derivative at the discontinuity points. I think in this case there should be no problem with integration accuracy, right?

[FFroehlich]

Bit late for the discussion, but in the spline PR Paul and I are using piecewise functions as a fallback in the case someone loads the SBML model with a different software. Having piecewise functions is not really important on the AMICI side at runtime, but we use them for testing that our spline implementation is behaving as expected. So it would be great if the functionality is not removed completely but could be enabled for example by some keyword argument.

Not sure I understand. AMICI would still use piecewise but this will change cpp generation. This should still be different from the spline code.

By the way, in our case the piecewise functions we use are continuous up to the first derivative at the discontinuity points. I think in this case there should be no problem with integration accuracy, right?

Probably yes.

However, I am not sure wether the formula for the error corrector that determines the adaptive step size is still correct if there are higher order discontinuities, but that will be difficult to address generally.

[paulstapor]

Maybe to note: This issue will be fixed before the spline implementation is finished. I consider this one here high prio, will work on it asap.