Reference result for Modelica.Electrical.Analog.Examples.AD_DA_conversion lacks periodicity

[qlambert-pro]

The example represents a model of a conversion of a sine wave from analog to digital and back to analog. Since the period of the sine wave is a multiple of the sampling period used for the AD conversion, I believe the output of the conversion should be periodic and have the same period as the sine wave. This property should also be shared by the output of the DA conversion.

The reference file, however, does not display it.

The values of several variables between time 0.15 and 0.151 differs from their values between time 0.05 and 0.051. Affected variables include: aD_Converter.y, dA_Converter.vout and dA_Converter.y.

[HansOlsson]

Where is this reference result? I could not see anything odd when running the model.

[qlambert-pro]

Here is the reference, I downloaded it from "modelica.org:/files/RegressionTesting/ReferenceResults/MSL" using the instructions available at: https://svn.modelica.org/projects/RegressionTesting/AccessInfo/read-access/README.html This is the v3.2.2+build.0-beta.3 version of the baseline: AD_DA_conversion.zip

[beutlich]

@HansOlsson This either looks like a tool issue (independent of #2282, i.e. doublePrecision=true) or the signals are not suitable for comparison.

At time 0.05 and 0.15 dA_Converter.y changes discontinuously to slightly different values in Dymola 2018:

0.05,67
0.05,64
...
0.15,67
0.15,63

In comparison MWorks.Sysplorer 2017:

0.05,67
0.05,63
...
0.15,67
0.15,63

and SimulationX 3.8.4:

0.05,67
0.05,64
...
0.15,67
0.15,64

AD_DA_conversion.zip

[HansOlsson]

I cannot reproduce this in Dymola 2018.

The model is, however, very sensitive and here value to digitize is 5.0/10.0 and any slight round-off error in 5.0 can push it below the discretization limit causing the result to be 63 - or above that limit causing it to be 64.

Thus this signal is not suitable for comparison at this accuracy at this point; both 63 and 64 are acceptable answers, and this round-off can occur differently for different times.

[sjoelund]

Should this really have been closed before we have an updated reference result?

[beutlich]

@sjoelund You mean by removing aD_Converter.y, dA_Converter.vout and dA_Converter.y from the reference results?

[sjoelund]

Yes, if as @HansOlsson says those signals are not good for comparison, they should be removed from the reference file so people don't get more false negatives from comparing those results.

[HansOlsson]

Yes, if as @HansOlsson says those signals are not good for comparison, they should be removed from the reference file so people don't get more false negatives from comparing those results.

Well, the signal is generally ok so removing it altogether seems excessive - it's just two integer values that can be one of two nearby values. I don't know exactly how detailed the filtering of references can be.

If it were a pure test-example we could shift time (shiftVoltage.phase=...) to avoid this issue - but for a an Example-model that would look odd.

[tidde]

I fully agree with @sjoelund that this cannot be closed without first removing the signal from the reference result (or that it is solved by shifting the phase in the model along with an updated reference result). Removing it from the list of comparison signals is not the same as somehow removing the variable from the example.

[henrikt-ma]

Sorry, that last comment was by me, signed in to the wrong account.

[HansOlsson]

I fully agree with @sjoelund that this cannot be closed without first removing the signal from the reference result (or that it is solved by shifting the phase in the model along with an updated reference result).

True, but there are two reference results: the mat-file being the result of simulation, and the csv-result used for regression testing by e.g. @GallLeo - this signal should at most only be removed in the latter; and I don't think that those results are on GitHub yet (planned to be on git-lfs I think).

I am not sure if you can just un-tighten the tolerance for a specific variable instead.

[sjoelund]

Well, even if the results are only in rsync, we should at least fix those files. Perhaps in a new build of MSL, perhaps in the current location. It's mainly comparisonSignals.txt that should be updated and then the csv-file (removing a column or several; ideally done by the library officer)

[GallLeo]

Removed aD_Converter.y, dA_Converter.vout and dA_Converter.y from the list of comparison signals.

Would be better to be able to define a specific tolerance per variable for comparison.

Possible workaround: Add a low-pass-filter, so that we have a continuous variable for comparison. This should also be able to check if the AD-DA chain works as expected.

Model:

model AD_DA_conversion
  extends Modelica.Electrical.Analog.Examples.AD_DA_conversion;

  Modelica.Electrical.Analog.Sensors.VoltageSensor voltageSensor annotation (Placement(transformation(
        extent={{10,-10},{-10,10}},
        rotation=90,
        origin={72,2})));
  Modelica.Blocks.Continuous.LowpassButterworth lowpassButterworth(f=100) annotation (Placement(transformation(extent={{90,-8},{110,12}})));

equation 
  connect(voltageSensor.v, lowpassButterworth.u) annotation (Line(points={{83,2},{88,2}}, color={0,0,127}));
  connect(dA_Converter.p, voltageSensor.p) annotation (Line(points={{44,16},{72,16},{72,12}}, color={0,0,255}));
  connect(dA_Converter.n, voltageSensor.n) annotation (Line(points={{44,-10},{72,-10},{72,-8}}, color={0,0,255}));

  annotation (experiment(StopTime=0.2), uses(Modelica(version="3.2.3")));

end AD_DA_conversion;

Results:

@kristinmajetta what do you think of this idea?

Ticket should be closed after finding better comparison variable.

[henrikt-ma]

Would be better to be able to define a specific tolerance per variable for comparison.

Setting comparison tolerances seems like a completely separate question. In our tests, we do have this possibility, allowing us to work around problems like this without having to exclude the variable, but I doubt that it would be worth the effort to standardize a system for specifying this.

Instead, I want the reference results to be as accurate and reliable as possible, so that we can leave it to all tool vendors to choose a test tolerance that is matched to the solver and solver settings they use for their tests.

[beutlich]

@kristinmajetta what do you think of this idea?

Reassigning and rescheduling milestone.

[kristinmajetta]

yes, let's do so