The NF does not evaluate a function with constant arguments

[casella]

The model Modelica.Mechanics.MultiBody.Examples.Elementary.SpringDamperSystem simulates with the NF, but produces wrong results, which do not pass verification. It works fine with the NF.

The -d=optdaedump output shows the following equations:

body2.g_0[3] = SpringDamperSystem.world.gravityAcceleration({bar2.frame_b.r_0[1] + body2.r_CM[1], body2.r_CM[2] - p2.s, body2.r_CM[3]}, Modelica.Mechanics.MultiBody.Types.GravityTypes.UniformGravity, {0.0, -Modelica.Mechanics.MultiBody.World.g, 0.0}, Modelica.Mechanics.MultiBody.World.mue)[3]   [dynamic |0|0|0|0|] 
body2.g_0[2] = SpringDamperSystem.world.gravityAcceleration({bar2.frame_b.r_0[1] + body2.r_CM[1], body2.r_CM[2] - p2.s, body2.r_CM[3]}, Modelica.Mechanics.MultiBody.Types.GravityTypes.UniformGravity, {0.0, -Modelica.Mechanics.MultiBody.World.g, 0.0}, Modelica.Mechanics.MultiBody.World.mue)[2]   [dynamic |0|0|0|0|] 
body2.g_0[1] = SpringDamperSystem.world.gravityAcceleration({bar2.frame_b.r_0[1] + body2.r_CM[1], body2.r_CM[2] - p2.s, body2.r_CM[3]}, Modelica.Mechanics.MultiBody.Types.GravityTypes.UniformGravity, {0.0, -Modelica.Mechanics.MultiBody.World.g, 0.0}, Modelica.Mechanics.MultiBody.World.mue)[1]   [dynamic |0|0|0|0|] 
body1.g_0[1] = SpringDamperSystem.world.gravityAcceleration(body1.frame_a.r_0 + {spring1.frame_b.R.T[1,1] * body1.r_CM[1] + spring1.frame_b.R.T[2,1] * body1.r_CM[2] + spring1.frame_b.R.T[3,1] * body1.r_CM[3], spring1.frame_b.R.T[1,2] * body1.r_CM[1] + spring1.frame_b.R.T[2,2] * body1.r_CM[2] + spring1.frame_b.R.T[3,2] * body1.r_CM[3], spring1.frame_b.R.T[1,3] * body1.r_CM[1] + spring1.frame_b.R.T[2,3] * body1.r_CM[2] + spring1.frame_b.R.T[3,3] * body1.r_CM[3]}, Modelica.Mechanics.MultiBody.Types.GravityTypes.UniformGravity, {0.0, -Modelica.Mechanics.MultiBody.World.g, 0.0}, Modelica.Mechanics.MultiBody.World.mue)[1]   [dynamic |0|0|0|0|] 
body1.g_0[2] = SpringDamperSystem.world.gravityAcceleration(body1.frame_a.r_0 + {spring1.frame_b.R.T[1,1] * body1.r_CM[1] + spring1.frame_b.R.T[2,1] * body1.r_CM[2] + spring1.frame_b.R.T[3,1] * body1.r_CM[3], spring1.frame_b.R.T[1,2] * body1.r_CM[1] + spring1.frame_b.R.T[2,2] * body1.r_CM[2] + spring1.frame_b.R.T[3,2] * body1.r_CM[3], spring1.frame_b.R.T[1,3] * body1.r_CM[1] + spring1.frame_b.R.T[2,3] * body1.r_CM[2] + spring1.frame_b.R.T[3,3] * body1.r_CM[3]}, Modelica.Mechanics.MultiBody.Types.GravityTypes.UniformGravity, {0.0, -Modelica.Mechanics.MultiBody.World.g, 0.0}, Modelica.Mechanics.MultiBody.World.mue)[2]   [dynamic |0|0|0|0|] 
body1.g_0[3] = SpringDamperSystem.world.gravityAcceleration(body1.frame_a.r_0 + {spring1.frame_b.R.T[1,1] * body1.r_CM[1] + spring1.frame_b.R.T[2,1] * body1.r_CM[2] + spring1.frame_b.R.T[3,1] * body1.r_CM[3], spring1.frame_b.R.T[1,2] * body1.r_CM[1] + spring1.frame_b.R.T[2,2] * body1.r_CM[2] + spring1.frame_b.R.T[3,2] * body1.r_CM[3], spring1.frame_b.R.T[1,3] * body1.r_CM[1] + spring1.frame_b.R.T[2,3] * body1.r_CM[2] + spring1.frame_b.R.T[3,3] * body1.r_CM[3]}, Modelica.Mechanics.MultiBody.Types.GravityTypes.UniformGravity, {0.0, -Modelica.Mechanics.MultiBody.World.g, 0.0}, Modelica.Mechanics.MultiBody.World.mue)[3]   [dynamic |0|0|0|0|] 

