ModelicaStandardLibrary icon indicating copy to clipboard operation
ModelicaStandardLibrary copied to clipboard

Add new source block BooleanConstantVariability and introduce BooleanParameter as an alternative name for BooleanConstant

Open henrikt-ma opened this issue 6 years ago • 26 comments

Sometimes, a Boolean signal may have structural impact on the model equations if it can be evaluated during translation. The Modelica.Blocks.Sources.BooleanConstant source cannot be used to this end in general, as it may be totally fine for a tool to not evaluate the parameter. Hence, a new source block is needed to express that the Boolean signal should really be considered a constant by the tools.

It is a pity that the old block was named BooleanConstant, but I guess this can be made right later using a conversion:

  1. Any use of BooleanConstant is changed to use BooleanParameter.
  2. Remove BooleanConstant.
  3. Rename BooleanConstantVariabilityBooleanConstant.

henrikt-ma avatar Sep 06 '19 10:09 henrikt-ma

I don't see that the documentation is technically correct for this block (both before and after change), and the change seems to be related to that text.

The output y (in both old and new model) is just a normal variable in terms of Modelica semantics - neither parameter nor constant. And we cannot change that(*). Deducing that it is 'constant' requires a BLT-transformation that isn't specified/needed in the current specification. Or in other words: don't use block-diagrams for computing parameters/constants.

Obviously in a loose sense y doesn't change during the simulation and is "constant" it that way - but I believe it would be better to describe that with a word that isn't part of the Modelica semantics.

This also implies that using the output as a structural parameter seems odd. However, I believe that the documentation of the current block should be updated - as these implications aren't clear.

(*): To clarify that we cannot change y to constant:

  • Technically we could change y, but blocks are connected to other blocks; so it is meaningless to only change y without modifying the connected blocks as well.
  • However, changing the connected blocks so that the variable y is connected to (apart from being messy) is parameter/constant doesn't work as the connection-equation would then be gone.

HansOlsson avatar Sep 06 '19 11:09 HansOlsson

The intention is similar to this situation:

model ConstantSimplification
  constant Boolean a = true;
  Boolean b = a;
  Real x(start = 1.0, fixed = true);
equation
  if b then
    der(x) = x;
  else
    der(x) = -x;
  end if;
end ConstantSimplification;

Here, it is possible to propagate the variability of a to b, and then keep just one of the if equation branches. With a a parameter, this wouldn't be possible.

I don't understand why the same technique wouldn't be useful with a source block.

henrikt-ma avatar Sep 09 '19 06:09 henrikt-ma

As a more concrete example of what one might want to do with a constant source block, it can be connected to a Modelica.Blocks.Logical.Switch to change the routing of a model without resorting to conditional components.

henrikt-ma avatar Sep 09 '19 12:09 henrikt-ma

As a more concrete example of what one might want to do with a constant source block, it can be connected to a Modelica.Blocks.Logical.Switch to change the routing of a model without resorting to conditional components.

But it requires a propagation of variability that isn't specified, and it mostly relevant when you want to change parameters for a translated model- and that isn't described in detail either (the Evaluate-annotation is specified - but only proposes something).

HansOlsson avatar Sep 09 '19 12:09 HansOlsson

But it requires a propagation of variability that isn't specified, and it mostly relevant when you want to change parameters for a translated model- and that isn't described in detail either (the Evaluate-annotation is specified - but only proposes something).

But I don't see that propagation of variability is something that is necessary to specify — it just happens due to the declarative nature of Modelica equations. If a = b, then a and b must have the same variability, bounded from above by the declared variabilities of a and b.

henrikt-ma avatar Sep 10 '19 07:09 henrikt-ma

By purpose there a blocks for different data types that provide a mathematical constant, so the output of the block does not change over time: Modelica.Blocks.Sources.Constant/.BooleanConstant/.IntegerConstant. In a similar way there are blocks for "Steps".

To summarize, whenever BooleanConstant would be changed, then Constant and IntegerConstant would have to be changed as well. These blocks are used so often, that I do not like that they are renamed, especially because the names make perfect sense from a control perspective.

MartinOtter avatar Sep 11 '19 08:09 MartinOtter

Do not change the names of the blocks: BooleanConstant, Constant, IntegerConstant

Sure. I see that it would be controversial, and that is why it isn't included as part of this PR. For this PR, the renaming discussion could be seen primarily as a defense for the lengthy name BooleanConstantVariability

henrikt-ma avatar Sep 16 '19 05:09 henrikt-ma

