Port Renumbering Is Incorrect

[denzchoe]

Hi David,

PyBERT/pybert/pybert_util.py

Lines 857 to 868 in ab44470

	def sdd_21(ntwk):
	"""
	Given a 4-port single-ended network, return its differential throughput.
	Args:
	ntwk(skrf.Network): 4-port single ended network.
	Returns:
	[float]: Sdd[2,1].
	"""
	if real(ntwk.s21.s[0, 0, 0]) < 0.5: # 1 ==> 3 port numbering?
	ntwk.renumber((2, 3), (3, 2))

I suspect there is a mistake here.
Port Number may start at 1 but Python Object Index starts at 0.

The correct fix should be at L868:
ntwk.renumber((1, 2), (2, 1))
or
ntwk.renumber([0, 1, 2, 3], [0, 2, 1, 3])

Also see documentation improvement for .renumber function https://github.com/scikit-rf/scikit-rf/blob/1fce5d4e4e1c40d14906ed245154124abcc59b3a/skrf/network.py#L2918-L2986 raised from Closed Issue here https://github.com/scikit-rf/scikit-rf/issues/499

[capn-freako]

Hi @denzchoe ,

Sorry for the delayed response.
And thank you! for all the time and energy you've invested in troubleshooting/debugging PyBERT.
Your contribution is greatly appreciated.

I have confirmed that your suggestion is correct and am affecting the needed change to the code now.

Thanks!
-db

[denzchoe]

No problemo David!

[capn-freako]

Note: This comment doesn't pertain to the immediate action required, but is included as an explanatory comment for the historical record.

Question: Why wasn't this bug in the sdd_21() function discovered long ago?

Answer: Because the buggy code produces a result of the correct order of magnitude.

Consider: the intent of the sdd_21() function is to affect the following conversion in assigned Touchstone port numbers:

1 ==> 3                            1 ==> 2
                     ==>
2 ==> 4                            3 ==> 4

Due to my array indexing error (i.e. - 1-based vs. 0-based), I was doing this instead:

1 ==> 3                            1 ==> 4
                     ==>
2 ==> 4                            2 ==> 3

Now, post-correction the calculation of SDD[2,1] is performed as follows:

    return 0.5 * (ntwk.s21 - ntwk.s23 + ntwk.s43 - ntwk.s41)

Using my original (incorrect) port number remapping (i.e. - 3 <=> 4) to translate this expression back to the nomenclature of the original model:

    return 0.5 * (ntwk.s21 - ntwk.s24 + ntwk.s34 - ntwk.s31)

And, using the following substitutions for terms of the form ntwk.sNN:

• ILF+: forward insertion loss of the "upper" channel,
• ILB+: backward insertion loss of the "upper" channel,
• ILF-: forward insertion loss of the "lower" channel,
• ILB-: backward insertion loss of the "lower" channel, and
• RL: any return loss term,

we get:

    return 0.5 * (RL - ILB- + RL - ILF+)

And, neglecting the two return loss terms:

    return -0.5 * (ILB- + ILF+)

Assuming the insertion losses of both "upper" and "lower" channels are symmetric, we see that we're calculating the inverted average of the two, not at all what we wanted, but of the same order of magnitude as our intended result.

Now, repeating the above for the correct port indexing yields:

    return 0.5 * (ntwk.s31 - ntwk.s32 + ntwk.s42 - ntwk.s41)

(Adding: ILFx: insertion loss "cross"/"complement"):

    return 0.5 * (ILF+ - ILFx + ILF- - ILFx)

And, now, we find that we're taking the average of each forward insertion loss path, corrected by its respective "cross", or "complement", insertion loss path, which is exactly the definition of SDD[2,1].