while the same output with the OF doesn't show any call to gravityAcceleration. In fact, the first and third components of g_0 are identically zero, while the second is constant and equal to -9.81. These values should be constant evaluated by the NF, so that the BE can make all the corresponding symbolic simplifications.

I strongly suspect that the lack of these constant evaluations causes the numerical issues reported in #5328, and possibly other verification failures in the MultiBody library, so I would suggest to fix this with high priority.

[perost]

Fixed in ff2558b3, but the model still does not seem to simulate correctly.

The issue was for once that the function was actually missing the Inline annotation, or rather that the new instantiation only considered the annotation on the function definition of gravityAcceleration (which doesn't have one). It inherits from a function that does have an Inline = true annotation though, but the new frontend didn't use it.

Now we copy the old frontend (literally, I copied the comment merging function from it) and consider annotations on the inherited classes too. The new frontend only does this for short class definitions though, i.e. function f = other_f(...), since merging annotations from extends clauses is a bit trickier and I'm not sure if it's a good idea for e.g. inlining anyway.

So now the gravityAcceleration function is inlined as it should be, but while the simulation results changed a bit they still don't seem to be correct.

[casella]

When I prepared the suite of experiments in the ScalableTestSuite library, I noticed that when I extended from an existing test case, I had to add the experiment annotation again, even though it was already present in the base class. I thought this was a feature, not a bug, but in fact I have now read the specification, Section 7.1, which says

The elements of the flattened base class become elements of the flattened enclosing class, and are added at the place of the extends-clause: specifically components and classes, the equation sections, algorithm sections, optional external clause, and the contents of the annotation at the end of the class, but excluding import-clauses.

Does your last commit imply that now if I extend from a class with the experiment annotation, I get it automatically in the derived class?

[casella]

Regarding the inlining, my idea was that if you have a (pure!) function call whose arguments are all constants or structural parameters, you should evaluate it anyway in the frontend, regardless what the Inline annotation says, and replace it with its constant literal output. Of course if the function is inlined, the backend should eventually do this anyway, but why not doing this in the frontend outright? What do you think?

Inlining would be useful in those cases where the gravity acceleration is actually a function of the position, but that's not the case in most applications, that take place on the ground and are therefore subject to a constant downards acceleration of 9.81 m/s². Only space applications actually require inlining - I've played with them in the past, but it's rather the exception than the norm.

[casella]

In any case, I'll check the results and report.

[perost]

Replying to casella:

Does your last commit imply that now if I extend from a class with the experiment annotation, I get it automatically in the derived class?

Only for function extending via a short class definition, i.e. function f = other_f(...) causes f to inherit annotations from other_f (which itself might have inherited annotations). I'd missed that annotations are supposed to be inherited via extends, but the old frontend does not seem to do that anyway, only for functions.

The specification is also rather vague in this regard, since it doesn't specify how inherited annotations should be handled (i.e. should they be merged, and if so how?). Anyway, I think my commit covers the most important case, but might need some revising.

Replying to casella:

Regarding the inlining, my idea was that if you have a (pure!) function call whose arguments are all constants or structural parameters, you should evaluate it anyway in the frontend, regardless what the Inline annotation says, and replace it with its constant literal output. Of course if the function is inlined, the backend should eventually do this anyway, but why not doing this in the frontend outright? What do you think?

The new frontend does that already. The constant evaluation module evaluates all constants and structural parameters, and then the simplification module tries to evaluate all functions with literal arguments.

But that's not applicable for these gravityAcceleration calls, since the arguments aren't constant. body1.frame_a.r_0 is for example just a normal variable, not even a parameter.

[perost]

I played around with the model a bit more, and figured out that it simulates correctly if the order of the equations is reversed in the flat model.

I found this out by looking at the Modelica.Thermal.FluidHeatFlow.Examples.SimpleCooling model, which is quite small but still fails to simulate with the new frontend even though it's pretty much identical when flattened with the old frontend except for the order of the equations. The Modelica.Thermal.FluidHeatFlow.Examples.IndirectCooling model fails for the same reason when using the old frontend though, but works with the new.

From my limited understanding of the backend it seems like the issue in the FluidHeatFlow models is that it chooses different aliases depending on the order of the equations. For example when using the new frontend to simulate SimpleCooling, it fails to solve a nonlinear system involving ambient2.flowPort.H_flow.

-d=optdaedump shows that with the new frontend you get the alias pipe.flowPort_b.H_flow = -ambient2.flowPort.H_flow, while with the old frontend you get ambient2.flowPort.H_flow = -pipe.flowPort_b.H_flow instead. So it seems like the order of the equations influences which variables are chosen as aliases in the backend. It probably shouldn't matter which variables are chosen as aliases, but evidently it does.

So this seems to me like something that should be fixed in the backend, seeing as there are some models that fail with the new frontend and some that fail with the old. I'll close this ticket and open a new for that issue.

[casella]

Replying to perost:

Replying to casella:

Does your last commit imply that now if I extend from a class with the experiment annotation, I get it automatically in the derived class?

Only for function extending via a short class definition, i.e. function f = other_f(...) causes f to inherit annotations from other_f (which itself might have inherited annotations). I'd missed that annotations are supposed to be inherited via extends, but the old frontend does not seem to do that anyway, only for functions.

OK.

The specification is also rather vague in this regard, since it doesn't specify how inherited annotations should be handled (i.e. should they be merged, and if so how?).

I started a discussion on the Modelica issue tracker, see ​#2314

[casella]

Regarding the inlining, my idea was that if you have a (pure!) function call whose arguments are all constants or structural parameters, you should evaluate it anyway in the frontend, regardless what the Inline annotation says, and replace it with its constant literal output. Of course if the function is inlined, the backend should eventually do this anyway, but why not doing this in the frontend outright? What do you think?

The new frontend does that already. The constant evaluation module evaluates all constants and structural parameters, and then the simplification module tries to evaluate all functions with literal arguments.

But that's not applicable for these gravityAcceleration calls, since the arguments aren't constant. body1.frame_a.r_0 is for example just a normal variable, not even a parameter.

That's true. As it turns out, gravityAcceleration does not actually depend on that input, but finding that out is something best left to the backend, after inlining.

In fact, that's what was happening with the OF. -d=dumpdaelow still shows those function calls. They are missing from the -d=optdaedump output, because the first thing it shows is the equations after inlining, I always forget about that.

[casella]

Replying to perost:

I played around with the model a bit more, and figured out that it simulates correctly if the order of the equations is reversed in the flat model.

That's bad indeed :)

So this seems to me like something that should be fixed in the backend, seeing as there are some models that fail with the new frontend and some that fail with the old. I'll close this ticket and open a new for that issue.

Agreed. I'll follow #5335 now.