Add missing experiment annotations #4516
Conversation
casella
requested review from
maltelenz,
christiankral,
AHaumer and
HansOlsson
|
I don't feel comfortable reviewing this, given that I have little to no experience in this domain. |
| @@ -226,5 +226,6 @@ package Polyphase "Polyphase quasi-static package" | |||
| connect(starI.plug_p,currentSource. plug_p) annotation (Line(points={{-20,-80},{-20,-50}}, color={85,170,255})); | |||
| connect(currentSource.plug_p, capacitor.plug_n) annotation (Line(points={{-20,-50},{-10,-50},{-10,-20},{-20,-20}}, color={85,170,255})); | |||
| connect(capacitor.plug_p,currentSource. plug_n) annotation (Line(points={{-40,-20},{-50,-20},{-50,-50},{-40,-50}}, color={85,170,255})); | |||
| annotation(experiment(StopTime = 1.0, Interval = 0.0002)); | |||
There was a problem hiding this comment.
| annotation(experiment(StopTime = 1.0, Interval = 0.0002)); | |
| annotation(experiment(StopTime = 1.0)); |
I don't see a need for Interval, as the other examples in the same package doesn't have it and still has the same number of outputs, and there is nothing interesting in these signals at that time-scale. I guess the added points are added to make the references "better" in some way.
There was a problem hiding this comment.
Seems reasonable to me. @christiankral, @AHaumer do you agree?
There was a problem hiding this comment.
Since a ramp is used, I would use Interval in the range 0.01 ... 0.001
There was a problem hiding this comment.
Are we discussing the same model? I don't see a ramp in this one (ModelicaTest.Electrical.QuasiStatic.Polyphase.FrequencySweep), but in the other two examples in the package that already had experiment-annotation.
There was a problem hiding this comment.
(without Interval for the other two).
| @@ -265,5 +265,6 @@ package SinglePhase "Single-phase quasi-static package" | |||
| connect(currentCurrent.pin_p, impedanceC.pin_n) annotation (Line(points={{20,-50},{20,-60},{50,-60},{50,-50}}, color={85,170,255})); | |||
| connect(groundI1.pin, currentCurrent.pin_p) annotation (Line(points={{0,-60},{20,-60},{20,-50}}, color={85,170,255})); | |||
| connect(complexRampC.y, impedanceC.Z_ref) annotation (Line(points={{69,-40},{62,-40}}, color={85,170,255})); | |||
| annotation(experiment(StopTime = 1.0, Interval = 0.0002)); | |||
There was a problem hiding this comment.
| annotation(experiment(StopTime = 1.0, Interval = 0.0002)); | |
| annotation(experiment(StopTime = 1.0)); |
I don't see a need for Interval, as the other examples in the same package doesn't have it and still has the same number of outputs, and there is nothing interesting in these signals at that time-scale. I guess the added points are added to make the references "better" in some way.
There was a problem hiding this comment.
In general you're right, for QuasiStatic models we don't need an interval as long as no ramps are included.
I don't see which of the Test Examples is meant.
If we use a ramp, we should consider specifying the Interval to see a meaningfull count of result points during the ramp (maybe 100 or 1000, 5000 is a little bit too much).
There was a problem hiding this comment.
I meant ModelicaTest.Electrical.QuasiStatic.SinglePhase.SerialConnection and ModelicaTest.Electrical.QuasiStatic.SinglePhase.Ideal - and the outputs are just smooth.
beutlich
changed the title
Fixes #4510.
If approved should be ported to maint/4.1.x