By purpose there a blocks for different data types that provide a mathematical constant, so the output of the block does not change over time: Modelica.Blocks.Sources.Constant/.BooleanConstant/.IntegerConstant. In a similar way there are blocks for "Steps".

But this is the Modelica Standard Library, not the Mathematics Standard Library, and Modelica has precise meanings for the terms constant and parameter.

To summarize, whenever BooleanConstant would be changed, then Constant and IntegerConstant would have to be changed as well. These blocks are used so often, that I do not like that they are renamed, especially because the names make perfect sense from a control perspective.

I totally agree that the blocks for Boolean, Integer, and Real should be named consistently. For completeness of this PR, I will extend it to also cover Integer and Real.

The Blocks are too fundamental to just be considered a control systems construct.

henrikt-ma avatar Sep 16 '19 06:09 henrikt-ma

The Blocks are too fundamental to just be considered a control systems construct.

I disagree.

All data given in menus of Modelica blocks are parameters (from a few exceptions). Therefore, it does not make sense to add "parameter" or "parameters" to the class name. The essential point is not that a generic parameter is defined in the component, but that a signal is returned from the block that is not changed over time and this signal is definend by a parameter. The block could be called "ConstantSignal" or "ConstantOutputSignal" to be more precise, but then one should change "Step" to "StepSignal", so not good (in general, all the classes of Modelica.Blocks generate output signals, so it does not make sense to include this in the class name).

Additionally, many users are used to a "Constant" block, because this is also the name used in other programs, such as "Simulink". I guess, no simulation program in the world is calling such a block "BooleanParameter".

MartinOtter avatar Sep 16 '19 09:09 MartinOtter

Sure. I see that it would be controversial, and that is why it isn't included as part of this PR. For this PR, the renaming discussion could be seen primarily as a defense for the lengthy name BooleanConstantVariability

Currently, this block would not be useful in Dymola or SimulationX, because constants with a Dialog annotation are not displayed in the parameter menu. Therefore, this needs more discussion in the language group, before such a block should be included in MSL.

In Dymola, I am used that Dymola evaluates a parameter at compile time, if the tool thinks this is needed (e.g. because a parameter is used as structural parameter). There are so many reasons why a tool needs to evaluate a parameter at compile time, that it does not make sense to me to move this decision to the modeler. A "constant" usage in a model should be an exceptional case (e.g. constant Real pi = Modelca.Constants.pi, or constant Real eps = ...). Everything else should be a parameter and the tool decides when to evaluate it.

Therefore, I am against to include a block with a "constant" in the parameter menu

MartinOtter avatar Sep 16 '19 09:09 MartinOtter

Currently, this block would not be useful in Dymola or SimulationX, because constants with a Dialog annotation are not displayed in the parameter menu. Therefore, this needs more discussion in the language group, before such a block should be included in MSL.

OK. Filed ModelicaSpecification/issues/2413.

In Dymola, I am used that Dymola evaluates a parameter at compile time, if the tool thinks this is needed (e.g. because a parameter is used as structural parameter). There are so many reasons why a tool needs to evaluate a parameter at compile time, that it does not make sense to me to move this decision to the modeler. A "constant" usage in a model should be an exceptional case (e.g. constant Real pi = Modelca.Constants.pi, or constant Real eps = ...). Everything else should be a parameter and the tool decides when to evaluate it.

In that case you wouldn't ever set Evaluate=true either, right?

henrikt-ma avatar Sep 20 '19 05:09 henrikt-ma

All data given in menus of Modelica blocks are parameters (from a few exceptions). Therefore, it does not make sense to add "parameter" or "parameters" to the class name. The essential point is not that a generic parameter is defined in the component, but that a signal is returned from the block that is not changed over time and this signal is definend by a parameter.

The naming isn't meant to reflect the internal parameterization of the block, but what sort of signal it provides to the outside world. The block I'm introducing here provides a signal with constant variability, whereas the old block provides a signal with parameter variability.

Additionally, many users are used to a "Constant" block, because this is also the name used in other programs, such as "Simulink". I guess, no simulation program in the world is calling such a block "BooleanParameter".

It is true that Simulink users may have a different idea about what a "Constant" block is, but I think it makes good sense for MSL to be more loyal to the Modelica Specification (where a constant is not the same as a parameter) than being loyal to Simulink. Besides, if a Simulink user would happen to choose the block with constant output, it wouldn't be the end of the world; it would just mean that it shouldn't be expected that k can be changed without translating the model again.

henrikt-ma avatar Sep 20 '19 05:09 henrikt-ma

