search

modelica / ModelicaSpecification

Specification of the Modelica Language
https://specification.modelica.org
Creative Commons Attribution Share Alike 4.0 International
98 stars 42 forks source link

Problematic default for Dialog.showStartAttribute #2823

Open henrikt-ma opened 3 years ago

henrikt-ma commented 3 years ago

Working on #2819, and considering that we actually made the effort to get rid of the default for Dialog.group, I'd like to see a similar improvement for Dialog.showStartAttribute. While a default of false may make sense for parameters (just like the previous default for group), it is a pretty bad default for a non-parameter with Dialog annotation.

However, rather than removing the default value, or making the default depend on the declared component variability, I suggest that showStartAttribute is redefined to only apply to parameters.

gkurzbach commented 3 years ago

suggest that showStartAttribute is redefined to only apply to parameters.

For me, showStartAttribute=true only makes sense for variables (those that become ODE states or iteration variables) and parameters with fixed=false that become iteration variables of the initial system. But this, the compiler does not know in advance.

henrikt-ma commented 3 years ago

suggest that showStartAttribute is redefined to only apply to parameters.

For me, showStartAttribute=true only makes sense for variables (those that become ODE states or iteration variables) and parameters with fixed=false that become iteration variables of the initial system. But this, the compiler does not know in advance.

Yes, this shows that there could be slightly different interpretations of Dialog in a modeling interface and in an interface for configuration of initialization after a model has been translated. It is similar that Dialog.enable = true shouldn't be respected after translation for start attributes of variables with calculated initializability.

Would it be OK to just add a general statement about some of the Dialog settings only making sense for modeling, while others are useful also for creation of interfaces for simulation initialization after translation?

HansOlsson commented 3 years ago

suggest that showStartAttribute is redefined to only apply to parameters.

For me, showStartAttribute=true only makes sense for variables (those that become ODE states or iteration variables) and parameters with fixed=false that become iteration variables of the initial system. But this, the compiler does not know in advance.

For me showStartAttribute=true also make sense for other variables to set start-values independently of state-selection and also for variables that potentially are in that category, but for the specified model don't become states so that we can set fixed=false to indicate that we don't want to set their start-values.

However, that doesn't apply to all non-parameters; if a non-parameter variable has Dialog annotation it is normally a kind of time-varying parameter or input; one could imagine that Modelica.Electrical.Analog.Basic.VariableResistor instead of having R as an input just had Dialog-annotation for it. The user could then set R to 2 or to 2+time.

To me that is a typical use of Dialog-annotation on a non-parameter, and thus I don't see that the default showStartAttribute=false is problematic.

henrikt-ma commented 3 years ago

However, that doesn't apply to all non-parameters; if a non-parameter variable has Dialog annotation it is normally a kind of time-varying parameter or input; one could imagine that Modelica.Electrical.Analog.Basic.VariableResistor instead of having R as an input just had Dialog-annotation for it. The user could then set R to 2 or to 2+time.

To me that is a typical use of Dialog-annotation on a non-parameter, and thus I don't see that the default showStartAttribute=false is problematic.

While I can see that it would be possible to use the same "dialog" for applying that sort of structural modification, it is certainly not what I would consider normal usage of the dialog. Clearly, the trick of only specifying the meaning of showStartAttribute for parameters would support this use case, so then the second best option in my opinion would be to make the default depend on whether it is a parameter/constant or not.

Now that I think more about the parameter case, I realize that a fixed default (of false) also gets in the way for a parameter with fixed = false; it would be much more convenient to allow the tool to figure out that the thing to modify for such a parameter is the start attribute, not the value modification. This is one more reason why the default should be expressed by a suitable logic in the text rather than a fixed value.

HansOlsson commented 3 years ago

While I can see that it would be possible to use the same "dialog" for applying that sort of structural modification, it is certainly not what I would consider normal usage of the dialog. Clearly, the trick of only specifying the meaning of showStartAttribute for parameters would support this use case, so then the second best option in my opinion would be to make the default depend on whether it is a parameter/constant or not.

I disagree. To me the natural default for showStartAttributeis false for both parameters and non-parameters; and the primary use case for showStartAttributeis for non-parameters - but if someone sets showStartAttribute=true (updated) for a parameter I don't see any problem.

You normally want to set the variable to a value - not its attributes such as start and fixed.

An exception are variable that are obviously states - but that is more user knowledge; and a typical use case is Modelica.Mechanics.Rotational.Components.Inertia

Basically if we want to argue that we have made the wrong choice for the default then we should see that there are lots of cases where default is overridden and I don't see that at the moment.

henrikt-ma commented 3 years ago

Basically if we want to argue that we have made the wrong choice for the default then we should see that there are lots of cases where default is overridden and I don't see that at the moment.

Yes, I must admit that this puzzles me. Could it be because of extensive use of separate initialization parameters (for models with different modes of initialization)? Could it be because those who want to set start attributes often do this after translation, and that tools tend to ignore Dialog after translation?

Alternatively, could it be because the feedback we get has a strong bias towards library developers? As a library developer, I can more easily see the value of specifying declaration equations as the "normal" operation. As a library consumer type of user on the other hand, the effects of adding declaration equations inside instances of components from a library seems like an advanced and scary thing to do; I'm much more comfortable just setting start attributes of the time-varying variables.

