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

incorrect $mfactor scaling for potential noise contributions #137

Open
gjcoram opened this issue Jul 8, 2024 · 9 comments
Open

incorrect $mfactor scaling for potential noise contributions #137

gjcoram opened this issue Jul 8, 2024 · 9 comments

Comments

@gjcoram
Copy link

gjcoram commented Jul 8, 2024

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

  • Contributions to a branch potential quantity using the noise functions of 4.6.4 shall have the noise power divided by $mfactor

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
Copy link
Author

gjcoram commented Jul 8, 2024

See also #107

gjcoram added a commit to gjcoram/OpenVAF that referenced this issue Jul 8, 2024
@gjcoram
Copy link
Author

gjcoram commented Jul 16, 2024

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
Copy link

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.0
P_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
Copy link
Author

gjcoram commented Jul 23, 2024

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
Copy link

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 added a commit to arpadbuermen/OpenVAF that referenced this issue Jul 31, 2024
@arpadbuermen
Copy link

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
Copy link

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 d5f1580 in main branch of openvaf. In branches/osdi_0.3 see 15db2df.

@dwarning
Copy link

dwarning commented Oct 4, 2024

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
Copy link

dwarning commented Oct 4, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants