Inherit experiment

[HansOlsson]

Specify inheritance of experiment annotation.
Closes #2314

[maltelenz]

Why should experiment annotations not be merged? That way you could specify common settings in one place in a partial base class, and "fill out" differing fields in the simulation model. I think that would be a much more useful behavior.

Backwards compatibility should not be a concern, since this was previously undefined behavior, and different tools have different behavior.

[HansOlsson]

Why should experiment annotations not be merged?

That was the result of the discussion at the previous meeting.

The issue is that if you in a model have
annotation(experiment(StopTime=1))
you currently don't expect that you need to check all base-classes to see whether they specify e.g. Tolerance. And to avoid that models will then specify annotation(experiment(StartTime=0, StopTime=1, Tolerance=1e-4, ...)).

Note that the text was based on the similar text for coordinate system.

I agree that having some way of specifying "inherit experiment and modify it" would be good, but this was just intended as a solution for the current issue - and not a complete design.

[maltelenz]

Why should experiment annotations not be merged?

That was the result of the discussion at the previous meeting.

The issue is that if you in a model have
annotation(experiment(StopTime=1))
you currently don't expect that you need to check all base-classes to see whether they specify e.g. Tolerance.

Who is the quoted "you" in this context? I certainly expect that I need to check all base classes, since that is the implementation in SystemModeler.

I agree that having some way of specifying "inherit experiment and modify it" would be good, but this was just intended as a solution for the current issue - and not a complete design.

But this paints us into a corner for no good reason, preventing the (in my opinion) better design of being able to inherit partial experiments.

[henrikt-ma]

I agree with @maltelenz here, but just wanted to mention that perhaps even more important than inheriting a partial experiment annotation, is the ability to define a base class with complete experiment annotation, and then being able to override just parts of it.

[henrikt-ma]

See comment above.

[HansOlsson]

Have (based on feedback) changed to specify that you only override specific fields.

[maltelenz]

Suggested fix for minor spelling error, otherwise looks good!

Dismissing my own review with request for changes, as I'd like to just leave a comment for now.

[henrikt-ma]

I am personally in favor of accepting the PR as it is now. However, I tried to find a clear language group statement about going with this formulation, but couldn't find it. @HansOlsson could you please point us to the relevant phone meeting decision if there is one, or plan to get language group approval in the next meeting?

[HansOlsson]

Language group:
(Should clearly allow StopTime in partial model for this to work; so that it is not considered a simulation model in that case.)
Favor of overriding specific ones: Elena, Dag, Stephan, Gerd, Quentin
Against:
Abstain: Markus

[henrikt-ma]

Language group: (Should clearly allow StopTime in partial model for this to work; so that it is not considered a simulation model in that case.) Favor of overriding specific ones: Elena, Dag, Stephan, Gerd, Quentin Against: Abstain: Markus

I take this for a language group approval of the current formulation, and will approve the PR accordingly.

[henrikt-ma]

OK, and as far as I can tell, according to phone meeting decision.

[HansOlsson]

I realized that I could either spend a long time trying to figure out how to rebase/merge the current status in the proper way, and be certain that nothing was missed. Or just rebase skip and then re-add the four lines that were added.