ModelicaSpecification icon indicating copy to clipboard operation
ModelicaSpecification copied to clipboard

Define experiment using a record.

Open HansOlsson opened this issue 3 years ago • 10 comments

Remove the empty experiment and define it using a record instead (per discussions). Closes #2985

HansOlsson avatar Sep 10 '21 12:09 HansOlsson

Note that the important part is that now it is clear that StopTime must be specified, but the others have defaults (0 for StartTime - which matters, Tolerance and Interval are less critical).

That is consistent with the use in MSL where some examples only specify StopTime, some StopTime and Interval, and some StopTime Interval and Tolerance. And some everything include StopTime, and some just StartTime and StopTime.

HansOlsson avatar Apr 12 '22 09:04 HansOlsson

According to the current text of the PR, StopTime must be specified. Does it mean an error must be given if the model is used a stand-alone?

I'd say no; it just means that from the specification's point of view the model isn't a simulation model. Maybe we don't need to specify anything further for this case and leave it up to tools to decide what to do?

henrikt-ma avatar Apr 14 '22 07:04 henrikt-ma

In general, I like the changes. But I feel like there is a point missing. Maybe it's the conflict between the intent of the experiment annotation the way it is defined in the specification and the way I see it being used in practice. Specifically, what I see in the MSL, for example, is that the presence of the experiment annotation with specified StopTime indicates that this is an example and StopTime, etc are the simulation parameters. In general, I see the presence of the annotation meaning that the model can be used as a stand-alone model for simulation. If there is no such annotation, it is does not mean it cannot be used a stand-alone simulation model but there is no guarantee that it has any meaning (like model of a resistor is not really a complete simulation model).

I agree with this general idea - but as I see it there are two possibilities for stating this requirement. Either it is the presence of experiment that triggers this or the presence of experiment.StopTime.

After some thinking I believe that we should use experiment.StopTime; since it seems that otherwise we have introduce an error case (experiment without StopTime) just to annoy users.

This meaning of the presence of the experiment annotation is not currently reflected in the Specification. And from that, there is also the question what it means if somebody specified experiment but did not specify StopTime (which was the point of #2985). The precedence in the MSL is that it is not considered an example model.

This is now handled (although I think those models should be corrected).

HansOlsson avatar Apr 21 '22 14:04 HansOlsson

We need to resolve that discussion together with this, or write this one in a way that doesn't prevent the flexible solution of merged inheritance.

In order to handle that in the future I ensure that the statement for StopTime requires that it is set; not modified.

HansOlsson avatar Apr 21 '22 14:04 HansOlsson

I disagree that providing a value for StopTime is a necessary condition.

I agree, and we have to be clear what we mean.

The line "If \lstinline!StopTime! is set the model should be a simulation model." wasn't intended as a necessary condition, but instead my intent was that value for StopTime requires that it should be a simulation model. (And similarly if the user wants to simulate the model then it required that it is simulation model.)

Is "If \lstinline!StopTime! is set the model is required to be a simulation model." clearer, or can you propose something capturing this intent?

Somewhat semi-formally:

  • M.experiment.StopTime having a value requires IsSimulationModel(M)
  • Simulating M requires IsSimulationModel(M)

where IsSimulationModel(M) are the requirements that M needs to satisfy to be a simulation model. That's why checking a library can attempt to simulate all models having a value for experiment.StopTime, since they are required to be simulation models.

HansOlsson avatar May 12 '22 12:05 HansOlsson

That's why checking a library can attempt to simulate all models having a value for experiment.StopTime, since they are required to be simulation models.

Except if they are partial, to allow specifying the stop time in a base class to the actual simulation model?

maltelenz avatar May 12 '22 13:05 maltelenz

That's why checking a library can attempt to simulate all models having a value for experiment.StopTime, since they are required to be simulation models.

Except if they are partial, to allow specifying the stop time in a base class to the actual simulation model?

I would say that we wait for #2314 before adding something like that.

Note that it actually more complicated than "use partial and experiment-annotation for the base-class of simulation model" as one common way of making base-classes for simulation models is to use templates with replaceable sub-components of partial classes. But the templates are normally not partial, and in theory you could construct a short-class definition (that would "inherit" the partial) turning them into a simulation model.

On the other hand, it is often circumvented by ensuring that the base-class is actually a simulation-model (and then it is only the components constraining types that are partial).

HansOlsson avatar May 12 '22 13:05 HansOlsson

Is "If \lstinline!StopTime! is set the model is required to be a simulation model." clearer, or can you propose something capturing this intent?

I think it is. That also answers the question "What is the meaning of an empty experiment annotation?" Nothing. It is the same as empty within clauses. Place-holder for something that can be specified but it is not needed for this model, so left empty.

eshmoylova avatar May 12 '22 13:05 eshmoylova

That's why checking a library can attempt to simulate all models having a value for experiment.StopTime, since they are required to be simulation models.

Except if they are partial, to allow specifying the stop time in a base class to the actual simulation model?

I would say that we wait for #2314 before adding something like that.

Sure, I just wanted to make sure we don't choose a formulation that prevents us from doing that in #2314 (if we want to).

maltelenz avatar May 12 '22 13:05 maltelenz

Summary of requested changes:

  • Leave door open for different interpretations of experiment inheritance.

They will be added later in a different PR, and shouldn't block this one.

  • Relation between StopTime and simulation model needs input from more parties.

This is a non-constructive blocker.

You first pushed for adding the relation between StopTime and simulation model, and when explaining the current status quo (as it is used in e.g. MSL) you then try to block the PR.

  • Some things about the style of presentation.

HansOlsson avatar May 20 '22 15:05 HansOlsson

@maltelenz @eshmoylova @henrikt-ma please redo the reviews.

HansOlsson avatar Nov 09 '23 14:11 HansOlsson

Looks OK.

Now that the annotation isn't defined in terms of UNSIGNED-NUMBER, this raises the question of expression variability… Perhaps we should open another PR about specifying which of these are parameter (evaluable/constant) expressions before we forget about it?

Yes. But in a bit, since I think we should first erase the other unneeded uses of grammar in that chapter.

HansOlsson avatar Nov 21 '23 11:11 HansOlsson