Clearly, this change shoud not be done as explained above, because it is standard in a very large community to name a constant signal block "constant". Therefore closing this pull request

MartinOtter avatar Oct 16 '19 15:10 MartinOtter

Please note that the name of the new block in this PR is BooleanConstantVariability, so the objection doesn't apply to the actual content of the PR, just to a discussion about a possible future change. Reopening.

henrikt-ma avatar Oct 17 '19 09:10 henrikt-ma

I would also prefer to keep the names Constant , BooleanConstant and IntegerConstant. The currently proposed PR does not change the original name(s). So this is good.

It however makes sense to add to the documentation of the "old" "constant" blocks that the variability is the one of a parameter, not a constant.

For the newly proposed block I guess we shall come to a decision in order to proceed with this PR. Personally, I think the name is OK, if there are no other proposals.

@MartinOtter @AHaumer Are you as library offices in favor of this PR, as it is?

If yes, an integer and real version shall be provided as well.

christiankral avatar Jan 16 '20 20:01 christiankral

@christiankral I'm also in favour of keeping the names Constant , BooleanConstant and IntegerConstant with variability = parameter. on the other hand, I see the intention of @henrikt-ma and I'm in favour of having components with variability = constant, but maybe we find a better name ... looks a little bit ugly: Constant , Constant Variability Any suggestions are welcome.

AHaumer avatar Jan 17 '20 06:01 AHaumer

@christiankral I'm also in favour of keeping the names Constant , BooleanConstant and IntegerConstant with variability = parameter. on the other hand, I see the intention of @henrikt-ma and I'm in favour of having components with variability = constant, but maybe we find a better name ... looks a little bit ugly: Constant , Constant Variability Any suggestions are welcome.

Note that we have to be careful with terminology here.

Even if the the signal-generator has constant (or parameter) variability it doesn't mean that the signal has constant (or parameter) variability, since that would create issues with generating equations from connect (connections between constants just generate asserts).

HansOlsson avatar Jan 17 '20 08:01 HansOlsson

The use case is not to connect two connectors with constants, but to connect the constant with a non-constant, allowing the tool to infer that the latter is also constant.

henrikt-ma avatar Jan 17 '20 08:01 henrikt-ma

The use case is not to connect two connectors with constants, but to connect the constant with a non-constant, allowing the tool to infer that the latter is also constant.

I understand, but I was merely warning that we have to careful what we describe as constant variability; and even if the tools infer that the latter connector-signal is constant (and practically speaking has constant variability) it isn't inferred to be "constant variability" in terms of Modelica semantics.

HansOlsson avatar Jan 17 '20 09:01 HansOlsson

These sources are usefull, no doubt. As far as I see there are 2 issues:

  1. It's a matter of finding a proper name.
  2. More tools should support Constant Real c annotation(Dialog(...));

Maybe we should collect suggestions and vote for the name? My suggestion: ConstantSignal

AHaumer avatar Jan 18 '20 09:01 AHaumer

ConstantVariable

christiankral avatar Jan 20 '20 06:01 christiankral

I don't think ConstantVariable is a good name as we currently have the three types of variability: Constants, Parameters and Variables. A name combining those would be quite confusing.

dietmarw avatar Jan 20 '20 07:01 dietmarw

From my point of view, where the source of the confusion is that BooleanConstant in fact introduces a parameter, the best way forward would be to find a migration path to an new naming convention where the name of the block is consistent with the Modelica variability of the variable it introduces. See the opening comment of this PR for my original proposal of such a migration path.

An alternative migration path would be to not reuse BooleanConstant and friends in the end, but just deprecate these and proceed with completely new names, such as ConstantBoolean and ParameterBoolean.

henrikt-ma avatar Jan 20 '20 07:01 henrikt-ma

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jun 11 '20 14:06 CLAassistant

I don't think ConstantVariable is a good name as we currently have the three types of variability: Constants, Parameters and Variables. A name combining those would be quite confusing.

The variabilities we have are:

  • Constant
  • Parameter
  • Discrete-time
  • Continuous-time

That is, ConstantVariable is not a combination of two of these. However, I really prefer the kind of names we have today, concentrating on the variability. I mean, isn't Parameter clearly a better name than ParameterVariable?

If someone would insist on a combined name such as ConstantVariable, wouldn't ConstantSource and ParameterSource be better names?

henrikt-ma avatar Jun 12 '20 05:06 henrikt-ma

OK, let's take it for granted that generalization to the other primitive types will be done as soon as we have an agreement about Boolean.

henrikt-ma avatar Jun 12 '20 21:06 henrikt-ma