#5301 closed defect (fixed)
The NF does not evaluate final parameters computed by functions of parameters with Evaluate = true and literal constants
Reported by: | Francesco Casella | Owned by: | Per Östlund |
---|---|---|---|
Priority: | high | Milestone: | 2.0.0 |
Component: | New Instantiation | Version: | |
Keywords: | Cc: | Karim Adbdelhak |
Description
Some MultiBody models still do not produce the correct results (or possibly fail) when the NF is employed to flatten the model. For example, the results of Modelica.Mechanics.MultiBody.Examples.Elementary.HeatLosses do not match with the reference, in particular the z-axis component of the angular velocity w_a[3]
is initialized to zero instead of 2 deg/s, and thus remains constant instead of oscillating.
Some analysis reveals that the start attribute of w_a[3]
is computed using R_start.T
, which is incorrectly computed to zeros[3,3]
instead of diagonal(1, 1, 1)
.
The NF-flattened model sets the fixed
attribute of R_start.T
to false, and adds the initial equation
body1.R_start = Modelica.Mechanics.MultiBody.Frames.axesRotations({1, 2, 3}, body1.angles_start, {0.0, 0.0, 0.0});
However, the Body
model contains the following code
final parameter Frames.Orientation R_start= Modelica.Mechanics.MultiBody.Frames.axesRotations( sequence_start, angles_start, zeros(3)) "Orientation object from world frame to frame_a at initial time";
Since R_start
is final
and both sequence_start
and angles_start
have Evaluate = true
, R_start
should be marked as structural and evaluated by the front-end.
The issue is similar to #5084; in that case the parameter had Evaluate = true
, while here it is final
, but the end result should be the same.
I suspect this issue is probably the cause of other simulation failures or errors with the MultiBody library, which are due to the lack of constant evaluation of such rotation matrices, preventing the proper symbolic manipulation of the multibody system equations. I would suggest to fix this asap to improve the coverage and avoid getting wrong results from models that simulate successfully.
Change History (6)
follow-up: 2 comment:1 by , 6 years ago
follow-up: 3 comment:2 by , 6 years ago
Replying to perost:
The issue seems to be that
angles_start
actually does not have anEvaluate
-annotation
You're right, I was mistaken, Evaluate = true
is actually on angle_fixed
.
Adding
Evaluate = true
to the definition ofangles_start
causes it and the binding ofR_start
to be evaluated as expected.
Hmm, another place where we should patch the MSL. I'd rather avoid that.
Right now we only evaluate bindings of complex components if the component or the binding is structural. One solution would be to also try to evaluate bindings of non-structural complex parameters, and only move the binding to an equation if the evaluation fails. We could also restrict it to only final parameters if that's more appropriate.
I would not evaluate all non-structural complex parameters: a reasonable use-case is to use records to store component data, and then change them on the fly when re-simulating.
I think that evaluating final parameters (ad moving them to initial equations if the evaluation fails) would be an excellent compromise.
In principle, the fact that a parameter has a final modifier doesn't necessarily mean it should be constant evaluated, because the bound expression may not be structural (as in this case). But I guess that's what most people writing libraries expect, and that's also WWDD. So I would go for that.
comment:3 by , 6 years ago
Replying to casella:
I think that evaluating final parameters (ad moving them to initial equations if the evaluation fails) would be an excellent compromise.
Ok, I've implemented that in e11c6c60 (I had already implemented it, was just waiting for you to comment :)). We'll see how it affects the libraries.
follow-up: 5 comment:4 by , 6 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
It seems like evaluating final complex parameters was mostly positive, see results. It fixed simulation of a couple MultiBody and Spice3 models, but also broke simulation of one Spice3 model.
It also didn't fix the HeatLosses model mentioned in the description but instead caused it to simulate much slower than expected, so it seems there's still some issue with that model. But I'll close this ticket since this particular issue is fixed, and you can open a new ticket if you figure out what the remaining issue is.
comment:5 by , 6 years ago
Replying to perost:
It seems like evaluating final complex parameters was mostly positive, see results. It fixed simulation of a couple MultiBody and Spice3 models,
That's quite an understatement. I see 8 more MultiBody models in ModelicaTest now passing verification (which likely means their previous results were plain wrong), and also 4 more Spice3 models going through verification. Overall +1.3% of global coverage. Not bad for one commit :)
but also broke simulation of one Spice3 model.
I'll investigate that later.
It also didn't fix the HeatLosses model mentioned in the description but instead caused it to simulate much slower than expected
I looked at the trajectories reported by the verification tool, and they are oddily irregular. I guess there is still some structural issue that leads to a poorly conditioned state choice. I'll investigate that as well.
comment:6 by , 6 years ago
Cc: | added |
---|
I need to reconsider my position on this topic. I ran the HeatLosses
model on Dymola, and I noticed that I can actually change body1.angles_start
at runtime, and that doing so indeed influences body1.R_start.T
accordingly.
So, my statement in comment:2 was indeed wrong: Dymola does not evaluate final parameters if they depend on non-final, non-structural parameters without an Evaluate annotation, as we currently do with both the old and new frontend, after commit e1c6c60.
I guess it is ok for the time being if we do so in this specific case, in which records contining arrays are involved, and we don't want those bindings to end up in the initial equation section. However, I guess we should eventually avoid evaluating final parameters unless they *exclusively* depend on structural or final parameters, and the back-end should handle that case correctly.
I just opened #5390 on this topic so we can track it.
The issue seems to be that
angles_start
actually does not have anEvaluate
-annotation, it's just a normal non-final parameter. AddingEvaluate = true
to the definition ofangles_start
causes it and the binding ofR_start
to be evaluated as expected.Right now we only evaluate bindings of complex components if the component or the binding is structural. One solution would be to also try to evaluate bindings of non-structural complex parameters, and only move the binding to an equation if the evaluation fails. We could also restrict it to only final parameters if that's more appropriate.