Evaluate parameters with intended structural impact in Mechanics

[henrikt-ma]

The PR is limited to Modelica.Mechanics. It changes some components declared parameter to be constant instead in order to make clear to both users and tools that they can and are expected to be evaluated during translation.

This kind of change is an important part of making a library portable between tools, since differences in which parameters that are turned into constants during translation will lead to different equation systems in different tools.

By trying to do this systematically instead of case by case when needed, we get a consistent and predictable user experience when dealing with these variables. Also, when all existing, say, enforceStates have the same variability, there is little risk of getting the wrong variability when creating a new component with an enforceStates variable.

That said, the variables affected by this PR have been selected based on improved compatibility with Wolfram SystemModeler.

[HansOlsson]

This kind of change is an important part of making a library portable between tools, since differences in which parameters that are turned into constants during translation will lead to different equation systems in different tools.

I'm unsure what this intends to say: I don't see that having the same equation systems is a goal for portability. Obviously we want different tools to generate the same solution, but that doesn't imply that we need the same equation systems.

However, many of them influence state-selection and in those cases I agree that at least having the same number of states is a really good idea.

I also note that this PR creates two kinds of issues:

• It generates a ripple effect, such that all libraries propagating these existing parameters will be impacted. Existing conversions cannot handle that.
• The new constants will not appear as parameters in some tools (e.g. Dymola); obviously that is trivial to change - but that will imply that constants in other models will be shown which wasn't the intention.

If we compare with the introduction of "const functions" in C++ we notice that this ripple effect can be a major concern, especially since we don't have any standard way of circumventing this.

[henrikt-ma]

This kind of change is an important part of making a library portable between tools, since differences in which parameters that are turned into constants during translation will lead to different equation systems in different tools.

I'm unsure what this intends to say: I don't see that having the same equation systems is a goal for portability. Obviously we want different tools to generate the same solution, but that doesn't imply that we need the same equation systems.

While the implication same solution ⇒ same equation systems isn't true at all (we can easily process equation systems to take different forms, use other variables to express them, etc), it should also be clear that solving different equation systems will lead to different solutions in general. This is of particular importance for non-linear systems at initialization and events, since how to best provide guess values (start attributes) will depend on which equation system that is being solved.

With all the trickiness involved in mixed systems, it should also be clear that getting fundamentally different mixed systems in different tools is a very likely cause for finding different solutions.

With this in mind, I think that doing what can be done to allow different tools to end up with equivalent equation systems should be a clear prerequisite for portability of simulation results.

However, many of them influence state-selection and in those cases I agree that at least having the same number of states is a really good idea.

Yes, then the simulations at least belong to the same universe.

[henrikt-ma]

I also note that this PR creates two kinds of issues:

• It generates a ripple effect, such that all libraries propagating these existing parameters will be impacted. Existing conversions cannot handle that.

We are talking about enforceStates, useQuaternions, and exact. I feel fairly confident that the damage will be very limited. Tools will be able to give very clear diagnostics for the incompatibilities, and the propagated changes will end up improving the dependent libraries in the same way as this PR improves MSL.

• The new constants will not appear as parameters in some tools (e.g. Dymola); obviously that is trivial to change - but that will imply that constants in other models will be shown which wasn't the intention.

Yes, at least it seems right that they shouldn't appear as parameters. For now, they are given a Dialog annotation for compatibility with common GUI logic for visibility of constants.

[MartinOtter]

The current solution is:

parameter Boolean enforceStates=false annotation(Evaluate=true, Dialog(tab="Advanced"))

• Due to parameter it is indicated that enforceStates should be in the parameter menu and should not change during simulation.
• Due to Evaluate=true it is indicated that this parameter should be evaluated during compilation and therefore cannot be changed any more after code generation.
• Due to tab="Advanced", the parameter is shown in tab "Advanced".

All this looks fine to me.

Your proposed solution

constant Boolean enforceStates=false annotation(Dialog(tab="Advanced"))

would be an alternative to the current pattern with parameter ... Evalute=true, and looks in principal also good. Hereby, one would have the implicit assumption that the constant enforceStates would appear in the parameter menu, provided a Dialog annotation is present.

Your argument is:

in order to make clear to both users and tools that they can and are expected to be evaluated during translation.

But this is also clear with the current solution, so it seems to be no argument at all to change the current solution. Note, "Evaluate=true" is only a suggestion and a tool can ignore it, but if a tool is ignoring it, it should cope with the variable structure (e.g. automatically re-compile the model when the parameter is changed).

I checked with Dymola and in Dymola constants with a Dialog annotation do not appear in a parameter menu. I did not check what the specification says here. The effect is that changing parameter to constant would result in loss of functionality of at least one current Modelica tool (and I expect also other Modelica tools might have the same effect, because they have been tuned to work with the current pattern).

