Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSL 4.1.0 Regressions: OpAmps/SignalGenerator #4485

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AHaumer
Copy link
Contributor

@AHaumer AHaumer commented Oct 21, 2024

Modelica/Electrical/Analog/Examples/OpAmps/SignalGenerator.mo set {opAmp1, opAmp2}.strict=true
see #4333

…nalGenerator.mo set {opAmp1, opAmp2}.strict=true
@AHaumer AHaumer added this to the MSL4.1.0 milestone Oct 21, 2024
@AHaumer AHaumer added bug Critical/severe issue L: Electrical.Analog Issue addresses Modelica.Electrical.Analog example Issue only addresses example(s) labels Oct 21, 2024
@AHaumer AHaumer requested a review from maltelenz October 21, 2024 16:27
Copy link
Contributor

@maltelenz maltelenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately System Modeler has unrelated issues with this model, and therefore I cannot comment on the effect of this change.

@HansOlsson
Copy link
Contributor

Unfortunately System Modeler has unrelated issues with this model, and therefore I cannot comment on the effect of this change.

Just to be sure - how do you know that those issues are unrelated?

The limits combined with the (optional) events are creating non-linear systems that can be difficult to solve, which can cause various problems that may seem unrelated.

@maltelenz
Copy link
Contributor

Just to be sure - how do you know that those issues are unrelated?

The limits combined with the (optional) events are creating non-linear systems that can be difficult to solve, which can cause various problems that may seem unrelated.

Thanks for pushing back, I just trusted what our internal issue tracker said was true, which may no longer be the case.

What I'm observing now, is that this PR makes no difference for System Modeler that I can see.

Our results (which come after a huge amount of warnings from nonlinear system solving) are similar to the current reference, but "delayed". Example variable:

image

If I decrease the interval size aggressively to 0.00001, our results match up with the reference, except System Modeler does not generate any events.

@AHaumer
Copy link
Contributor Author

AHaumer commented Oct 22, 2024

Thanks for your feedback @maltelenz
As far as I see the time drift should be avoided by "strict = true". Here you have a comparison between the rectangle out of the circuit and the analytic solution:
SignalGenerator
The deviation is caused by the finite amplification of the opAmps.
opAmp2 is an integrator coupled with opAmp1 which switches between positive and negative supply.
You might try the enclosed example. I could also include the analytic solution in the PR if desired.
SignalGenerator.zip
Analytic solution:
f = 10 Hz
Vps = -Vns = 15 V
VAmp = 10 V desired amplitude of rectangle
gain of integrator = 4 f VAmp/Vps

@maltelenz
Copy link
Contributor

Digging a bit further, our heuristics for when events should be generated inside smooth, already puts us in the strict case (no events), which explains why we don't see a difference from this PR.

Maybe strict is a misleading name here, since it means no events, and therefore step-length-dependent results?

@AHaumer
Copy link
Contributor Author

AHaumer commented Oct 22, 2024

Now I am confused. Simulation with OM also reveals a time drift between the rectangle out of the circuit and the analytic solution.
SG
This example is just a simple circuit for students of electrical engineering.
I have to admit: Originally I implemented the IdealizedOpAmpLimited, but just a naked version without homotopy and noEvent, as well as the examples. I do not know who implemented the improvements, and I have some doubts that we are trapped in those improvements ...

@maltelenz
Copy link
Contributor

The more advanced handling with strict was introduced in https://github.com/modelica/ModelicaStandardLibrary/pull/2561/files#diff-de34e2dc37ee35839e04f1a11a6f88260080407e14525f85d4839e06eca53b57
with the corresponding long discussion: #2561

The homotopy was introduced in 9d5bc99 referring to #2017 and #2270

@AHaumer AHaumer marked this pull request as draft October 22, 2024 15:27
@AHaumer
Copy link
Contributor Author

AHaumer commented Oct 22, 2024

