Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formalize 'evaluable parameter' #2754

Merged
merged 43 commits into from Apr 5, 2023

Conversation

henrikt-ma
Copy link
Collaborator

@henrikt-ma henrikt-ma commented Dec 7, 2020

The need for formalization of evaluable parameter is far from new, and has recently come up again in different issues:

This PR proposes definitions of evaluable parameter and evaluable expression (corresponding variability) in a way that I hope is more or less in line with how these things work in tools already today (therefore hoping that this doesn't need to be considered MCP content). The three issues above are just some examples of things that would be clarified, making the specification easier to digest for uses, and more straight-forward to implement for tool makers.

At the time of setting up this PR, the changes are restricted to the following essential parts:

  • Definition of evaluable parameter and normal parameter.
  • Definition of evaluable expression (variability).
  • Updated acyclic binding rule.
  • Refined meanings of the Evaluate annotation.

Once we agree what these should be, it remains to update the specification to say evaluable expression rather than parameter expression everywhere something needs to be known at translation time.

I'd be interested to know what other tool makers are willing to share regarding support for these variants, in case that would reveal more things that should actually be locked down by the specification:

  • Respecting Evaluate = false for a constant.
  • Ensuring that no evaluated parameters without Evaluate = true are used in simplifications when verifying the acyclic binding rule.

Original formulation with structural parameter terminology

The rest of this comment shows what this first comment looked like before the change of terminology from structural parameter to evaluable parameter. It might be good to be aware of this original variant in order to be able to make sense of some of the subsequent comments…

The need for formalization of structural parameter is far from new, and has recently come up again in different issues:

This PR proposes definitions of structural parameter and structural expression (corresponding variability) in a way that I hope is more or less in line with how these things work in tools already today (therefore hoping that this doesn't need to be considered MCP content). The three issues above are just some examples of things that would be clarified, making the specification easier to digest for uses, and more straight-forward to implement for tool makers.

At the time of setting up this PR, the changes are restricted to the following essential parts:

  • Definition of structural parameter and normal parameter.
  • Definition of structural expression (variability).
  • Updated acyclic binding rule.
  • Refined meanings of the Evaluate annotation.

Once we agree what these should be, it remains to update the specification to say structural expression rather than parameter expression everywhere something needs to be known at translation time.

I'd be interested…

This includes updating the relation to Evaluate = true.
With separation of normal and structural parameters, it seems possible to give a more imperative meaning of Evaluate, removing some room for ambiguities and unexpected differences between tools.
@perost
Copy link
Collaborator

perost commented Dec 7, 2020

In OpenModelica we define a parameter as structural if it must be evaluated by our frontend at compile time, such as a parameter used to define an array dimension in a simulation model. Your definition seem to be slightly different in that you define all parameters to be structural unless there's a reason for them not to be.

Our implementation is somewhat incompatible in this regard since we require structural parameters to have a binding equation that can be evaluated at compile-time, like constants, while you allow them to be given a value during initialization. Of course, OM is stricter in this case than the specification, so it could be considered a tool issue if this is something we want to allow.

Also, it seems to me like your definition is too broad for "structural" to be the correct word to use, since it will include a lot of parameters that don't actually affect the "structure" of the model. I don't have a better suggestion, but it's perhaps something to consider in order to avoid potentially confusing terminology.

@henrikt-ma
Copy link
Collaborator Author

In OpenModelica we define a parameter as structural if it must be evaluated by our frontend at compile time, such as a parameter used to define an array dimension in a simulation model. Your definition seem to be slightly different in that you define all parameters to be structural unless there's a reason for them not to be.

Also, it seems to me like your definition is too broad for "structural" to be the correct word to use, since it will include a lot of parameters that don't actually affect the "structure" of the model. I don't have a better suggestion, but it's perhaps something to consider in order to avoid potentially confusing terminology.

Yes, I know that the proposed nomenclature doesn't match exactly what many of us have in mind. I think it could be both good and bad to reuse terms that already have associate meanings outside the current specification:

  • By finding a formal definition of structural parameter that we can unite behind, we might in the end be able to move away from a situation where the term is used differently in different tools.
  • Of course, learning a new meaning for familiar term can also be quite challenging.

I should have said it from the beginning: What is commonly thought of as a "structural parameter" today, would in the current proposal translate into a (structural) parameter required by a structural expression (and as you noted, such a parameter has to be structural). Often, however, I believe that a more interesting classification is an evaluated structural parameter, which doesn't have to do with where the structural parameter is used, but emphasizing that this structural parameter is determined at translation time.

When choosing terminology, I'd like to start with the kind of expression variability that is required for things like array dimensions. Here, I think structural expression is a very natural name considering that we all seem to have some sort of "structural" in mind when it comes to these things. Then, we also need a name for a (more or less explicitly) declared variability associated with structural expression, and then structural parameter was the most natural I could think of.

Our implementation is somewhat incompatible in this regard since we require structural parameters to have a binding equation that can be evaluated at compile-time, like constants, while you allow them to be given a value during initialization. Of course, OM is stricter in this case than the specification, so it could be considered a tool issue if this is something we want to allow.

This sounds like a misunderstanding, unless this is only confusion due to reuse of terminology. The point is that just like today's "unqualified" parameters, a structural parameter does not have to be determined during translation, but the intention is really that a structural parameter is something as easy to evaluate at translation time as a constant. Wherever the specification requires a structural expression, all parameters involved will need to be structural, and will be possible to determine during translation. Hopefully, tools will be able to agree on which the structural parameters are (this PR might not be sufficient to really get us there), and then only differ when it comes to which of those that become determined during translation.

chapters/classes.tex Outdated Show resolved Hide resolved
Copy link
Collaborator

@gkurzbach gkurzbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally agree with this proposal.

chapters/annotations.tex Outdated Show resolved Hide resolved
chapters/annotations.tex Outdated Show resolved Hide resolved
chapters/classes.tex Outdated Show resolved Hide resolved
@henrikt-ma
Copy link
Collaborator Author

When choosing terminology, I'd like to start with the kind of expression variability that is required for things like array dimensions. Here, I think structural expression is a very natural name considering that we all seem to have some sort of "structural" in mind when it comes to these things. Then, we also need a name for a (more or less explicitly) declared variability associated with structural expression, and then structural parameter was the most natural I could think of.

To not only write in defense of the proposed nomenclature, let's make a list of alternatives:

Expression variability Component variability Comment
1 structural expression structural parameter As currently proposed (approved by spell checker)
2 evaluatable expression evaluatable parameter
3 constish expression constish parameter It's almost like a constant (German vibes, anyone?)

Co-authored-by: Gerd Kurzbach <Gerd.Kurzbach@esi-group.com>
@MartinOtter
Copy link
Member

I only scanned this pull request due to lack of time (and maybe missed essential points):

If this proposal is just for making the specification clearer without changing existing models (and the existing semantics of the Modelica language), this is of course good. However, I fear that you plan to introduce restrictions on the language that are currently not present, and this in a core part of the language that is present in nearly every model, at least if arrays are declared. Please, can you confirm, that the current semantics of the Modelica language is not changed with your proposal and that it will not have any effect on existing models.

@henrikt-ma
Copy link
Collaborator Author

I only scanned this pull request due to lack of time (and maybe missed essential points):

If this proposal is just for making the specification clearer without changing existing models (and the existing semantics of the Modelica language), this is of course good. However, I fear that you plan to introduce restrictions on the language that are currently not present, and this in a core part of the language that is present in nearly every model, at least if arrays are declared. Please, can you confirm, that the current semantics of the Modelica language is not changed with your proposal and that it will not have any effect on existing models.

The goal is to specify this in a way that doesn't require changing existing models, at least as long as one stays away from models that exhibit tool specific corner case behaviors. For example, since none of the MSL models should depend on such tool specific behaviors, it should work just like as before.

Trying to summarize implications for semantics (let's assume #2754 (comment) is accepted):

  • Criterion for being a structural parameter is in some ways more liberal (and easier to check) than the old acyclic binding rule.
  • Non-fixed parameters are not structural, whereas it used to be possible to have them evaluated (see example below).
  • Refined meanings of the Evaluate annotation by taking advantage of separation of normal and structural parameters, giving users better control over what is and what isn't determined during translation.

Example of changed semantics for non-fixed parameter:

model M
  parameter Integer p = 1;
  parameter Integer n(fixed = false);
  /* Before, this wasn't illegal, since there was no rule against solving for n at translation time,
   * and force evaluation of p.
   * Now, this becomes illegal, since n is a normal parameter.
   */
  Real x[n];
initial equation
  n = p + 1;
end M;

Copy link
Collaborator

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If people don't agree about the implications of this proposal it is clear that we need to change it.

@henrikt-ma
Copy link
Collaborator Author

If people don't agree about the implications of this proposal it is clear that we need to change it.

Yes. I am doing my best to identify any implications causing disagreement, and find ways to adapt the proposal. The proposal is in Draft state for good reasons. :)

chapters/classes.tex Outdated Show resolved Hide resolved
chapters/classes.tex Outdated Show resolved Hide resolved
@henrikt-ma henrikt-ma marked this pull request as ready for review November 10, 2022 13:42
@henrikt-ma
Copy link
Collaborator Author

Removed Draft status to indicate that this seems to have converged to something where at most some minor remarks remain to be resolved.

@HansOlsson
Copy link
Collaborator

I think we might need to revisit this again in detail the next year, as we are getting user requests for allowing at least some more attributes to be non-evaluated parameters. (Currently Dymola forces all attributes except start to be evaluated for various reasons.) That needs more investigation.

It shouldn't prevent us from formalizing structural parameters - and it may even make it easier to describe as it reduces the need to have parameters as structural.

However, when glancing through the proposal I couldn't see anything related to these attributes, which makes me a bit confused. On one hand it seems in line with not requiring some of the attributes to be evaluated, but on the other hand at least the fixed-attribute seems like a structural parameter that normally needs to be evaluated (it determines the structure of the initialization problem).

A minor note is that Dymola also allow Evaluate to take a parameter-expression (that needs to be evaluated, of course), and not only literal values - but that can be a separate enhancement.

@henrikt-ma
Copy link
Collaborator Author

However, when glancing through the proposal I couldn't see anything related to these attributes, which makes me a bit confused. On one hand it seems in line with not requiring some of the attributes to be evaluated, but on the other hand at least the fixed-attribute seems like a structural parameter that normally needs to be evaluated (it determines the structure of the initialization problem).

This is a correct observation. The first step is to formalize the concept. The next step is to apply the new concept in selected places, and then we might need to have separate issues about different places of application.

@henrikt-ma
Copy link
Collaborator Author

henrikt-ma commented Mar 14, 2023

Design meeting:

Gerd: end is not a function call.

Elena: "normal" is a very overloaded word.

Alternatives with poll:

  • normal parameter: 0
  • non-evaluable parameter: 5
  • abstain: 2

Henrik: Good to at least include array size as first case of an evaluable expression.

Language group: Include array size as first example.

Hans: There are two lists of things today that should be required evaluable: things that are explained one way or another today, and things where nothing related to evaluability is said today. Examples of the first: if-equation with connect-equation inside, or unbalanced if-equations. Examples of the latter fixed and stateSelect attributes.

Plan:

  • Change terminology to non-evaluable parameter.
  • Open separate issue for end, and don't start making changes until this is merged.
  • Include array size as first case of evaluable.
  • Merge new terminology without deciding where it should be applied (as in the current state of this PR).

@henrikt-ma henrikt-ma force-pushed the formalize-structural-parameter branch from 7d99775 to 3b05128 Compare March 14, 2023 21:42
@henrikt-ma
Copy link
Collaborator Author

henrikt-ma commented Mar 14, 2023

The terminology has been updated to non-evaluable parameter, the problem with end has been filed separately as #3351, and merge conflicts has been resolved.

@HansOlsson, you have a very old review that is still blocking this. Could you please provide a fresh review?

chapters/classes.tex Outdated Show resolved Hide resolved
Copy link
Collaborator

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok, except for some minor issues.

@HansOlsson HansOlsson self-requested a review March 20, 2023 13:02
Copy link
Collaborator

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comment, but seems good enough.

@HansOlsson HansOlsson merged commit 5cfbbf4 into modelica:master Apr 5, 2023
@henrikt-ma henrikt-ma deleted the formalize-structural-parameter branch April 10, 2023 20:50
@HansOlsson HansOlsson added the M37 For pull requests merged into Modelica 3.7 label Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request M37 For pull requests merged into Modelica 3.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet