pascalkuthe / OpenVAF

An innovative Verilog-A compiler
https://openvaf.semimod.de/
GNU General Public License v3.0
123 stars 20 forks source link

incorrect $mfactor scaling for potential noise contributions #137

Open gjcoram opened 2 months ago

gjcoram commented 2 months ago

If one has V(out) <+ white_noise(1.0, "thermal"); the Verilog-AMS LRM says (section 6.3.6 in VAMS-LRM-2023.pdf):

However, in openvaf/sim_back/src/dae/builder.rs in fn add_source_equation (for handling voltage-source equations), we have

    self.add_noise(contrib, SimUnknownKind::Current(dst.into()), None, F_ONE);

where F_ONE is the mfactor argument to add_noise.

gjcoram commented 2 months ago

See also https://github.com/pascalkuthe/OpenVAF/issues/107

gjcoram commented 2 months ago

res_v.zip In the attached example, compiling res_v.va with OpenVAF 23.5.0, I get different results for 5 resistors in parallel rather than one resistor with m=5, due to incorrect scaling of potential noise contributions. For 5 resistors in parallel (res_v.out.bad): 0 1.000000e+00 3.315218e+02 3.315218e-18 but with m=5 0 1.000000e+00 1.657609e+03 1.657609e-17

Whereas after including the update from commit 477e71c, I get the same value for both (res_v.out.good): 0 1.000000e+00 3.315218e+02 3.315218e-18 and 0 1.000000e+00 3.315218e+02 3.315218e-18

dwarning commented 2 months ago

psphv v.1.0.6 using a macro for node collapsing and noise contribution, line 3813:

collapsibleR(b_rg , RGE_i , RGE_i , TKD, rthresh, mMod, 0, swnoise, P_K_NIST2002, "rgate thermal noise")

The macro is as follows (general_v1_0_4.va):

define collapsibleR(b_r, r_t, r_tNom, tdevK, rThreshold, m, strict, sw_noise, P_K, noiseName) if ((r_tNom)>((m)(rThreshold))) begin I(b_r) <+ V(b_r)/(r_t); if (sw_noise) begin I(b_r) <+ white_noise(4.0P_K(tdevK)/(r_t), noiseName); end end else begin if (strict) begin V(b_r) <+ 0.0; end else begin V(b_r) <+ MAX(r_t, 0.0)I(b_r); end end

If I remove the commit 477e71c everything works fine.

But with this patch "OpenVAF encountered a problem and has crashed!"

gjcoram commented 2 months ago

Arpad reported what is probably the same problem, in his case compiling a version of BSIM4. When I tried it, I got some message I think from LLVM about "PHI nodes not grouped at the top of the basic block" I commented out code and found that the problem is related to the noise contributions for some switch branches (V or I depending on the resistance, just like you have above). But I don't know Rust/LLVM well enough to figure out why. I do know that my patch is necessary to get the noise right for the example I uploaded.

dwarning commented 2 months ago

I can confirm that the Cogenda model gives memory access error with commented out XYCE macro.

bsim4_release.va.txt

I removed the false __XYCE_VAMS__ branch what is in the end only to have current contribution. So that means that commit 477e71c has problems with voltage contribution if in the code is node collapsing too:

case (BSIM4rgateMod)
0: begin
   V(g,gm)  <+ 0.0;

3: begin
   V(g,gm)  <+ I(g,gm)  / BSIM4grgeltd;

Noise comes later with current contribution:

    I(gm, g) <+ white_noise(4 * `P_K * T * BSIM4grgeltd, "Rg");

The bsim4 code in my repo works with ngspice and Xyce.

arpadbuermen commented 2 weeks ago

I guess this one is closed now. I hear incorrect noise scaling in case of factor*white_noise(...) is still an issue. Have some ideas, will report when I find a solution. No time now...

arpadbuermen commented 2 days ago

Noise scaling is fixed now.

a*white_noise(...) scales the noise power by a**2

$mfactor=a scales the noise power by a factor of a.

See d5f158009022d62c239255643a53b4e76b83b2ee in main branch of openvaf. In branches/osdi_0.3 see 15db2dfb341e7993918f6003745f62cdd72bb2e0.

dwarning commented 2 days ago

Sorry @arpadbuermen can't confirm: I am under linux Ubuntu 22.04, llvm16.0.6. sitting in you /branches/osdi_0.3, can see your latest commit 15db2df from yesterday. No problems with "cargo build --release --bin openvaf". The building osdi from p6, p7 and psp103p82 sources. All get same behaviour. Bildschirmfoto vom 2024-10-04 10-04-21

dwarning commented 2 days ago

Want complement that the res_v test case from @gjcoram gives correct results wit the mentioned 15db2df commit on branches/osdi_0.3.