I converted this to draft since it needs more investigation, and I'm afraid that there should be changes applied to the IdealizedOpAmpLimited - for sure not for that release.
If we omit the homotopy we read:
if strict then
v_out = smooth(0, noEvent(if V0*v_in>vps then vps else if V0*v_in<vns then vns else V0*v_in));
else
v_out = smooth(0, if V0*v_in>vps then vps else if V0*v_in<vns then vns else V0*v_in);
end if;
"smooth should be used instead of noEvent in order to avoid events for efficiency reasons. A tool
is free to not generate events
for expressions inside smooth. However, smooth does not guarantee
that no events will be generated, and thus it can be necessary to use noEvent inside smooth."
The result depends on event generation. As far as I see event generation is necessary to provide a result without time drift.
So this example with this implementation of the IdealizedOpAmpLimited is a bad example for comparison between tools.
@casella should we remove the example from MSL until this issue is clarified?

@HansOlsson
Copy link
Contributor

I converted this to draft since it needs more investigation, and I'm afraid that there should be changes applied to the IdealizedOpAmpLimited - for sure not for that release. If we omit the homotopy we read: if strict then v_out = smooth(0, noEvent(if V0*v_in>vps then vps else if V0*v_in<vns then vns else V0*v_in)); else v_out = smooth(0, if V0*v_in>vps then vps else if V0*v_in<vns then vns else V0*v_in); end if; "smooth should be used instead of noEvent in order to avoid events for efficiency reasons. A tool is free to not generate events for expressions inside smooth. However, smooth does not guarantee that no events will be generated, and thus it can be necessary to use noEvent inside smooth." The result depends on event generation. As far as I see event generation is necessary to provide a result without time drift. So this example with this implementation of the IdealizedOpAmpLimited is a bad example for comparison between tools. @casella should we remove the example from MSL until this issue is clarified?

To me this indicates that we should have the opposite of strict as an option for OpAmp (one that always generates event), and update the example to use that new flag. Whether it should be for this release or the next isn't clear, I can see arguments both ways:

  • We are passed the dead-line
  • But the example not working shows that the underlying model isn't working; it's not just an issue in the Example

@AHaumer
Copy link
Contributor Author

AHaumer commented Oct 23, 2024

Yes @HansOlsson it is an issue in the IdealizedOpAmpLimited, and this needs some more investigation.

There are 3 groups of examples:

  1. The OpAmps-Examples {NonInvertingAmplifier, InvertingAmplifier, DifferentialAmplifier, Adder, Subtracter, Differentiator, Integrator, LowPass, HighPass, ControlCircuit, VoltageFollower, LCOscillator} are parameterized so that the operational amplifiers are used in the linear range without output saturation (only the Differentiator limits the output voltage).
    All these examples use homotopyType = NoHomotopy and strict = true.
  2. The OpAmps-Examples {Comparator, InvertingSchmittTrigger, SchmittTrigger} are designed such way that the output voltage switches between positive and negative supply, i.e. the output is (nearly) all the time saturated.
    All these examples use homotopyType = NoHomotopy and strict = true.
  3. The critical OpAmps-Examples {Multivibrator, SignalGenerator} are also designed such way that the output voltage switches between positive and negative supply, i.e. the output is (nearly) all the time saturated.
    SignalGenerator: The exact time when the output of opAmp1 gets saturated is of importance, since opAmp2 integrates the output of opAmp1 and its output is fed back to opAmp1 causing it to switch.
    Multivibrator uses homotopyType = LimiterHomotopy.LowerLimit and strict = true,
    SignalGenerator uses homotopyType = LimiterHomotopy.UpperLimit and strict = false.

The parameter strict = true performs saturation of the output with noEvent, i.e. the exact time when the output gets saturated is not properly defined. That is ok for the first group of examples.
The second and the third group of examples should be used with strict = false.
“smooth should be used instead of noEvent in order to avoid events for efficiency reasons. A tool is free to not generate events for expressions inside smooth. However, smooth does not guarantee that no events will be generated, and thus it can be necessary to use noEvent inside smooth.”
Therefore the usage of smooth is a bad idea for the second and third group of examples, since the result is dependent on the tool (whether or not events are generated).

