nec2diff treats any two negative values as equal (missing fabs in BaseInput.h diff()) #138

Summary

nec2diff treats any two negative values as equal, so it reports Difference = 0 for real changes in negative reactance, negative dB gains, and negative voltage/current components. A related edge case gives inf when a value only changes sign, and the program always exits with status 0, so scripts can't detect a difference.

Found at commit 7605c05 (v2.3.4), building with CMake on macOS (arm64, AppleClang 21) and reproduced with GCC 11.4 on Ubuntu 22.04.

Cause

src/BaseInput.h, real-valued diff():

nec_float diff(nec_float a, nec_float b)
{
	if (a == b)
		return 0;
	if ((a < 1e-8) && (b < 1e-8))   // <-- true for ANY two negative numbers
		return 0;
	...

The intent appears to be "both values are effectively zero", but without fabs() every pair of negative numbers takes this early return. The complex overload just below uses abs(a) < 1e-8, so it isn't affected.

A second issue in the same function: the result is |a − b| / |a + b|. When b == −a (a pure sign change, e.g. X = −179 Ω vs. +179 Ω), a + b == 0 and the division gives inf.

Steps to reproduce

Deck short_dipole.nec (a short, capacitive 20 m dipole in free space):

CM Short (capacitive) 20 m dipole in free space
CE
GW 1 21 0 -4.5 0 0 4.5 0 0.001
GE 0
EX 0 1 11 0 1 0
FR 0 1 0 0 14.2 0
RP 0 1 3 1000 90 0 0 45
EN

nec2++ -i short_dipole.nec -o a.out

Feedpoint impedance is 49.889 − j179.39 Ω. To isolate the comparison logic, make copies of a.out with one value changed in each:

# 1. Reactance -179.39 -> -1.00 ohm (both negative)
sed 's/4.9889E+01 -1.7939E+02/4.9889E+01 -1.0000E+00/' a.out > b_reactance.out
# 2. Pattern gain at phi=45 deg: -1.71 -> -9.71 dBi (both negative)
sed 's/   -1.71    -1.71/   -9.71    -9.71/' a.out > b_gain.out
# 3. Control: resistance 49.889 -> 59.889 ohm (positive)
sed 's/4.9889E+01 -1.7939E+02/5.9889E+01 -1.7939E+02/' a.out > c_resistance.out
# 4. Reactance sign flip: -179.39 -> +179.39 ohm
sed 's/4.9889E+01 -1.7939E+02/4.9889E+01  1.7939E+02/' a.out > d_signflip.out

for f in b_reactance b_gain c_resistance d_signflip; do nec2diff a.out $f.out; echo "exit=$?"; done

Actual results

Output for case 1, a 178 Ω reactance change reported as identical:

NecDiff 0.12
File1: a.out
File2: b_reactance.out
Impedance : 49.889 -179.39
Impedance : 49.889 -1
AntennaInput Difference = 0
PowerBudget Difference = 0
Radiation pattern: 3 lines
Radiation pattern: 3 lines
RadiationInput Difference = 0

Expected results

• Cases 1 and 2 report a non-zero difference, as the positive-value control (case 3) does.
• Case 4 reports a large, finite difference.
• The exit status is non-zero when any section differs, so nec2diff can be used in scripts and test harnesses.

Impact

Most of a radiation pattern is negative dBi, and capacitive feedpoints have negative reactance, so a large share of the values nec2diff reads are never actually compared. Anything relying on it as a regression check (including the project's own test harness, if it uses nec2diff) can pass when results have changed.

Suggested fix

Minimal change (fixes cases 1 and 2):

	if ((std::fabs(a) < 1e-8) && (std::fabs(b) < 1e-8))
		return 0;

Also avoiding the inf in case 4 (normalising by the larger magnitude instead of |a + b|):

nec_float diff(nec_float a, nec_float b)
{
	if (a == b)
		return 0;
	nec_float scale = std::max(std::fabs(a), std::fabs(b));
	if (scale < 1e-8)
		return 0;
	nec_float ret = std::fabs(a - b) / scale;
	if (ret > 1e-2)
		cout << "    diff(" << a << "," << b << ") = " << ret << endl;
	return ret;
}

With this version applied locally, comparing a.out with itself still gives 0, and the four cases give 0.994 (1), 0.824 (2), 0.167 (3) and 2 (4).

Note this changes the scale of the metric slightly (|a−b|/max(|a|,|b|) instead of |a−b|/|a+b|), so the 1e-4 and 1e-2 thresholds may need a look.

For the exit status, main() in src/necDiff.cpp could track whether any equalto() check failed and return 1.

Minor, related

main() checks argc < 2 but continues anyway, and two file names are needed (argc < 3). Running nec2diff with zero or one argument prints the usage line and then aborts:

terminate called after throwing an instance of 'std::logic_error'
  what():  basic_string::_M_construct null not valid

Returning after printing usage when argc < 3 would fix it.

Thanks for maintaining nec2++!