modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
472 stars 169 forks source link

Problematic models for regression test #1741

Open modelica-trac-importer opened 7 years ago

modelica-trac-importer commented 7 years ago

Reported by jfrenkel on 30 Jun 2015 08:30 UTC The MSL includes the following models with problems for regression tests. Some of them only needs simple changes but some also seems to be more complicate to solve.


List of problematic models


Modelica.Mechanics.MultiBody.Examples.Systems.RobotR3.oneAxis

Modelica.Mechanics.MultiBody.Examples.Systems.RobotR3.fullRobot

For the initialization of the Model the following equation needs to be solved for axis.gear.bearingFriction.sa:

        a_relfric/unitAngularAcceleration = if locked then 0 else if free then sa
     else if startForward then sa - tau0_max/unitTorque else if 
    startBackward then sa + tau0_max/unitTorque else if pre(mode) ==
    Forward then sa - tau0_max/unitTorque else sa + tau0_max/unitTorque;

The problem is that for initialization locked needs to be true and in this case sa is not incident in the equation. Because of that the solution of sa is not clearly defined and the initial value of axis.motor.La.i, which is an continuous time state, is also not clearly defined.


Modelica.Mechanics.Rotational.Examples.HeatLosses

The model is not robust. During the event iteration at t=0.5304, 0.7126, and 0.918 s the variables oneWayClutch.startForward and oneWayClutch.locked switch between 0 and 1. If one of these discrete states is selected as valid, the results correspond to the reference results. If the other discrete state is chosen, the model sticks at the discontinuity. Dymola prints the message "Fix point iteration did not converge at time : xxx." There might be a problem with Modelica.Mechanics.Rotational.Components.OneWayClutch.


Modelica.Mechanics.MultiBody.Examples.Elementary.DoublePendulumInitTip

The mechanical mechanism is initialized with an absolute position of the pendulum tip. The problem is that therefore two valid solutions for revolute1.phi and revolute2.phi exist. In addition the solution for s1 and s2 can be multiple with

modelica mechanics multibody examples elementary doublependuluminittip_s1 modelica mechanics multibody examples elementary doublependuluminittip_s2 The start values of revolute1.phi and revolute2.phi are both Zero. Because of the multiple valid solutions the model is not suitable for validation.


Modelica.Electrical.Analog.Examples.ChuaCircuit

Chua's circuit is a chaotic model: small differences lead to large changes in the behavior. This behavior can be checked with the variable C1.v, when the model is simulated with varying settings for the tolerances (for example: 1e-4, 1e-5, 1e-6). The simulation results results differ heavily.


Modelica.Electrical.Analog.Examples.AmplifierWithOpAmpDetailed

The results depend heavily from the tolerance settings. Small tolerances lead to wrong behavior (at least in Dymola). opAmp.v_source cannot be larger than its operating voltage (constantVoltage.V). But this is (in Dymola) the case with tolerances smaller than 2e-7 (for example with 1e-7). The reason for that behavior might be an ill conditioned Jacobian matrix, which is indicated (in Dymola) when a tolerance of 1e-8 is selected.


Modelica.StateGraph.Examples.FirstExample_Variant2

The variable transition1.enableFire has at the stopTime an event if the time points of the events are exactly calculated. The problem is that this is not part of the reference result. Hence if a tool exactly calculates the event time points it found an event for transition1.enableFire at the end of the simulation. To avoid this it would be good to change the stopTime to a value which is not an event time point. (For example 5.1s).


Migrated-From: https://trac.modelica.org/Modelica/ticket/1741

modelica-trac-importer commented 7 years ago

Comment by leo.gall on 30 Jun 2015 09:29 UTC Thanks for the analysis. For MSL 3.2.1, it was just pragmatism to use the examples from MSL for regression testing. Problems with comparing tools with different solvers have been expected. This is a tradeoff. Maybe one wants to have some numerically challenging examples.

We should try to improve the examples so that they are likely to work in the same way in all tools.

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 30 Jun 2015 11:24 UTC I agree that we cannot expect perfection yet. As far as I understand these are primarily existing issues that were not resolved in the new release, which should be ok.

However, I don't understand what is meant with 'valid' below (since they are discrete states I thought both should be active states): -- Modelica.Mechanics.Rotational.Examples.HeatLosses

The model is not robust. During the event iteration at t=0.5304, 0.7126, and 0.918 s the variables oneWayClutch.startForward and oneWayClutch.locked switch between 0 and 1. If one of these discrete states is selected as valid, the results correspond to the reference results. If the other discrete state is chosen, the model sticks at the discontinuity. Dymola prints the message "Fix point iteration did not converge at time : xxx." There might be a problem with Modelica.Mechanics.Rotational.Components.OneWayClutch. --

modelica-trac-importer commented 7 years ago

Comment by jfrenkel on 30 Jun 2015 11:50 UTC The 'valid' for the Model Modelica.Mechanics.Rotational.Examples.HeatLosses means that the solver decide to choose at a certain point the solution for oneWayClutch.startForward and oneWayClutch.locked although the event iteration is not finished.

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 10 Jul 2015 10:09 UTC Replying to [comment:3 jfrenkel]:

The 'valid' for the Model Modelica.Mechanics.Rotational.Examples.HeatLosses means that the solver decide to choose at a certain point the solution for oneWayClutch.startForward and oneWayClutch.locked although the event iteration is not finished.

After having examined the problem I would describe in a slightly more complex way (not saying the above is incorrect): The event iterations finally converge at each event point (such that the equations are satisfied and x=pre(x)).

However, during one step of the event iteration the solver uses fixed-point iteration to find a solution for the variables - that does not work so the equations are not fully satisfied at an intermediate point during the event iteration, but then the event iteration is continued and converges.