This means, it is not possible to accept this pull request now. Instead a longer process is needed, by which Modelica specification and tools agree. But as I clearly wrote: The current solution and pattern looks good, so why changing it?

[HansOlsson]

It took me some time to figure out the solution - and then @MartinOtter had already described it.

However, I also noticed that the parameters in question differ slightly, which is part of the problem as these different categories are not obvious.

For useQuaternions and enforceStates it seems quite "challenging" for a tool to not evaluate them.

For exact it seems entirely possible to not evaluate, but Dymola has long had the heuristic that the conditions of if-then-else equations are evaluated, if possible.

I believe that evaluate exact=false is the fast variant, and evaluated exact=true and unevaluated exact gives the same performance. Following Martin's statements we should ensure that exact-parameters have Evaluate=true.

[henrikt-ma]

There are reasons for using constant instead of Evaluate=true:

• The Evaluate=true may be propagated to other parameters in long chains, effectively forcing other things declared parameter without Evaluate=true to behave as if they were declared constant. Declaring them constant makes it clear to everybody what they really are.
• The Evaluate=true is hiding in an annotation, but annotations are not supposed to hide things with such fundamental impact on the model equations. In SystemModeler (and probably also some other tools), a user must choose to expand annotations in order to even see that Evaluate=true has been set.

[henrikt-ma]

Having a logic that relies on what is challenging and what is not is also a problem for tools, as one may not know from the beginning what will be considered challenging enough to evaluate parameters in the end. If all things that are allowed to be evaluated during translation were declared constant, a tool can take advantage of this all the way from flattening to the later analysis of equations and code generation.

This is why I want to make it explicit that useQuaternions and enforceStates are constants.

[henrikt-ma]

I checked with Dymola and in Dymola constants with a Dialog annotation do not appear in a parameter menu. I did not check what the specification says here. The effect is that changing parameter to constant would result in loss of functionality of at least one current Modelica tool (and I expect also other Modelica tools might have the same effect, because they have been tuned to work with the current pattern).

OK, I wasn't expecting any tool to ignore a Dialog annotation this way. Obviously, it works with SystemModeler, but it would be interesting to hear about other tools as well. It's a pity we haven't settled modelica/ModelicaSpecification#2211 yet.

[beutlich]

I checked with SimulationX 4.0.2 and in SimulationX constants with a Dialog annotation do not appear in a properties window. Of course, this can be adapted if there is agreement to change this behaviour.

[henrikt-ma]

I checked with SimulationX 4.0.2 and in SimulationX constants with a Dialog annotation do not appear in a properties window. Of course, this can be adapted if there is agreement to change this behaviour.

Thanks for checking this. I've now opened modelica/ModelicaSpecification#2413 for this specific question.

[claassistantio]

All committers have signed the CLA.

[MartinOtter]

Just to make this clear: The Modelica Language group should clarify the recommended implementation of this feature, then the tools should support it and then something can be changed in MSL. I still think that the current pattern is fine.

[henrikt-ma]

The very similar #3119 was just resolved by setting Evaluate=true on things that are expected to have structural impact. I suggest that this PR is resolved in the same way. In the end, having something that works reliably across tools seems more important right now than debating how it should be done.

If I see support for doing this with Evaluate=true, I'll update this PR right away with this method instead of using constant.

[tobolar]

@MartinOtter When accepting the suggestion Evaluate=true from #3113 (comment), this would actually turn to a consistency issue only. This is because Evaluate=true is already used for enforceStates and useQuaternions in e.g. Mechanics.MultiBody.Parts.Body. But for some elements this is, in contrast, not the case.
I have no concerns.

[HansOlsson]

@MartinOtter When accepting the suggestion Evaluate=true from #3113 (comment), this would actually turn to a consistency issue only. This is because Evaluate=true is already used for enforceStates and useQuaternions in e.g. Mechanics.MultiBody.Parts.Body. But for some elements this is, in contrast, not the case.
I have no concerns.

I agree that with that proposed change it would be ok. These parameters are such that they basically have to be structural parameters, in contrast to e.g., axis n for Revolute joint which may have Evaluate=true to optionally evaluate it for performance.

[beutlich]

@henrikt-ma Please go ahead with the proposed changes. Thanks.

[henrikt-ma]

Done. The old commits with constant are gone, completely replaced by new commits with Evaluate=true.

[beutlich]

Done. The old commits with constant are gone, completely replaced by new commits with Evaluate=true.

There are merge conflicts. Did you rebase on current HEAD?

[henrikt-ma]

Sorry, failed to do that. Will be fixed right away.

[henrikt-ma]

Fixed.

[HansOlsson]

Looks good now.