From 748e71144ca86afb69b798a713e42af97cfffa80 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 1 May 2026 18:59:00 +0000 Subject: [PATCH] Fix nonsense signal/SNR for HF paths longer than 7000 km For any path >= 7000 km, predict() called _combine_short_and_long with a long_pred produced by _evaluate_long_model -- which is a stub: def _evaluate_long_model(self, frequency: float) -> Prediction: \"\"\"Evaluate long path model (not fully implemented).\"\"\" # Simplified implementation - would need two Reflectrix objects # For now, return a basic prediction return Prediction() A default Prediction() has signal.power_dbw = 0.0, power10 = 0.0, etc. _combine_short_and_long happily mixed those zeros into the result: * dist >= 10000 km: returned long_pred unchanged (all-zero signal), back-deriving total_loss_db = tx_power_dbw - 0 = ~20 dB. * 7000 km <= dist < 10000 km: smooth-interp pulled power_dbw toward the long path's 0 dBW. With a typical short_pwr10 of -120 dBW, the math collapses to power_dbw ~= 10*log10(r) ~= -0.7 dBW, giving the constant signal_dbw the dashboard was rendering. The dashboard reported this as flat -0.7 dBW signal and ~+155 dB SNR for every region beyond 7000 km, with all 24 hours identical and reliability pinned at 100%. Closer regions had the opposite failure mode -- short predictions at 28+ MHz that legitimately couldn't propagate produced total_loss values bounded by the muf-penalty term, which then surfaced as -200 to -270 dB SNR via the same combine step writing back through power_dbw - tx_power_dbw. Until a real long-path model exists, skip the combine step entirely and use the short prediction for every distance. Short-mode VOACAP results are already valid out past 10000 km; the F2 multi-hop tracking just gets less accurate, which is preferable to the engine inventing signal. Also fix a secondary bug: tx_power_dbw was stamped onto current_antenna before the per-frequency loop, where current_antenna still points at the isotropic default. select_antenna() then swapped in a user-added antenna whose tx_power_dbw came from its constructor (typically a placeholder like 10 dBW for the dashboard). Move the assignment inside the loop, after select_antenna(), so the real params.tx_power reaches whichever antenna gets used. Add regression tests asserting that: * signal.power_dbw < -50 dBW, snr_db < +100 dB, total_loss_db > 80 dB for paths at 4500 / 8500 / 11000 km against a 100 W TX with isotropic antennas (each clearly violated by the stub-mixing bug), * params.tx_power flows through to a user-added antenna, surviving its placeholder tx_power_dbw. https://claude.ai/code/session_01BLfWHK9hf8k4aBBaQjfQa8 --- src/dvoacap/prediction_engine.py | 20 ++++--- tests/test_prediction_engine.py | 91 ++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 tests/test_prediction_engine.py diff --git a/src/dvoacap/prediction_engine.py b/src/dvoacap/prediction_engine.py index bf9cdde..549baff 100644 --- a/src/dvoacap/prediction_engine.py +++ b/src/dvoacap/prediction_engine.py @@ -227,8 +227,6 @@ def predict( self.utc_time = utc_time self.frequencies = frequencies.copy() - # Initialize transmit power - self.tx_antennas.current_antenna.tx_power_dbw = self._to_db(self.params.tx_power) self.muf_calculator.min_angle = self.params.min_angle # Allocate results array @@ -296,6 +294,12 @@ def predict( for f, freq in enumerate(self.frequencies): self.tx_antennas.select_antenna(freq) self.rx_antennas.select_antenna(freq) + # Stamp the configured TX power onto whichever antenna was just + # selected; setting it once before the loop only writes to the + # isotropic default and gets shadowed when select_antenna swaps + # in a user-added antenna whose own tx_power_dbw came from its + # constructor (typically a placeholder). + self.tx_antennas.current_antenna.tx_power_dbw = self._to_db(self.params.tx_power) # Compute noise distribution fof2 = self._profiles[-1].f2.fo @@ -311,11 +315,13 @@ def predict( # Evaluate short model prediction = self._evaluate_short_model(reflectrix, f) - # Combine with long model if needed - if self.path.dist >= self.RAD_7000_KM: - long_pred = self._evaluate_long_model(freq) - prediction = self._combine_short_and_long(prediction, long_pred) - + # Long-path model is not implemented (_evaluate_long_model is a + # stub that returns an empty Prediction). Calling + # _combine_short_and_long with that stub would let its zero-valued + # power_dbw pollute the smooth-interpolation branch and back- + # derive a near-zero total_loss for any path between 7000 and + # 10000 km, or hand back an all-zero prediction beyond 10000 km. + # Until the long-path model is real, always use the short result. self.predictions[f] = prediction def _compute_control_points(self) -> None: diff --git a/tests/test_prediction_engine.py b/tests/test_prediction_engine.py new file mode 100644 index 0000000..361dbd1 --- /dev/null +++ b/tests/test_prediction_engine.py @@ -0,0 +1,91 @@ +"""Regression tests for PredictionEngine. + +Guards against the long-path stub bug where _evaluate_long_model returned an +empty Prediction() and _combine_short_and_long mixed it into the result for +paths >= 7000 km, producing absurdly high signal_dbw / SNR for mid-range +distances and an all-zero prediction beyond 10000 km. +""" + +from __future__ import annotations + +import numpy as np +import pytest + +from dvoacap.path_geometry import GeoPoint +from dvoacap.prediction_engine import PredictionEngine + + +def _run_prediction(distance_km_target: float, freq_mhz: float = 14.1): + """Run a prediction along the equator at roughly the requested distance.""" + eng = PredictionEngine() + eng.params.ssn = 60 + eng.params.month = 5 + eng.params.tx_power = 100.0 + eng.params.tx_location = GeoPoint.from_degrees(0.0, 0.0) + + # Approximate longitude offset for a great-circle distance along the equator. + # Earth radius is 6370 km in the engine. + lon_deg = float(distance_km_target / 6370.0 * 180.0 / np.pi) + rx = GeoPoint.from_degrees(0.0, lon_deg) + + eng.predict(rx_location=rx, utc_time=18 / 24.0, frequencies=[freq_mhz]) + return eng.predictions[0], eng.path.dist * 6370.0 + + +@pytest.mark.parametrize("distance_km", [4500, 8500, 11000]) +def test_long_distance_signal_is_physically_plausible(distance_km): + """Signal level must reflect a real path-loss budget, not a stub bypass. + + For HF on 20 m at these distances, received power against a 100 W TX with + isotropic antennas should sit well below 0 dBW; SNR likewise capped well + below the dB regime that signaled the stub-mixing bug (>+100 dB). + """ + pred, actual_distance = _run_prediction(distance_km) + + assert pred.signal.power_dbw < -50.0, ( + f"Received power {pred.signal.power_dbw:.1f} dBW at " + f"{actual_distance:.0f} km is implausibly high for HF; long-path stub " + f"likely leaking back in." + ) + assert pred.signal.snr_db < 100.0, ( + f"SNR {pred.signal.snr_db:.1f} dB at {actual_distance:.0f} km is " + f"physically impossible; long-path stub bug regressed." + ) + assert pred.signal.total_loss_db > 80.0, ( + f"total_loss_db {pred.signal.total_loss_db:.1f} dB at " + f"{actual_distance:.0f} km is far too low for an HF skywave path." + ) + + +def test_tx_power_propagates_to_user_added_antenna(): + """tx_power_dbw must reach whichever antenna select_antenna picks, not + just the isotropic default that current_antenna points at before the + per-frequency loop.""" + from dvoacap.antenna_gain import VerticalMonopole + + eng = PredictionEngine() + eng.params.ssn = 60 + eng.params.month = 5 + eng.params.tx_power = 100.0 # 100 W -> 20 dBW + eng.params.tx_location = GeoPoint.from_degrees(0.0, 0.0) + + # Construct an antenna with a deliberately wrong placeholder power so the + # bug, if regressed, is unmistakable. + ant = VerticalMonopole( + low_frequency=14.0, high_frequency=14.5, tx_power_dbw=-30.0 + ) + eng.tx_antennas.add_antenna(ant) + eng.rx_antennas.add_antenna(ant) + + eng.predict( + rx_location=GeoPoint.from_degrees(0.0, 40.0), + utc_time=18 / 24.0, + frequencies=[14.1], + ) + + assert eng.tx_antennas.current_antenna.tx_power_dbw == pytest.approx( + 20.0, abs=1e-6 + ), ( + "params.tx_power=100W must be stamped onto the selected antenna; " + f"got {eng.tx_antennas.current_antenna.tx_power_dbw} dBW." + )