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
479 stars 169 forks source link

IdealizedOpAmpLimted has issues #2270

Closed JoeRiel closed 6 years ago

JoeRiel commented 7 years ago

The new part Modelica.Electrical.Analog.Ideal.IdealizedOpAmpLimted has a few issues.

A minor issue is that the name has a typo; Limted should be Limited.

A serious bug is that the external supply pins, s_p and s_n, which are enabled when the boolean input useSupply is true, do not work because the equation that controls the output is in terms of Vps and Vns, the parameters, rather than vps and vns, the variables that should have been used.

From a performance viewpoint, the use of smooth(0, min(vps, max(vns, V0*v_in))) to generate the output, while convenient, is not ideal. The smooth(0, .) asserts that the output is continuous. That is not necessarily the case if v_in is not continuous, which can happen if the input is derived from a step signal or the circuit being modeled employs positive feedback. The bigger issue is that the use of smooth with min and max precludes the creation of events which would otherwise improve the engine performance at the transition to limiting. In a typical circuit where the external gain, set by the feedback, is much smaller than the open-loop gain (V0), the change in slope of the controlled signal as the circuit transitions to and from limiting is a few orders of magnitude. Without an event this can be problematic for some integrators to maintain error control. A better solution is to use a condition:

v_out = if AtNegRail then vns elseif AtPosRail then vps else V0*v_in;

where AtNegRail and AtPosRail are boolean variables defined as

Boolean AtNegRail = V0*v_in < vns "True when output is at the negative rail"; Boolean AtPosRail = V0*v_in > vps "True when output is at the positive rail";

The inequalities can be put directly into the conditions of the if-expression, however, declaring them as probeable variables provides a convenient means to detect that the opamp is at the rails, a normally undesirable condition.

HansOlsson commented 7 years ago

The smooth(0, .) asserts that the output is continuous. That is not necessarily the case if v_in is not continuous, which can happen if the input is derived from a step signal or the circuit being modeled employs positive feedback.

Actually smooth does not state that - it only states that the expression within smooth is continuous. The Modelica specification has this as a comment: "Note that smooth does not guarantee a smooth output if any of the occurring variables change discontinuously"

The bigger issue is that the use of smooth with min and max precludes the creation of events which would otherwise improve the engine performance at the transition to limiting.

That is a problem with min/max, and to avoid that we could use: v_out = smooth(0, if V0*v_in<vns then vns else if V0*v_in>vps then vps else V0*v_in); which a tool should be able to handle as efficiently as before (or better). (Obviously that should be tested, but that is the goal of smooth.)

We might even add a protected variable for V0*v_in.

However, adding:

 Boolean AtNegRail = V0*v_in < vns "True when output is at the negative rail"; 
 Boolean AtPosRail = V0*v_in > vps "True when output is at the positive rail"; 

will force the creation of events, and it is not clear that it is always desired. Having that as an option might be possible, but complicates the code and it might be simpler to have it as another model.

beutlich commented 7 years ago

See also #2017.

beutlich commented 7 years ago

The naming typo will be dealt with by a separate issue.

HansOlsson commented 6 years ago

There seems to be an issue with v_out = smooth(0, if V0*v_in<vns then vns else if V0*v_in>vps then vps else V0*v_in); specifically that it causes Modelica.Electrical.Analog.Examples.OpAmps.Multivibrator to fail to simulate when using events.

Basically if we have events then v_out can increase 15,000 fold (due to V0) and that seems even more problematic for integrators than stopping for an event. We could either go back to the original with min/max or use noEvent to make it clear that we are not generating events.

Obviously we would then be stuck with the original issue that it is difficult to maintain error control if the equations aren't continuously differentiable; but new simulation failures aren't good either.

beutlich commented 6 years ago

Checked again: Modelica.Electrical.Analog.Examples.OpAmps.Multivibrator both succeeds in OMC and SimulationX. Not sure, what to do here.

HansOlsson commented 6 years ago

@beutlich Do OMC/SimulationX generate events in this case? Since smooth allows a tool to skip events it isn't wrong to skip the events - even without noEvent. (A simple test would be to remove the smooth(0, -part.)

dietmarw commented 6 years ago

No events in OMC.

beutlich commented 6 years ago

With events for SimulationX. Succeeds with BDF solver (DAE solver), but failing to terminate with SimulationX CVODE solver (ODE solver) at time after first event.

beutlich commented 6 years ago

A simple test would be to remove the smooth(0, -part.)

Does not seem to help: ERROR: Finding consistent restart conditions failed at time: 0.03448226997836783

HansOlsson commented 6 years ago

Leave a comment

A simple test would be to remove the smooth(0, -part.) Does not seem to help: ERROR: Finding consistent restart conditions failed at time: 0.03448226997836783

I meant "test" to see if the model works with events; so it wasn't proposed as a solution. So the solution for avoiding the regression is either to get back to the original min/max or use smooth(0, noEvent(if ...) - but then we haven't resolved the issue in this ticket.

beutlich commented 6 years ago

As already asked in https://github.com/modelica/ModelicaStandardLibrary/issues/2017#issuecomment-289688270 and https://github.com/modelica/ModelicaStandardLibrary/pull/2561#issuecomment-393414145. Why cannot we reuse the Modelica.Blocks.Nonlinear.Limiter class in IdealizedOpAmpLimted with the strict (and future homotopyKind - once the PR gets reviewed by @MartinOtter or @AHaumer) parameters and adopt the examples accordingly?