#121 Continue Newton iteration until currents settle

[Keith OHara]

The code testing convergence of currents through each nonlinear device, as described §1.4.2 of the User's Manual, uses node voltages from the previous iteration where it should use the latest matrix solution --- each test computes zero for the change in currents. Also, any reports of needing more iterations (flag CKTnoncon) are not communicated from the non-linear-device testers to the driver. These issues exist since Berkeley Spice 3f5

This patch addresses only arbitrary source devices (B-devices). There is a similar error in the tests for other nonlinear devices.

The patch prodcues the expected behaviour for the input in bug #695.
The regression test in the patch is a more realistic example, than the one in the bug report.

[marcel hendrix]

That is really good news.

[Francesco Lannutti]

Hi all,
are you sure we use NEWCONV by default or is it anyway enabled through configure? I don't remember this.
Second point is: the trouble node is always 0. Is it correct?

@Keith: thanks for the patch! :)

Thank you,
Fra

[Keith OHara]

NEWCONV is a simple always-active #define in src/include/macros.h since 1999.
Berkeley Spice 3f5 has it as an always-active #define in src/include/capabil.h since 1990.

In the circuit in the regression test, there is only one non-ground node, N01

[Francesco Lannutti]

Ok! Good to know! :)

Ok for this circuit, but in general the troubleNode should be discovered, I guess... But anyway, this is a second order problem.
Let's try to fix the main problem and generally extend the methodology.
I also assume (because I don't remember the code now) that you act before the RHSold and RHS are swapped. I'm saying that because RHSold contains always the final solution at the end of NR cycle.

Thank you,
Fra

[Keith OHara]

I misunderstood you earlier about CKTtroubleNode=0.
That field is set to the first voltage node found with voltage not yet converged.

If the first found problem is a branch current not yet converged, the code sets CKTtroubleNode=0 to indicate no node single node has the problem, and sets CKTtroubleElt to point to the device whose current has not yet converged. This was done for linear elements like G current sources, now nonlinear elements as well.

And yes, the iteration routine in niiter calls NIconvTest() after the matrix solve,
So at this point CKTrhs contains the newest node voltages (and linear branch currents) but nonlinear elements still hold their internal state computed from the voltages in CKTrhsOld
Later, at the end of the main loop in niiter.c, CKTrhs is swapped into CKTrhsOld.

(For anyone reading this without context of the code, it might be confusing that "rhs" is first loaded with the currents on the right-hand side of the matrix equation [G] v = i but at the moment under discussion, after solving the matrix equation, CKTrhs holds the solution v, which is mostly node-voltages. This is the world of solving large sparse matrices with limited memory in the 1980s.)

[Keith OHara]

Updated patch. I notice now that I formed the patch from a version before I had added the code comments.

     i = CKTconvTest(ckt);

+    /* The individual testers, called by CKTconvTest, set
+     * ckt->CKTnoncon and ckt->CKTtroubleElt appropriately. */
+    if (ckt->CKTnoncon != 0) {
+        ckt->CKTtroubleNode = 0;
+        return(1);
+    }
+    /* CKTconvTest early-returns nonzero 'i' on the first error
+     * in evaluating convergence (such as parameter out of range) so
+     * there may be untested devices that have not yet converged *
     if (i)
        ckt->CKTtroubleNode = 0;
     return(i);

Sorry about that. Correction attached, with plots
(the amount violation of Kirchhoff's current law, which should be less than 20nA)
from the regression test before versus after patch.

[Francesco Lannutti]

Ok, now we are on the same page! :)

BTW, you should try the branch KCL where I added a KCL Verification as convervenge criterion for f(v). It's quite old, so it needs to be at least rebased...

Thank you,
Fra

[Keith OHara]

I do not see KCL but suppose you mean KLU-kirchhoff-3. That is interesting.

I have seen the developer of Spectre, which enforces Kirchhoff's current sums at each node,
disparage Spice for failing to ensure that currents sum to zero.

This confused me, because Spice /claimed/ to be enforcing the convergence of each branch current, in the context of a matrix solve that implicitly enforces the sums of currents to balance at each node. Spice's solution seemed, to me, superior in the area Spectre was pointing to.

[Francesco Lannutti]

Yep, that one. Check if there is a more recent Kirchhoff branch, BTW.
Yes, the idea is that the independent variable V can be checked with ABSTOL and RELTOL and RHSold vs RHS, while the dependent variable I is actually a function of V, so only the model knows exactly the convergence criterion for each function of V (which is the NIConvTest part we were talking about before). But this could contain error or being not precise or other things, so I developed another criterion, which is the KCL verification at each node.
The problem is that, as far as I remeber, the model has to be modified, because the currents have to be injected in a separate database visible at circuit level.
However, the idea still stands...
So, I'm quite sure I did only a proof of concept as you did and I have modified only few models...

Thank you,
Fra

[Holger Vogt]

Applying this patch, the VDMOS power transistor model stops functioning. All examples from https://sourceforge.net/p/ngspice/ngspice/ci/master/tree/examples/vdmos/ are failing.

[Keith OHara]

The VDMOS code https://sourceforge.net/p/ngspice/ngspice/ci/master/tree/src/spicelib/devices/vdmos/vdmosconv.c
is setting the flag CKTnoncon even when the currents have converged within tolerance.
This flag was formerly ignored, so the decisions of vdmosconv.c have been ignored since this code was created. This seem to have in turn ignored be two bugs in (Edit: should have written vdmosconv.c ) :

VDMOStype was missing so vd had the wrong sign for p-channel devices.

-            vd = *(ckt->CKTrhsOld+here->VDIOposPrimeNode)-
-                    *(ckt->CKTrhsOld + here->VDMOSdNode);
+            vd = model->VDMOStype * (
+                    *(ckt->CKTrhsOld+here->VDIOposPrimeNode)-
+                    *(ckt->CKTrhsOld + here->VDMOSdNode))

(One of the not-yet-addressed 'similar errrors' that I mentioned at the top remains here. After the fix above, vd will now be the correct sign, but the wrong timestep due to its use of CKTrhsOld.)

There are two locations, VDMOSgNode and VDMOSgNodePrime, holding versions of the voltage at the gate. The similar code for MOS1 devices updates and tests MOS1gNode during Newton Iteration; the code for VDMOS updates VDMOSgNodePrime but used VDMOSgNode to test for convergence.

             vgs = model->VDMOStype * ( 

-                *(ckt->CKTrhs+here->VDMOSgNode) -
+                *(ckt->CKTrhs+here->VDMOSgNodePrime) -
                 *(ckt->CKTrhs+here->VDMOSsNodePrime));

So vdmosconv.c evaluated |current^hat_k+1 - current_k| with a miscalculation of current^hat_k+1 and incremented CKTnoncon, and with this patch CKTnoncon causes further Newton iterations. These iterations, however, do not bring the miscalculated current_hat closer to the correctly calculated current_k, so the convergence routine fails.

Correcting vdmosconv.c as shown here /seems/ to make the examples work as intended with the rest of this patch, but I would have to compare carefully with how they worked before the patch to be sure, and I have not checked that the gNode/gNodePrime difference still works if there is significant gate resistance.

There could be other problems that will be discovered if we start acting on results of the long-ignored *conv.c files in each of the device models. I do not know any realistic examples that are significantly helped by iterating the until nonlinear branch currents settle.

Given possible harm from uncovering bugs versus unknown benefit, I withdraw this patch.

[Dietmar Warning]

In which line is the missing VDMOStype in vdmosload.c?

[Keith OHara]

line 85
of vdmosload.c needs the factor VDMOStype

[Dietmar Warning]

You meant vdmosconf.c. l.85 and this is done last year with commit 8c3d4d79.
I will look regarding the bug in vgs equation.
Thank you.

[Giles Atkinson]

Now that version 46 is done, it seems a good time to integrate the patch. It will get perhaps 6 months of informal testing in pre-master. Dietmar has fixed the VDMOS problem. I reviewed the other convergence check functions and to me all but the (now fixed) VSRC look sane.

[Holger Vogt]

Patch is applied to pre-master-47

[Francesco Lannutti]

Hi guys,
what do you think if we also include my work on KCL Verification? It's basically on the same topic of this one. Maybe we can combine it...

Thank you,
Fra

[Holger Vogt]

Is it ready for patching, or does it need further optimisation?

[Holger Vogt]

Which branch are you talking about? Is it 'new-kirchhoff-5' ?

What is the advantage of this approach ?

Do you have made some description or publication?

[Francesco Lannutti]

Hi Holger,
no, no, because it was experimental for some models. It needs code model modification, like KLU...
But we could brainstorm offline among all of us.

Thank you,
Fra

[Francesco Lannutti]

Hi Holger,
I think it's that one, but it's a 10years old work, so I don't remember exactly! :)
It's part of my PhD, attached, along with LnL Separation (I think there is a branch for this too).

Thank you,
Fra

[Robert Turnbull]

This patch has eliminated the false convergence Giles noted on bug #824. My test circuit now fails to converge with or without the added 0V source.

[Giles Atkinson]

The patch was missing a file, convergence.cir, causing Bug #839. Keith OHara, please post the file.

[Keith OHara]

2 files, attached, go in tests/regression/misc
They are in the patch up-thread, so it might be easier reapply and 'git add' the two new files.

I did follow up on my comment "there is a similar error in the tests for other nonlinear devices" from the top post, and in the end found only one or two copies of this error. I'll try within one day to find and recheck that completion of this patch, and then post it here.

[Giles Atkinson]

Thanks! G.