Enclosed you'll find a small library with an implementation of the OpAmp which allows to choose smooth and strict (noEvent) separately. Feel free to test different parameter combinations and give feedback. With Dymola and OM I can test myself, @maltelenz your test with SystemModeler is highly appreciated.
OpAmps.zip

@maltelenz
Copy link
Contributor

I tested the SignalGenerator example from the above zip file, in System Modeler, using this test library:

package OpAmpSignalGeneratorTest
  model Base
  extends OpAmps.SignalGenerator;
    annotation(
      Documentation(figures = {
        Figure(
          title = "Compare OpAmps",
          identifier = "reverent-mandelbrot",
          preferred = true,
          plots = {
            Plot(curves = {Curve(y = opAmp1.v_out), Curve(y = pulse.y)}),
            Plot(curves = {Curve(y = opAmp2.v_out), Curve(y = integrator.y)}),
            Plot(curves = {Curve(y = opAmp1.smoothed), Curve(y = opAmp1.strict)})
        })
      }),
      experiment(StopTime = 1)
    );
  end Base;

  model SmoothStrict
    extends Base(opAmp1.smoothed = true, opAmp2.smoothed = true, opAmp1.strict = true, opAmp2.strict = true);
  end SmoothStrict;

  model SmoothNoStrict
    extends Base(opAmp1.smoothed = true, opAmp2.smoothed = true, opAmp1.strict = false, opAmp2.strict = false);
  end SmoothNoStrict;

  model NoSmoothStrict
    extends Base(opAmp1.smoothed = false, opAmp2.smoothed = false, opAmp1.strict = true, opAmp2.strict = true);
  end NoSmoothStrict;

  model NoSmoothNoStrict
    extends Base(opAmp1.smoothed = false, opAmp2.smoothed = false, opAmp1.strict = false, opAmp2.strict = false);
  end NoSmoothNoStrict;
end OpAmpSignalGeneratorTest;

with these results:

SmoothStrict:
image

SmoothNoStrict:
image

NoSmoothStrict:
image

NoSmoothNoStrict:
image

Let me know if I should try something more.

@casella
Copy link
Contributor

casella commented Oct 24, 2024

Sorry if I couldn't keep up with the discussion. Here are my 2 cents. Op-Amps can be used for two kinds of application: one is linera amplifiers, where you try as much as you can to steer clear of saturation, and one is signal generators, where the outputs are often saturated and there are very fast swings from Vcc to -Vcc and vice-versa. For the first category, using smooth and avoiding events could be a good idea; for the second, it is pretty obvious that simulations will be inaccurate in terms, e.g., of predicting the oscillator frequency. Which is not a marginal issue when analyzing such circuits.

So, even though we are well past the deadline, I would be inclined to try to fix the problem by allowing both options in the OpAmp model, and setting up the example with the appropriate one. As others have already said, this is really a modelling issue, not a tool issue. I think this shouldn't take much effort.

I'll look at @AHaumer's smal library ASAP.

@AHaumer
Copy link
Contributor Author

AHaumer commented Oct 24, 2024

Thanks a lot @maltelenz. That's really a pity, none of the results is correct:
Either we get no output of the opAmps at all, or the solution drifts away.
I would have expected that NoSmoothNoStrict should deliver correct results.
I get similar results with Dymola and OM.
@casella I don't know how to handle this, I'm afraid we won't get it fixed within reasonable time.
My small library implemented all 4 combinations (smooth x strict), Malte and I tested with different tools.
Maybe we should ignore the regression for this release ...

@HansOlsson
Copy link
Contributor

Regarding smooth: I would more say that tools assume that a derivative using smooth is "smooth enough" for the solvers (so not only C0 but C5 or similarly as required by the theory underlying the ODE/DAE-solvers), and this isn't the case for those use-cases.

Regarding zero-solutions, apart from the smooth-ness and events there's another important aspect of the SignalGenerator-example: it has a "state" in the non-linear solver. Specifically in Dymola (also when generating events for smooth) depending on the start-value for r2.i (setting it to -Vps/R2, 0, +Vps/R2) you get inverted, zero, or normal result.
If a tool does not use r2.i as iteration variable I don't know what happens.