-- I don't see that can fundamentally change the friction-logic in a new build, and I am unsure if we can improve it (even in a major release). I don't like that (well - I don't like the need for startBackward and startForward either), but it is an old problem and I am basically not sure if there exists a better solution (the current Friction-model updates from 2008 was intended to reduce this problem; it might be that we need a more detailed investigation).

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 10 Jul 2015 10:36 UTC Replying to [ticket:1741 jfrenkel]:

=== Modelica.Electrical.Analog.Examples.ChuaCircuit === Chua's circuit is a chaotic model: small differences lead to large changes in the behavior. This behavior can be checked with the variable C1.v, when the model is simulated with varying settings for the tolerances (for example: 1e-4, 1e-5, 1e-6). The simulation results results differ heavily.

I looked at this and it is problematic for the testing procedure - but not for the library: Chua's circuit is intentionally a chaotic model. The intended use of the example is to demonstrate the chaotic result; thus it seems counter-intuitive to modify the simulation setup to reduce the thing we want to show - just because we want to satisfy some test.

I see some possibilities: have some way of disabling regression testing for specific models, or ideally switch to another way of testing that model (directly comparing the fractal(?) phase-space).

modelica-trac-importer commented 7 years ago

Comment by fcasella on 10 Jul 2015 13:03 UTC For smooth nonlinear chaotic systems such as Chua's circuit, a pragmatic approach for regression testing is to limit the simulation interval so that there is not enough time for trajectories that are slightly different within the error tolerance to diverge significantly. The chances that a tool has a bug that doesn't show up in that interval already are quite small.

In any case, this is much better than a test which is theoretically guaranteed to fail or no testing at all, and much easier than comparing fractal phase-spaces.

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 10 Jul 2015 13:11 UTC Replying to [comment:6 fcasella]:

For smooth nonlinear chaotic systems such as Chua's circuit, a pragmatic approach for regression testing is to limit the simulation interval so that there is not enough time for trajectories that are slightly different within the error tolerance to diverge significantly. The chances that a tool has a bug that doesn't show up in that interval already are quite small.

I agree that it is simpler and is likely to find problems if there are any; especially since I don't know how to implement the "proper" phase-space comparison test.

However, my main point is that this restriction of simulation interval (or stricter tolerance) for testing should not influence users running the example in the library - i.e. we need a special setting to separate the testing-use of the model and the example-use of the model. (Only for special cases such as this one.)

modelica-trac-importer commented 7 years ago

Comment by fcasella on 10 Jul 2015 13:44 UTC Replying to [comment:7 hansolsson]:

However, my main point is that this restriction of simulation interval (or stricter tolerance) for testing should not influence users running the example in the library - i.e. we need a special setting to separate the testing-use of the model and the example-use of the model. (Only for special cases such as this one.)

Absolutely - in fact I wanted to mention that as well in my post.

For this, we can just use Modelica Association vendor-specific annotations that override the standard experiment annotation only for MSL regression testing purposes.

modelica-trac-importer commented 7 years ago

Comment by leo.gall on 10 Jul 2015 14:10 UTC Replying to [comment:7 hansolsson]:

However, my main point is that this restriction of simulation interval (or stricter tolerance) for testing should not influence users running the example in the library - i.e. we need a special setting to separate the testing-use of the model and the example-use of the model. (Only for special cases such as this one.)

Currently, the example has a StopTime=1. Changing the solver tolerance didn't show chaotic behavior (in Dymola).

The info layer of this example recommends to use StopTime=5e4 in order to investigate the chaotic behavior. Using this longer simulation time, I do see severe result deviations when changing solver type or tolerance.

So, Francesco's proposal of a shorter simulation time has already been included in this example.

The original ticket doesn't state a specific simulation time. Therefore I assume differences between Dymola and SimulationX already showed in the first second.

An MA vendor-specifc annotation would be a good approach, if we can find an appropriate setting for multiple tools.

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 10 Jul 2015 14:26 UTC Replying to [comment:9 leo.gall]:

Replying to [comment:7 hansolsson]:

However, my main point is that this restriction of simulation interval (or stricter tolerance) for testing should not influence users running the example in the library - i.e. we need a special setting to separate the testing-use of the model and the example-use of the model. (Only for special cases such as this one.)

Currently, the example has a StopTime=1.

It used to have StopTime=1 - but I downloaded the RC2 to test and it had StopTime=5e4

modelica-trac-importer commented 7 years ago

Comment by dietmarw on 13 Jul 2015 06:20 UTC Replying to [comment:9 leo.gall]:

An MA vendor-specifc annotation would be a good approach, if we can find an appropriate setting for multiple tools.

Although we allow vendor specific annotations in the language the goal has always been to get rid of them in the MSL as it is "the model library". So we should keep that in mind and try really hard to avoid using them here.

modelica-trac-importer commented 7 years ago

Comment by fcasella on 13 Jul 2015 10:39 UTC The requirement of not having vendor-specific annotations in the MSL is motivated by the need of not giving any specific tool some unfair competitive edge, by exploiting tool-specific annotations. I don't think this will be the case here, if the "vendor" is the MA. BTW, I think the choice of the word "vendor" in this context has really be unfortunate, as there are some tool vendors which are not really vendors as they are not selling their tool.

Anyway, I think it is prefereable to embed settings for regression testing in the models, rather than writing them in separate files which are bound to always be out-of-date and out-of-sync.

If the MA is ultimately responsible for such regression test set-up, I don't see anything bad having MA-specific annotations in the MSL.

modelica-trac-importer commented 7 years ago

Comment by jfrenkel on 20 Jul 2015 12:40 UTC The Modell Modelica.Fluid.Examples.Explanatory.MomentumBalanceFittings is also not suitable for regression test. Especially the upper part of the model has two valid solutions for suddenExpansion1.m_flow which is calculated using the function:

Modelica.Fluid.Fittings.BaseClasses.QuadraticTurbulent.pressureLoss_m_flow_totalPressure

The function output is piecewise quadratic with different values of the slope for left and right side. Because of that there are two valid solutions. One right and one left.

With the used start value for the initial State suddenExpansion1.m_flow_start the jacobian with respect to the initial state is zero. Hence a numerical calculated jacobian is used. The solution depends on the sign of the perturbation.

modelica-trac-importer commented 7 years ago

Comment by jfrenkel on 20 Jul 2015 13:49 UTC Modelica.Mechanics.MultiBody.Examples.Systems.RobotR3.fullRobot: If pre(axis1.gear.bearingFriction.mode) == Stuck (= 0) then is w_relfric = der(axis1.motor.speed.flange.phi) near zero. The sign depends from the numeric. Hence, if axis1.gear.bearingFriction.sa < - axis1.gear.bearingFriction.tau0_max then axis1.gear.bearingFriction.startBackward becomes true. If der(axis1.motor.speed.flange.phi) < 0 then axis1.gear.bearingFriction.mode = -1 (Backward) and in the next event step of the same event-iteration axis1.gear.bearingFriction.startBackward becomes false. Otherwise, if der(axis1.motor.speed.flange.phi) > 0 axis1.gear.bearingFriction.mode keeps 0 (Stuck) and axis1.gear.bearingFriction.startBackward keeps true. axis1.gear.bearingFriction.startBackward becomes false in the next event-iteration after at least one time step.

Hence, axis1.gear.bearingFriction.startBackward has a value true in this case in the results but not in the first case. Hence, the value of axis1.gear.bearingFriction.startBackward is not a good value for the test.

modelica-trac-importer commented 7 years ago

Comment by schnabel on 25 Aug 2015 09:15 UTC Modelica.Fluid.Examples.Explanatory.MeasuringTemperature: The test values are computed with tolerance=1e-006, but the values of *Tank*.level and *Tank*.medium.h are different from the values with tolerance=1e-007 in Dymola, especially *TankHot*.medium.h (difference 2 with the absolute difference between the maximum and minimum of the curve smaller than 50). Hence, the test values are not good values for the regression test.

modelica-trac-importer commented 7 years ago

Comment by leo.gall on 25 Aug 2015 10:34 UTC Replying to [comment:15 schnabel@…]:

Modelica.Fluid.Examples.Explanatory.MeasuringTemperature: The test values are computed with tolerance=1e-006, but the values of *Tank*.level and *Tank*.medium.h are different from the values with tolerance=1e-007 in Dymola, especially *TankHot*.medium.h (difference 2 with the absolute difference between the maximum and minimum of the curve smaller than 50). Hence, the test values are not good values for the regression test.

We automatically selected dynamic states as comparison variables. The dynamic state variables are nearly constant (because the tanks are huge compared to mass flow).

I simulated with Dymola 2016 and Dassl tolerance 1e-6 vs. 1e-7: The relative deviation for openTankHot2.medium.h is about 3e-6. In terms of temperature: a deviation of 0.0003 degC! This should be acceptable. As always: if you only consider the value range of a (nearly) steady variable, this might lead to problems.

As this example demonstrates temperature measurements, the following variables would be better suited for comparison: {"T_onePort.T","T_twoPort.T","T_junction.T"} We could consider updating the reference results, accordingly.

modelica-trac-importer commented 7 years ago

Comment by schnabel on 26 Aug 2015 13:40 UTC Replying to [comment:16 leo.gall]: Modelica.Fluid.Examples.Explanatory.MeasuringTemperature: Thank you for your answer.

We compared the values of "T_onePort.T", "T_twoPort.T", "T_junction.T" in SimulationX and Dymola. We found a difference between the value of T_junction.T at the start point time = 0. The reason is, that Dymola use the computation of inStream(T_junction.port.h_outflow)how it is described in the Modelica Language Specification Version 3.3 in chapter 15.2 using a smooth curve with alpha.

Here, this is a special case (one port m_dot==0) which is described in D.3.3. There is a definition of inStream where, depending on the direction of the mass flow, the enthalpy from the source side is used.

It is unclear, which kind of implementation should be used.

modelica-trac-importer commented 7 years ago

Comment by leo.gall on 26 Aug 2015 20:41 UTC Replying to [comment:17 schnabel@…]:

Replying to [comment:16 leo.gall]: Modelica.Fluid.Examples.Explanatory.MeasuringTemperature: Thank you for your answer.

We compared the values of "T_onePort.T", "T_twoPort.T", "T_junction.T" in SimulationX and Dymola. We found a difference between the value of T_junction.T at the start point time = 0. The reason is, that Dymola use the computation of inStream(T_junction.port.h_outflow)how it is described in the Modelica Language Specification Version 3.3 in chapter 15.2 using a smooth curve with alpha.

Here, this is a special case (one port m_dot==0) which is described in D.3.3. There is a definition of inStream where, depending on the direction of the mass flow, the enthalpy from the source side is used.

It is unclear, which kind of implementation should be used.

Thanks for further investigation. Please create a ticket for this issue aiming at clarification of Modelica spec.

modelica-trac-importer commented 7 years ago

Comment by jfrenkel on 27 Aug 2015 16:01 UTC Modelica.Fluid.Examples.ControlledTankSystem.ControlledTanks: The test values are computed with tolerance=1e-006, but the values of tank2.medium.T are different from the values with tolerance=1e-007 in Dymola (difference 0.01 with the absolute difference between the maximum and minimum of the curve smaller than 0.15). Hence, the test values are not good values for the regression test.

modelica-trac-importer commented 7 years ago

Comment by schnabel on 7 Sep 2015 08:09 UTC Modelica.Fluid.Examples.Tanks.EmptyTanks: In creation.txt:

Tolerance=1e-006 // used default, because no tolerance annotation in model

but annotation in simulateModel

  tolerance=1e-007,

The comparison results are computed with 1e-6. Moreover, the value of tank1.medium.T starts at time = 0 with 293.15 K = 20 °C and ends at time = 50 with 293.144 K. This is a difference of 0.006 K. But the difference of tank1.medium.T at time = 50 between the tolerances 1e-6, 1e-7, 1e-8, and 1e-9 is more than 1e-5 K and with tolerance = 1e-10 the computation fails at time = 35.4 s, see:

EmptyTanks - Modelica Fluid Examples Tanks

modelica-trac-importer commented 7 years ago

Comment by leo.gall on 7 Sep 2015 12:16 UTC Replying to [comment:20 schnabel@…]:

Modelica.Fluid.Examples.Tanks.EmptyTanks: In creation.txt:

Tolerance=1e-006 // used default, because no tolerance annotation in model

but annotation in simulateModel

  tolerance=1e-007,

Could you please explain this comment in more detail? What is "annotation in simulateModel"? Thanks!

modelica-trac-importer commented 7 years ago

Comment by leo.gall on 7 Sep 2015 12:18 UTC This ticket should be discussed at the next MAP-LIB meeting.

modelica-trac-importer commented 7 years ago

Comment by schnabel on 7 Sep 2015 14:39 UTC Replying to [comment:21 leo.gall]:

Replying to [comment:20 schnabel@…]:

Modelica.Fluid.Examples.Tanks.EmptyTanks: In creation.txt: {{{ Tolerance=1e-006 // used default, because no tolerance annotation in model }}} but annotation in simulateModel {{{ tolerance=1e-007, }}}

Could you please explain this comment in more detail? What is "annotation in simulateModel"? Thanks!

See the row 49 and compare it with the row 21 in creation.txt.

modelica-trac-importer commented 7 years ago

Comment by henrikt on 17 Sep 2015 06:23 UTC Regarding DoublePendulumInitTip, setting a non-fixed start value for revolute2.phi to 90° should be enough. Then, the model will also test the initialization procedure's ability to take guess values into account.

To elaborate, non-fixed start values may serve two purposes:

  1. Help the initialization procedure to find the solution, when the initialization problem has a unique solution.
  2. Give the user control over which solution to pick, when the initialization problem has multiple solutions. If the intention of the spec is to support both, then the suggested change to DoublePendulumInitTip makes sense. If the purpose is only to support 1. (thereby avoiding to advocate modeling with non-unique behavior), then the model is truly problematic for regression testing.
modelica-trac-importer commented 7 years ago

Comment by frenkel on 22 Sep 2015 12:49 UTC Modelica.Mechanics.Rotational.Examples.LossyGearDemo2: There are two parallel rigid frictions in the model, one in bearingFriction and another in gear. At time = 0.3083 s bearingFriction.mode and gear.mode change from 1 to 0. Hence, the system becomes singular, i.e., there is not a unique solution for this system, especially for bearingFriction.tau = bearingFriction.sa and gear.tauLoss = gear.sa. Hence, also the discrete state of the system is not unique and this is not good for the regression test.

modelica-trac-importer commented 7 years ago

Comment by ahaumer on 24 Sep 2015 10:42 UTC Chaotic / ambiguous models: Fine for demo examples. Maybe exclude them (using an annotation: Language group!) from regression tests. Try to resolve at least some of them by better start values.

modelica-trac-importer commented 7 years ago

Comment by dietmarw on 2 Oct 2015 20:51 UTC Ticket retargeted after milestone closed

modelica-trac-importer commented 7 years ago

Comment by efredriksson on 14 Oct 2015 07:19 UTC Modelica.StateGraph.Examples.ExecutionPaths seems to have the same problem as Modelica.StateGraph.Examples.FirstExample_Variant2 (mentioned in ticket).

modelica-trac-importer commented 7 years ago

Modified by leo.gall on 13 Jan 2016 15:54 UTC

modelica-trac-importer commented 7 years ago

Comment by leo.gall on 26 Jan 2016 08:28 UTC

modelica-trac-importer commented 7 years ago

Comment by henrikt on 10 Jan 2017 08:40 UTC Regarding the ChuaCircuit example, the reference result for MSL 3.2.1 build 2 (tags/v3.2.1+build.2.release/Modelica/Electrical/Analog/Examples/ChuaCircuit) had a duration of 1 second. Back then, the situation was still manageable, and we could discuss the problem of how to handle example with chaotic dynamics in general for testing purposes.

This situation changed with MSL 3.2.1 build 4 (tags/v3.2.1+build.4/Modelica/Electrical/Analog/Examples/ChuaCircuit) where the duration was changed to 5e4. Since then, we have a concrete case with a reference result that is impossible to reproduce within any tolerance.

The change in the example itself is one thing, and I think 5e4 is a good setting there. The problem is when the reference results are generated with that duration. If there was an experiment(__MA_TestStopTime = 1.0), then this could be used when generating the reference results, and be reflected in the creation.txt so that tool vendors wouldn't even need to bother with the __MA_TestStopTime in order to run simulations with the same duration as the reference result.

GallLeo commented 7 years ago

Henrik, I agree we should improve this for the next release. But, just one attribute __MA_TestStopTime might not be flexible enough. It would be nice, if we optionally could specify a stricter tolerance or more result points. While this might not be relevant for chaotic models, we already change tolerance and interval when generating reference results. It would be nice, if the model developer could specify a "strict" setting, to be used for regression testing.

Proposal:

What do you think?

henrikt-ma commented 7 years ago

When discussing models that are problematic for regression testing, adding features to support having examples that depend on a particular integrator seems counter-productive to me.

On the contrary, based on experiments with CVODES, I suspect that we have several models where the reference result contains artifacts of the particular integrator used to produce the results, and where the only way of producing results that match within a nice tolerance is to use the same integrator; with another integrator that computes more accurate results, one then has to increase tolerances in order to match the reference result.

I'm not saying that CVODES consistently computes more accurate results, just that I suspect that it sometimes does, so it is a hard work to differentiate between:

Even though this is hard work, I think it is better to admit that this type of problem exists, rather than hiding it by specifying what integrator one has to use to have any chance of reproducing the reference result.

beutlich commented 7 years ago

See also #2350 for Modelica.Electrical.Analog.Examples.AD_DA_conversion.

GallLeo commented 6 years ago

I follow @henrikt-ma's arguments, but for automatic cross-tool comparison, it would be nice to have a documentation "how to get the same results, most likely".

We could also discuss switching to Cvode instead of Dassl for creating the reference results. This might be a better basis for reproducing the results.

At least, we should go through this ticket at a MAP-LIB meeting. Especially, regarding annotation __MA_testExperiment(). Do we want it for problematic/chaotic models, or not?

I don't think we can fix these models before release of MSL 3.2.3.

henrikt-ma commented 5 years ago

For the problem with RobotR3.oneAxis and fullRobot, I've opened a separate issue to get a more focused discussion: #2941

henrikt-ma commented 5 years ago

Regarding AmplifierWithOpAmpDetailed, I'd be curious to hear more about that numeric ill-conditioning, as it's not something I'm able to reproduce.

Would it make sense to open a separate issue for the problems with this example, similar to what I did for RobotR3?

henrikt-ma commented 5 years ago

Never mind, there is already #1131 for AmplifierWithOpAmpDetailed. However, the last comment on that dates 2017-01-14, which pre-dates this issue. Is it this issue or #1131 that should be updated? For what it's worth, I still find the example extremely troublesome.