Automatic inference of discrete parameters in events

[TorkelE]

When creating a symbolic event through SymbolicContinuousCallback or SymbolicDiscreteCallback it is possible to infer which parameters are updated by the event (everything within Pre(...) statements in the affect are protected, but everything outside can be updated). This PR adds a small routine to do this for the user if the discrete_parameters argument is not provided. This should prevent the possibility of silent errors if they forgets this argument. Furthermore, it enables programmatic use of events without explicitly importing SymbolicContinuousCallback/SymbolicDiscreteCallback (as one can how just use the normal map for in the input, instead of using these functions).

I have tried, and it is possible to remove all discrete_parameters statements from all events in the test files, and everything passes. Right now I have not done so, as I wanted to provide minimal changes (isntead I added a bunch of additional new tests as well).

[TorkelE]

There is an error in this test. The discrete_parameters argument was forgotten, so the event is not actually triggered. Unless the intention of the test is to check that everything works as (un)expected when the input is omitted (but the naming and test do not suggest that), this should be changed. With this PR, the event is now triggered, and the correct value 0.75 is achieved.

[TorkelE]

There are test errors, however, they are the same as in: #3989

[AayushSabharwal]

I'm not so sure of this. If I understand correctly, this takes all parameters in affect equations that don't occur inside a Pre and considers them discretes? Isn't this breaking, since prior to this change the callback

@parameters p(t)
@variables x(t)

cb = [some_condition] => [x ~ Pre(x) + p]

is well-defined, but after the change p would be considered discrete thus making the affect underdetermined?

[TorkelE]

Wouldn't cb = [some_condition] => [x ~ Pre(x) + p] be overdetermined, as we have two unknowns (x and p), and one known (Pre(x))?

[TorkelE]

For non time-dependent parameters I could make it optional to use Pre though, i.e. if you have

@parameters p # not `(t)` 

but I was hesitant as I figured it might make more sense to just enforce Pre on everything that should not be solved for.

[AayushSabharwal]

For non-time-dependent parameters, Pre isn't necessary. I'm not sure if having them in discrete_parameters makes a difference, but I don't think it does.

Wouldn't cb = [some_condition] => [x ~ Pre(x) + p] be overdetermined, as we have two unknowns (x and p), and one known (Pre(x))?

We have 2 unknowns and one equation. Pre(x) is a parameter here. It's underdetermined.

[TorkelE]

Sounds good, I can discount non-time dependent parameters if that the case.

We have 2 unknowns and one equation. Pre(x) is a parameter here. It's underdetermined.

Sorry, yes, meant underdetermined. But my point is that cb = [some_condition] => [x ~ Pre(x) + p] is already underdetermined without this change (it yields and error if you try to simualte it). It being underdetermined is not something this change affects.

[TorkelE]

Or maybe you meant

@parameters p # Not `(t)`
@variables x(t)

cb = [some_condition] => [x ~ Pre(x) + p]

that would make sense, I will update the PR to discount non-time-dependent parameters, which should harmonize this case with current behaviours.

[AayushSabharwal]

Weird. I investigated, and turns out that mtkcompile doesn't handle the system right. If p isn't provided to discrete_parameters, the system it creates is

julia> pasys
Model affectsys:
Equations (1):
  1 standard: see equations(affectsys)
Unknowns (1): see unknowns(affectsys)
  x(t)
Parameters (2): see parameters(affectsys)
  Pre(x(t))
  p(t)

julia> equations(pasys)
1-element Vector{Equation}:
 x(t) ~ p(t) + Pre(x(t))

However, after simplification the equation becomes x(t) ~ Pre(x(t)) + pₜ₋₁(t) which is a bug.

[ChrisRackauckas]

It's not generally correct for the user to not be selecting it? What do you do about the case of p1 + p2 ~ 1? How do you choose which parameter to change? That is underdetermined.

[ChrisRackauckas]

It being underdetermined is not something this change affects.

It causes malformed systems to be automatically created rather than requiring the user to give a correct system. That seems like a recipe for trouble.

[TorkelE]

Right, but doesn't that align with this PR? I agree that conditon => [p1 + p2 ~ 1] should yield an error, and this should correctly deduce that both p1 and p2 (if both are declared as time varying) are free in the update equation. This wield yield an error due to an underdetermined system. If you want either p1 or p2 to stay fixed, you can designate this using the current notation, e.g. Pre(p1) + p2 ~ 1 to note that the p1 value should be the pre-event p1 value (not the post-event one, as the initial equations suggest).
(again, correct me if i am wrong)