beutlich commented 3 years ago

That attribute showStartAttribute has caused headache and controversial discussions already: #2043, https://github.com/modelica/ModelicaStandardLibrary/issues/1855, https://github.com/modelica/ModelicaStandardLibrary/pull/2896, https://github.com/modelica/ModelicaStandardLibrary/pull/2897.

henrikt-ma commented 3 years ago

Thanks for pointing this out. My search was too narrow and only revealed #2043, which I thought was actually being properly resolved by the removal of the default for group.

HansOlsson commented 3 years ago

Basically if we want to argue that we have made the wrong choice for the default then we should see that there are lots of cases where default is overridden and I don't see that at the moment.

Yes, I must admit that this puzzles me. Could it be because of extensive use of separate initialization parameters (for models with different modes of initialization)? Could it be because those who want to set start attributes often do this after translation, and that tools tend to ignore Dialog after translation?

To me the simple explanation is that if a library developer want to tell users that they can set start-values for this variable they add showStartAttributes=true and otherwise they don't do this. If it is "time-varying parameter" (like R) they add Dialog-annotation (possibly with group etc).

I simply don't see that there is any problem here.

That attribute showStartAttribute has caused headache and controversial discussions already: #2043, modelica/ModelicaStandardLibrary#1855, modelica/ModelicaStandardLibrary#2896, modelica/ModelicaStandardLibrary#2897.

The first is just naming of the group, so not deeply connected to this. The others is just one issue, which is a combination of misunderstanding of how fixed-attribute work in Modelica and how to initialize in steady state. Since showStartAttributes can also be used in e.g., Modelica.Media to decouple state-selection and initialization I don't see that it is critical for this discussion.

HansOlsson commented 3 years ago

To me this issue can now be closed.

henrikt-ma commented 3 years ago

Basically if we want to argue that we have made the wrong choice for the default then we should see that there are lots of cases where default is overridden and I don't see that at the moment.

Any chance we could get some numbers to support this statement? What is the number of non-parameters with a Dialog? How many out of these have showStartAttribute = true?

HansOlsson commented 3 years ago

There seems to be 21 (28-7) classes using showStartAttribute in MSL 4.

There are about 598 classes using Dialog( and many of them are for non-paramters. Looking through the possibilities in alphabetical order I get:

So it doesn't seem common, and the case for classes makes it clear that changing the default would be problematic. I thus propose we close this one.

henrikt-ma commented 3 years ago

Could it be that the cases when a non-parameter as Dialog but not showStartAttribute = true are typically input variables? Among the classes listed above, only Bend has Dialog without showStartAttribute = true for a component that is neither parameter nor input, but when looking at how Bend is used, an inputs like IN_con is what I found (and then I stopped looking).

In summary: I still haven't seen an example where it wouldn't be helpful to make showStartAttribute default to true for a component that is neither parameter nor input, also taking into account prefixes applied where a record is being used.

To take how a record is used into account could actually become more than just a convenience: we could avoid the need to duplicate the record if we want to use it both as an input/parameter and as a model component in need of guess values.

HansOlsson commented 3 years ago

Could it be that the cases when a non-parameter as Dialog but not showStartAttribute = true are typically input variables?

Possibly. But I don't see that having to explicitly set showStartAttribute for variables that are neither input nor parameter is more complicated for users than understanding it as a default. And if tools want to help users the obvious idea is to base the default for showStartAttribute on such factors when constructing the Dialog-annotation using the GUI.

henrikt-ma commented 3 years ago

Could it be that the cases when a non-parameter as Dialog but not showStartAttribute = true are typically input variables?

Possibly. But I don't see that having to explicitly set showStartAttribute for variables that are neither input nor parameter is more complicated for users than understanding it as a default. And if tools want to help users the obvious idea is to base the default for showStartAttribute on such factors when constructing the Dialog-annotation using the GUI.

OK. That argument at least makes more sense to me. To really make me convinced, I'd need to see relevant real world examples where the intelligent default I'm suggesting wouldn't do the right thing. Because if there aren't any such examples, even a GUI could be simplified by shoving away the showStartAttribute in a dark corner of advanced settings.

HansOlsson commented 3 years ago

There is at least one relevant case where a non-parameter has DialogAnnotation and neither is an input nor should have showStartAttribute: Modelica.ComplexBlocks.Sources.ComplexExpression (there are likely additional cases like this). Clearly we might still add yet another special case to handle that, but to me it's clearly simpler if you write 'showAttribute=true' if you want to show start-attributes.

Thus I propose to close as 'WorksForMe'.

henrikt-ma commented 3 years ago

Yes, that's a valid example where the suggested default wouldn't correspond to the desired behavior. I think I agree that solving that by also making the default depend on output would make the default behavior too complicated, so I would have preferred having to specify showStartAttribute = false for this usage pattern. After all, I think this pattern is extremely rare compared to non-parameters for which it is the start attribute one wants to specify.

As I see a chance of significantly simplifying use of the Dialog annotation, and that we now have found a concrete example where the proposed default based on declared variability wouldn't do the desired thing, I'd like to see a poll before closing as WorksForMe.