Closed casella closed 4 years ago
Refs #3472
I will take care of this issue later today
Another possibility would be to redesign the comparison tool to have some "absolute tolerance" for these variables, as just removing them from comparison mean that we accept any value for them, even a linear trend going up to 1e10 or so. (Obviously that tolerance could be used to reduce the reference trajectory to basically 0 if that reduces storage requirements.)
Another possibility would be to redesign the comparison tool to have some "absolute tolerance" for these variables, as just removing them from comparison mean that we accept any value for them, even a linear trend going up to 1e10 or so. (Obviously that tolerance could be used to reduce the reference trajectory to basically 0 if that reduces storage requirements.)
I fully agree with @HansOlsson that from a quality assurance point of view it makes more sense to specify an absolute tolerance for the zero sequence components of the electric machine simulation examples (and possibly some other variables of other models, too). If we delete the zero sequence components from the list of reference signals we are not able to catch drift errors of the zero sequence components. I understand that this needs a change
comparisonSignals.txt
stored in the repository directory Resources/Reference/Modelica/...
I think such change may not happen before releasing MSL 4.0.0. So I guess we have to handle the issues caused by the zero sequence components manually. However, I briefly checking additional machine examples which are not listed above and found that some simulation models do not compare the zero sequence component at all. I will summarize the affected variables and machine examples.
After re-thinking this issue I see an alternative which does not require to check the zero sequence currents with an absolute tolerance:
aimc.is[1]
, to the reference listEach phase current does include the zero sequence component. Therefore, checking at least one phase current should inherently reveal significant drift errors, in case they occur. Up to now no phase currents are not included in the list of reference signals.
@AHaumer do you agree?
Refs #3472
Sorry @christiankral, I remembered seeing some notification about this issue, but I couldn't figure out where it was. Thanks for reporting.
I also agree that in principle we should check that the zero component is indeed close to zero. Unfortunately we can't do that with the current specification. @christiankral, feel free to open an issue about this on the ModelicaLanguage tracker.
I also agree that in principle we should check that the zero component is indeed close to zero. Unfortunately we can't do that with the current specification. @christiankral, feel free to open an issue about this on the ModelicaLanguage tracker.
Can you please stop spreading false statements about things missing in the specification?
That is not a language issue, it is an issue for the tool checking the simulation results.
In the Modelica Language "close to zero" is already defined, using the nominal attribute for variables (since Modelica 1.2) and the unbounded attribute (since Modelica 3.4).
The following is already recommended for tolerance checking: tol*(abs(nominal)+(ifx.unbounded then 0 else abs(x)))
- but I haven't checked if the comparison tool uses that (and I don't think it has access to the nominal values in the model).
When investigating Modelica.Electrical.Machines.Examples.InductionMachines.IMC_YD
I found that the peak value of the zero sequence current aimc.i_0_s
becomes
I could unfortunately not create a simulation results with OpenModelica for comparison reasons as the simulation run was not completed... Other electric machine simulation examples show a zero sequence current of less than 1E-10 A. So the zero sequence currents may vary on a very different magnitude of order. As the zero sequence current shall be ideally zero, one could only empirically determine the absolute tolerance for each simulation example individually. And possibly the result would vary depending on the used simulation tool and numerical solver.
Even though the choice of a nominal value may improve the situation, the original dilemma remains: picking a reasonable order of magnitude of the absolute tolerance.
In order to cover the numerical impact of the zero sequence currents of electric machine examples, I rather tend to additionally check the phase currents instead of the zero sequence currents. In both cases a significant drift of the zero sequence current would be caught. However, when checking the phase currents instead of the zero sequence current, in relative relationship between zero sequence and (nominal) phase current are inherently covered.
Please don't remove signals from comparison just because they appear as numeric noise around zero (or some other constant). These are still useful for our testing of SystemModeler, as we aren't affected by the shortcomings of the CSV compare tool.
Thanks,
Henrik
On 2020-03-04, at 15:49, Christian Kral notifications@github.com wrote:
When investigating Modelica.Electrical.Machines.Examples.InductionMachines.IMC_YD I found that the peak value of the zero sequence current aimc.i_0_s becomes
greater than 1E-6 A using the standard tolerance 1E-6 (in Dymola 2020) greater than 1E-7 A using the standard tolerance 1E-7 (in Dymola 2020) I could unfortunately not create a simulation results with OpenModelica for comparison reasons as the simulation run was not completed... Other electric machine simulation examples show a zero sequence current of less than 1E-10 A. So the zero sequence currents may vary on a very different magnitude of order. As the zero sequence current shall be ideally zero, one could only empirically determine the absolute tolerance for each simulation example individually. And possibly the result would vary depending on the used simulation tool and numerical solver.
Even though the choice of a nominal value may improve the situation, the original dilemma remains: picking a reasonable order of magnitude of the absolute tolerance.
In order to cover the numerical impact of the zero sequence currents of electric machine examples, I rather tend to additionally check the phase currents instead of the zero sequence currents. In both cases a significant drift of the zero sequence current would be caught. However, when checking the phase currents instead of the zero sequence current, in relative relationship between zero sequence and (nominal) phase current are inherently covered.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/modelica/ModelicaStandardLibrary/issues/3476?email_source=notifications&email_token=AGA7KPYW56IGD6WOMOFA2XDRFZS77A5CNFSM4LAA2WZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENYG2XY#issuecomment-594570591, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGA7KPYFFZGBOGBHDIGGW7DRFZS77ANCNFSM4LAA2WZQ.
My investigations showed, that the actual zero sequence current components of different simulations models of the MSL vary by the factor 1E5. To pick the "correct" absolute tolerance is a non-deterministic and thus very empirical process. Additionally, different versions of different simulations tools may over and over again cause different results which may not be compared in a deterministic way.
However, to me it makes more sense to replace the zero sequence currents by the phase currents as the zero sequence current components are always included in the phase currents. As long as the zero sequence currents remains low compared to the phase currents the comparison of the phase currents does not cause a regression and validates the simulation results. To me this is the more practical way to handle and cover the zeros sequence current components.
@AHaumer Do you have a comment here?
See #3472 full ack with @christiankral IMHO zero sequence currents have to be removed frm the reference results, and compare phase currents instead.
I agree @henrikt-ma that we shouldn't just remove reference results since the comparison cannot handle small changes around zero.
Since we cannot quickly redesign the comparison tool I could see two possible solutions:
I understand that other signals should catch the problems without these reference signals, but sometimes things break in unexpected ways.
I'd remove the zero sequence currents permanently, except in rare cases where they aren't supposed to be zero or near zero. Otherwise we'll face the sane problems afain and again.
Is it allowed to include comments in comparisonSignals.txt? We could comment out the zero sequence currents.
Is it allowed to include comments in comparisonSignals.txt?
File comparisonSignals.txt is meant to contain the comparison signals.
ok for now, but for the future: // comments could be usefull ...
ok for now, but for the future: // comments could be usefull ...
Done, see https://github.com/modelica-tools/msl-release/pull/3.
Folks,
I'm afraid we still have this problem in MSL 3.2.3 for the following test cases:
Modelica.Electrical.Machines.Examples.AsynchronousInductionMachines.AIMS_Start
i_0_r
Modelica.Magnetic.FundamentalWave.Examples.BasicMachines.AIMC_DOL
i_0_s, i0
Modelica.Magnetic.FundamentalWave.Examples.BasicMachines.AIMC_Transformer
i0
Modelica.Magnetic.FundamentalWave.Examples.BasicMachines.AIMC_YD
i0
Modelica.Magnetic.FundamentalWave.Examples.BasicMachines.AIMC_withLosses
i0
Modelica.Magnetic.FundamentalWave.Examples.BasicMachines.SMPM_Braking
i0
Modelica.Magnetic.QuasiStatic.FundamentalWave.Examples.BasicMachines.InductionMachines.IMC_Conveyor
i0
Modelica.Magnetic.QuasiStatic.FundamentalWave.Examples.BasicMachines.InductionMachines.IMC_Initialize
i0
Modelica.Magnetic.QuasiStatic.FundamentalWave.Examples.BasicMachines.InductionMachines.IMC_Transformer
i0
Modelica.Magnetic.QuasiStatic.FundamentalWave.Examples.BasicMachines.InductionMachines.IMC_YD
i0
@AHaumer, @christiankral, @beutlich, I guess the problem is that this change was applied to the 4.0.0 reference files (I don't see this problem in our MSL 4.0.0 reports) but not to the 3.2.3 maintenance reference files, which was the original intent of this ticket, as it is clear from the title 😃
We should apply #3497 also to the maint/3.2.3 branch. @AHaumer, @christiankral, can you take care of that or shall I do it?
I guess we should also re-generate the reference files for MSL 3.2.3 accordingly. @beutlich could you take care of that? I could also run those examples in Dymola myself and only upload those files to GIT, but I guess re-running the scripts is much easier.
Please let me know how we can proceed.
Created #3587 for it.
We are testing the results obtained on MSL 3.2.3 models with StopTime annotation in OpenModelica, comparing the results to the reference files produced by MAP-LIB using the CSV compare tool.
In a number of electrical machine tests, we get verification failures because of a individual variables, named either
zeroInductor.i0
ori_0_s,
that should probably be zero if an exact solution of the DAEs was computed, but instead fluctuate randomly close to zero due to the inevitable numerical noise of numerical integrators. Since the numerical noise is very close to zero, high and low limits based on relative tolerances end up being very close to the noisy trajectory, which is basically impossible to replicate unless one uses exactly the same tool and the very same solver settings that were used to generate the reference result.This means that it makes no sense at all to include these variables in the reference file.
Here is a list of models and variables that show this issue:
These results are basically impossible to replicate with the accuracy expected by the CVS compare tool. This doesn't mean that the simulation is wrong, it just means that those reference variables should really not be checked, because they are structurally unreliable.
@christiankral , @AHaumer, can you please confirm that those variables should indeed be zero in an exact solution?
@beutlich, @GallLeo , I would recommend to remove them at once from the reference results of MSL 3.2.3. Please also make sure the same problem is not present in the MSL 4.0.0 reference files.