@AHaumer
Copy link
Contributor Author

AHaumer commented Nov 10, 2024

Since this issue seems to depend of event generation of the specific solver:
Why not try a regularization function which is nearly linear in the middle and saturates left and right?
Similar as used in Modelica.Mechanics.Rotational.Sources.SignTorque.
I tried with Dymola 2024x: The results are ok!
OpenModelica v1.24.0 has some problems - @casella could you pls. have a look?
I wonder what are the results using System Modeler - @maltelenz could you pls. have a look?
OpAmps.zip

@maltelenz
Copy link
Contributor

@AHaumer The new version of the SignalGenerator in System Modeler:
image
I'm guessing you were expecting a different initialization?

@AHaumer
Copy link
Contributor Author

AHaumer commented Nov 11, 2024

@maltelenz Dymola shows a correct result, OpenModelica shows the same result as SystemModeler.
SignalGenerator1
SignalGenerator2
I have no idea how to "cure" the IdealizedOpAmpLimited ...

Enclosed the whole package Modelica.Electrical.Analog.Examples.OpAmps from MSL with an enhanced version,
see docu layer of IdealizedOpAmpLimited for the choices.
OpAmps.zip
All examples work fine with Dymola.
OpenModelica fails for the last four {nvertingSchmittTrigger, SchmittTrigger, MultiVibrator, SignalGenerator}.

@HansOlsson
Copy link
Contributor

@AHaumer The new version of the SignalGenerator in System Modeler: image I'm guessing you were expecting a different initialization?

How is System Modeler solving the system of equations?

  • (I assume it is a system of equations, right?)
  • What variable is iteration variable?
  • What is its guess-value?

@henrikt-ma
Copy link
Contributor

How is System Modeler solving the system of equations?

I think this conversation is becoming too inefficient. I'd prefer investigating this in an online meeting instead.

@casella
Copy link
Contributor

casella commented Nov 12, 2024

Thank you @AHaumer for providing the new test implementation and the tests in your OpAmps package!

As I wrote above, I think that if you want to use the OpAmps as triggers and you want a precise estimation of the oscillator period, events should be used, otherwise we are depending on how ODE solvers react to signals that are not smooth enough and/or on which specific strategy each solver uses to handle smooth(), which I think it's not good. I hope there is agreement on this point.

That said, I checked OpAmps.SignalGenerator. Indeed, the op-amps output voltages remain at zero in OpenModelica, as I understand it is the case with System Modeller. The problem is, this is a perfectly legitimate solution for the system, which is strongly non-linear and has three valid initial solutions.

The initial voltage of the capacitor is zero. The first possible solution is the one found by OpenModelica and System Modeller: all node voltages are zero, the op-amp input voltages are both zero, so they are equal, so their output is zero, and the system is at equilibrium. We know this is an unstable equilibrium, so in the real system small fluctuations in the op-amp input voltages due to electronic noise fluctuations will drive the system out of this equilibrium onto a stable periodic trajectory. Unfortunately, this doesn't happen in a numerical simulation, which has no such random fluctuations. If you simulate:

model Unstable
  Real x(start = 0, fixed = true);
equation
  der(x) = x;
end Unstable;

which is obviously unstable, you get a nice x = 0 solution also with numerical solvers, because any integration formula will just compute a zero increment, add it to the zero state and obtain an exact zero.

The other two valid initial solutions correspond to opAmp1 ouptut being saturated either at +15 or -15 Volt; they are not equilibrium solutions, so the oscillations start from there. BTW, the choice of the actual initial solution by the circuit is completely non-deterministic. In practice, if you switch on such a circuit in real life, whether the intial output voltage of opAmp2 will be positive or negative will be due to chance.

This is actually a very instructive example if you want to learn the possible intricacies of Modelica models involving nonlinear equations. I am sure @dzimmer would love that.

How we deal with this, we'll discuss in the meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Critical/severe issue example Issue only addresses example(s) L: Electrical.Analog Issue addresses Modelica.Electrical.Analog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants