Skip to content

named the wideband fit's elements the data does not determine, kept a CLI batch going past a file that crashes the engine, and noted the line's fragile refit - #30

Merged
simi1505 merged 8 commits into
mainfrom
fix/cli-batch-and-fit-spread
Oct 18, 2026
Merged

simi1505 merged 8 commits into
mainfrom
fix/cli-batch-and-fit-spread

Conversation

@simi1505

Copy link
Copy Markdown
Collaborator

Three follow-ups from the work on #29, none of them visible on a default conversion of a bundled file except the new note on two coils.

Added

  • A note naming the elements of a wideband or center-tapped coil fit that the data does not determine (inductor_fit._spread_warnings()), for example Rskin2_h1 (x3), Lskin2_h1 = Lskin2_h2 (x3) and Rskin2_h2 (x3) are not determined by the data, each can change by that factor or more while the fit's RMS error grows by 0.5 % or less (skin section 2 has its corner above the fit band, where it acts as plain L, and Basic model leaves section 2 out).
    • On the bundled center-tapped coil a change of 1e-12 in the data moved R_skin2 by 75 % and L_skin2 by 41 %, and those are the elements it names. On the 500 pH coil it names R_skin2. It names nothing on the d20 coil or any basic model.
    • How: the curvature at the optimum (the Gauss-Newton Hessian) picks the candidates and a starting direction. Each candidate is then moved by 3, 10, 100 and 1000 times both ways, the others re-fitted by Gauss-Newton on the optimum's Jacobian, and the element is named with the largest factor whose re-fit stays within the cost limit. Every name therefore comes with a witness model, and the factor is a lower bound.
    • The curvature alone was the first version and failed both ways: a saturating cost put the 500 pH coil's R_skin2 at x1.65, while a re-fit with it at 100 x costs 0.6 % more, and on a noisy synthetic coil it named an element that a re-fit at 3 x shows determined.
    • It can miss elements: on 16 synthetic coils with 1e-3 of S noise it named 35 of the 48 elements that a full least-squares re-fit moves by 3 x, and none that such a re-fit pins down.
    • The cost limit is floored at the cost of a 1e-3 relative error on every point of every goal, which does not depend on the number of frequency points, and a fit below that floor gets a message with the absolute bound.
    • The check takes 2 % to 7 % of a conversion and has its own progress message.

Fixed

  • A CLI batch went on only until a file made engine.convert raise: the command ended with a traceback, and the files after it were never converted. Such a file is now reported as [FAIL] (after its progress bar has cleared the line), its stale outputs are removed, and the batch goes on with exit code 1.
  • Tied half coils kept one skin section order: the center-tapped fit sorted the skin sections of each half on its own, so with the inductances tied and the resistances free a tie could pair section 1 of one half with section 2 of the other. Not seen on the bundled coil.

Docs

  • README, help and doc/architecture.md on the note above, and a caveat on tline_100um: at orders 9 and 13 it comes out passive through the refit with its 0 Hz point, but a random 1e-12 change of its S values made that refit miss in 1 to 6 of 8 tries (the line then reads near-passive at 5.24), on scikit-rf 1.12, 2.1 and 2.2 alike. Order 6 gave the same result in every try.

Verification against main

  • Option-space dump, 574 records (the 370 of the earlier dumps plus one-port and mixed-reference variants), on scikit-rf 1.12.0 (seeds 0 and 7) and 2.2.0: 27 records changed, all wideband or center-tapped, each checked by a script to differ only by the new progress line naming the check (all 27) and one added note (13 of them). Netlists, element values and every other field identical.
  • 297 tests on Windows with scikit-rf 1.12.0 and 2.2.0, and in IIC-OSIC-TOOLS with scikit-rf 2.1.0. Each fix has a test that fails on its parent. reuse lint clean, no file with CRLF in the index.
  • Two reviews. The first found the curvature check's false negative and false positive, a cost floor that grew with the number of points, a misplaced Basic model hint, the [FAIL] line on the progress bar and the sort tie issue, all fixed. The second confirmed that every name has a valid witness and found misses near flat directions of the other elements (fixed: the search starts from the cheaper of the element alone and the linear guess, and leaves those directions out of its steps), the wording of a fit below the floor, and three wrong numbers in the docs, all fixed.

… which ended the command with a traceback and left the files after it unconverted
…the fit can move by 3 x at 1 % more error, from the curvature at the optimum (`_spread_warnings`), with the reason when a skin section lies above the band
… refit that a 1e-12 change of the data tips back to near-passive in 1 or 2 of 8 tries
…actor at no more than 0.5 % more RMS error, where the curvature missed a saturating cost and invented a spread, floored the cost per goal instead of per point, and kept tied half coils on one skin section order
…ne and the linear guess, kept the near-null directions of the others out of its steps, stated the bound of a fit below the floor, and named the check in the progress line
@simi1505
simi1505 merged commit 446e2b3 into main Oct 18, 2026
1 check passed
@simi1505
simi1505 deleted the fix/cli-batch-and-fit-spread branch October 18, 2026 17:35
@VolkerMuehlhaus

Copy link
Copy Markdown
Contributor

Hi @simi1505

please notify if there is something that I should fix in the upstream code.

BR,
Volker

@simi1505 simi1505 mentioned this pull request Oct 18, 2026
@simi1505

Copy link
Copy Markdown
Collaborator Author

@VolkerMuehlhaus a follow-up on the wideband inductor models you contributed, released now in snp2le 0.1.18.

Elements the data does not determine. On the bundled center-tapped coil (ind_ct_n2_d82_w4_s4_ihp-sg13g2.s3p), scaling every S value by 1 + 1e-12 moved R_skin2 by 75 % and L_skin2 by 41 %, while the RMS error moved by 2 %. The second skin section has its corner at about 215 GHz, far above the 60 GHz fit band, where it acts as plain L: its R is free and its L trades against L_s. The 500 pH 2-port example has the same case at a smaller scale (corner 66 GHz, band 50 GHz): a re-fit with R_skin2 held at 100 x costs only 0.6 % more. snp2le now names such elements in the message line. An element is named only with a witness: a re-fit with that element moved by 3, 10, 100 or 1000 x and the others free, at no more than 0.5 % higher RMS error. The fit itself is unchanged.

Your 1.2 skin penalty. The snp2le port follows inductor_fit from before your 1.1. 1.2 (9c7a78f, cb24926) adds --skin-sections, the consistent second skin section and the weak skin_penalty, which picks the smallest L_skin that still fits and so should make exactly these directions unique. Do you see it as the fix for this case, and should we port it to snp2le? The witness check would then show whether anything is still free. We have not run 1.2 on these coils yet.

A latent issue in the center-tap sort, also in your current inductor_fit.py (lines 1331-1332). Each half's skin sections are sorted by corner on their own, while the half-coil inductances can be tied (Lskin1_h2 = Lskin1_h1) with the resistances free. If the two halves order their corners differently, the swap in one half breaks the tie and pairs section 1 of one half with section 2 of the other. snp2le now applies the first half's order to the second whenever skin elements of the two halves are tied. Not seen on the bundled coil.

On why the values move: with ftol=1e-6 a run ends while it still creeps along such a direction. Some of our re-fits of the center tap ended at a cost about 0.4 % below the fit's own, so a 1e-12 change of the data mostly changes where the optimizer stops, not what the data says.

@VolkerMuehlhaus

Copy link
Copy Markdown
Contributor

Hi Simon @simi1505

to give some background: There were some issue when I used this model for inductor surrogate work, the N=1 inductors dind't fit well and we did some extra checks for smoothness and consistency in those models.

This is what Claude answers to your post:

Thanks — I checked all three points against inductor_fit.

Is the 1.2 skin penalty the fix? No. On the bundled center-tapped coil, the uniform 1 + 1e-12 scaling barely moves 1.1 or 1.2 (R_skin2_h1 13.75 → 13.64 Ω in 1.1, 7.64 → 7.73 Ω in 1.2), so your 75 % comes from the pre-1.1 code. A random relative change of 1e-12 (three seeds) moves it in both versions, though:

  | R_skin2_h1 range | spread | fit cost range -- | -- | -- | -- 1.1 | 9.3 – 27.2 Ω | 2.9x | 0.007266 – 0.007288 1.2 | 7.6 – 18.0 Ω | 2.4x | 0.007304 – 0.007312 1.2, ftol=xtol=gtol=1e-12, 5000 evaluations | 8.7 – 11.2 Ω | 1.3x | 0.007307 – 0.007310

The penalty lowers L_skin2 and narrows the spread, but it is too weak to make the direction unique. The corner of section 2 stays above the band: 113 GHz on this 60 GHz coil, about 450 GHz on the 500 pH coil. Your remark on the stopping criterion holds: tighter tolerances narrow the spread further but don't remove it. So your witness check would still name these elements after a 1.2 port. --skin-sections and the consistent second section are worth porting on their own, but don't expect the note to go quiet.

Center-tap sort: confirmed, thanks. It is fixed in inductor_fit 1.2.1 the same way you did it: when skin elements of the halves are tied, half 2 takes half 1's order. Only the reported values were affected; the circuit was not, since the series sections within a half can swap. As you found, a real fit never triggered it: synthetic center-tapped coils built with reversed corner order in half 2 were fitted with the same order in both halves.

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

Successfully merging this pull request may close these issues.

2 participants