MSL 4.1.0 Reference results Modelica.Electrical.Machines.Examples.SynchronousMachines.SMPM_NoLoad

[maltelenz]

After #4031 the reference signals for Modelica.Electrical.Machines.Examples.SynchronousMachines.SMPM_NoLoad changed, so we need new reference results.

I also question why the test is not marked red in the report: https://www.ltx.de/download/MA/Compare_MSL_v4.1.0/Compare/Modelica/testrun_report.html

[beutlich]

I also question why the test is not marked red in the report: https://www.ltx.de/download/MA/Compare_MSL_v4.1.0/Compare/Modelica/testrun_report.html

To answer this question. It is marked as N/A since none of the new signal names

toPolar.y[1]
toPolar.y[2]
hallSensor.y

is available in https://github.com/modelica/MAP-LIB_ReferenceResults/blob/2be1c7e08a39d658b1326607917a0a32749e76e1/Modelica/Electrical/Machines/Examples/SynchronousMachines/SMPM_NoLoad/SMPM_NoLoad.csv

[maltelenz]

That sounds serious enough to warrant red coloring, if the reference result doesn't even contain the variables one expects to test...

[beutlich]

Well, it is neither red nor green, but inclonclusive indicating that the comparison test could not be completed with the data available. There is yellow color also in other places used, e.g., if simulation time exceeds some threshold, I guess.

[beutlich]

Well, csv-compare writes a failed marker filer indeed: compare_failed.log whereas the report marks it as UNTESTED:

[MatthiasBSchaefer]

We are only counting failed comparison.
Something, that can not be compared is not a failed comparison.

We can introduce a yellow instead of a gray field for this, if this makes you happy ...

[maltelenz]

We can introduce a yellow instead of a gray field for this, if this makes you happy ...

That sounds better than gray. It is a serious issue with the test or regression result that needs solving, so it would be nice if the report can clearly mark it so it stands out.

[GallLeo]

Fixed via reference update
modelica/MAP-LIB_ReferenceResults@6db6c50

See also discussion in in /issues/4333

[casella]

The test of Modelica.Electrical.Machines.Examples.SynchronousMachines.SMPM_NoLoad on the LTX server now shows green.

[casella]

While checking MSL 4.1.0 regressions in OMC, I noticed that Modelica.Electrical.Machines.Examples.SynchronousMachines.SMPM_NoLoad was passing the verification testing successfully in MSL 4.0.0, while it doesn't in MSL 4.1.0, due to a mismatch in toPolar.y[2]

I checked the new reference file. The value of toPolar.y[2] at time = 0.015 is 3.1415927 (i.e. pi), so the mismatch is probably due to the fact that the value at time = 0.015 can either be -pi or pi depending on small numerical rounding errors.

In any case, the new reference file stops at time = 0.039999999, despite the model having StopTime = 0.32.

[casella]

@MatthiasBSchaefer, @GallLeo why these settings were changed in modelica/MAP-LIB_ReferenceResults@7d62e18?

StartTime = 0.0    // set by user or program default value
StopTime = 0.04    // set by user or program default value
Interval = 5e-05    // set by user or program default value
Tolerance = 1e-06    // set by user or program default value

[maltelenz]

@MatthiasBSchaefer, @GallLeo why these settings were changed in modelica/MAP-LIB_ReferenceResults@7d62e18?

StartTime = 0.0    // set by user or program default value
StopTime = 0.04    // set by user or program default value
Interval = 5e-05    // set by user or program default value
Tolerance = 1e-06    // set by user or program default value

They were not, it is just whitespace and comment changes?

[MatthiasBSchaefer]

@casella
The StopTime was already 0.04 before modelica/MAP-LIB_ReferenceResults@7d62e18

The StopTime was changed in 01c3f90, but never used for new reference files, which is unfortunately a bit complicated in our tooling:
We need ONE single source to get the right experiment-settings (completely: tolerance, outputInterval, StopTime,...).
Currently this is the creation.txt to get the tighter tolerance for testing and such changes in stopTome are ignored.

[AHaumer]

The example has been changed from 4.0.0 to 4.1.0 (01c3f90):
4.0.0 the machine is driven at constant speed, stop time is 0.04.

4.1.0 the machine is driven by a speed ramp, stop time is 0.32.

However, the diagram shown by @casella is obtained from 4.1.0.
The difference stems from the equation in Modelica.Electrical.Machines.SpacePhasors.Blocks.ToPolar:
y[2] = if noEvent(y[1] <= small) then 0 else Modelica.Math.atan2(u[2], u[1]);
As far as I remember the "noEvent" is used to avoid a problem for detecting the angle of (x=0, y=0), i.e. for y[1]=0.
IMHO the ramp from -pi to +pi should be a step. Is this a problem buried in atan2?
It seems that in case of that step/ramp the result of atan2 depends on the Interval.

[casella]

We need ONE single source to get the right experiment-settings (completely: tolerance, outputInterval, StopTime,...).

I couldn't agree more.

I actually made a proposal to address this issue with suitable annotations, see modelica/ModelicaSpecification#3473. Unfortunately, this generated a lot of controversy and at some point I moved on to other things and nobody else seemed to care any longer. Maybe we should refocus on it, once MSL 4.1.0 is released 😃

In the meantime, we should get this fixed.

[casella]

The example has been changed from 4.0.0 to 4.1.0 (01c3f90): 4.0.0 the machine is driven at constant speed, stop time is 0.04.
4.1.0 the machine is driven by a speed ramp, stop time is 0.32.

OK, so we should edit creation.txt accordingly, changing the two values of stopTime that show up in there, then have LTX re-generate the reference results and upload them to the 4.1.0 branch of the reference results repo.

However, the diagram shown by @casella is obtained from 4.1.0.

Sure. So, it should last 0.32 s, not 0.04 as currently shown.

The difference stems from the equation in Modelica.Electrical.Machines.SpacePhasors.Blocks.ToPolar: y[2] = if noEvent(y[1] <= small) then 0 else Modelica.Math.atan2(u[2], u[1]); As far as I remember the "noEvent" is used to avoid a problem for detecting the angle of (x=0, y=0), i.e. for y[1]=0. IMHO the ramp from -pi to +pi should be a step.

It isn't, precisely because of the noEvent. We would see a step if there was a (recorded) event, so there would be two values a the same time point, one before the event and one after. But there are no events here, so we get the literal result. Which is not well defined whenever the angle is pi + 2*k*pi.

@AHaumer I think comparing angles when stuff rotates more than one turn is always going to give us problems, because of the discontinuous output. I would suggest to change the reference signals and use toSpacePhasor.y instead of toPolar.y. The information is basically the same, but the output of toSpacePhasor.y is continuous and always well-defined, so it won't give us problems.

Do you agree?

[AHaumer]

@casella I agree: For now let us change the comparison signals, I'll prepare a PR - see #4560
However, it has to be possible to obtain proper angles even in case the shaft rotates more than one turn.

In this case, the mechanical angle is not the problem, it is a problem of conversion from rectangular representation of a phasor to polar representation. Enclosed you find a small test example with an alternative formulation of ToPolar (you may switch between original implementation noEvent(... atan2) and an alternative without noEvent using acos (hopefully no division by zero).
The test setup is a phasor with constant angle but length following a ramp from -1 to 1.
Therefore just in the middle the angle has to jump by pi (not producing a ramp!).
Sad news: The ramp when using atan2 depends on the Interval!
We should keep an eye on that.
Polar.zip