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

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

@parametersp(t)@variablesx(t)cb= [some_condition]=> [x~Pre(x)+ p]

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

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

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

@parameters p# not `(t)`

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

For non-time-dependent parameters,Pre isn't necessary. I'm not sure if having them indiscrete_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.

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 thatcb = [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.

Or maybe you meant

@parameters p# Not `(t)`@variablesx(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.

Weird. I investigated, and turns out thatmtkcompile doesn't handle the system right. Ifp isn't provided todiscrete_parameters, the system it creates is

julia> pasysModel affectsys:Equations (1):1 standard: seeequations(affectsys)Unknowns (1): seeunknowns(affectsys)x(t)Parameters (2): seeparameters(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 becomesx(t) ~ Pre(x(t)) + pₜ₋₁(t) which is a bug.

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

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.

Right, but doesn't that align with this PR? I agree thatconditon => [p1 + p2 ~ 1] should yield an error, and this should correctly deduce that bothp1 andp2 (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 eitherp1 orp2 to stay fixed, you can designate this using the current notation, e.g.Pre(p1) + p2 ~ 1 to note that thep1 value should be the pre-eventp1 value (not the post-event one, as the initial equations suggest).
(again, correct me if i am wrong)

@AayushSabharwal thoughts?

Me and Aayush had a meeting discussing this, and I think we managed to iron things out. There are definitely some additional syntactic sugar that could be done, but it will require some more thoughts than here. I will update the PR to only fix the test issue, and then we can think if there are any long-term improvements that can be made later on.