From a3f1f24f52b341d4e6fead7bb7fb77d7bf32984d Mon Sep 17 00:00:00 2001 From: ff225 Date: Fri, 17 Jul 2026 14:52:03 +0200 Subject: [PATCH] fix(edge): single owner for edge timings and variance (#72, #73) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two tangled problems in the same code path; the fixes share the same lines, so they land together. #72 — Edge per-layer times were smoothed twice. `track_inference_time` applied an EMA with a hard-coded alpha=0.2 to `self.inference_times` — which is the very dict `device_profiles[device_id]["edge_inference_times"]`, passed by reference — and the request handler then applied the configured EMA on top. Edge times were therefore smoothed twice while device times were smoothed once, so the two sides of the offloading cost comparison responded to change at different rates; and the `ema_alpha` setting added in #9 never reached the first application. `Edge.run_inference` already measures and returns the raw per-layer times, so the decorator's copy was redundant. It is now instrumentation only, and `DeviceStateManager.update_edge_times` is the single place that smooths, with the configured alpha. The timing there now uses perf_counter rather than time.time, since it became the authoritative measurement. #73 — `Edge.variance_detector` was a second, process-global detector fed from the same decorator. Nothing ever read it: no production code calls `should_retest_offloading()` on it, so it only burned time on the hot path and logged re-test warnings that triggered no re-test. Removed; the per-device detectors from #11 remain the only ones. Also drops the now-unwritten `inference_times` plumbing from ModelManager, `save_inference_times` (no callers), and a commented-out __init__ referencing attributes that no longer exist. `scripts/analysis/variance_analysis.py` moves to the per-device API. --- coverage-fast.xml | 5407 +++++++++++++++++ scripts/analysis/variance_analysis.py | 58 +- src/server/communication/request_handler.py | 2 - src/server/edge/edge_initialization.py | 32 +- src/server/models/model_manager.py | 81 +- .../test_edge_inference_time_ownership.py | 85 + 6 files changed, 5544 insertions(+), 121 deletions(-) create mode 100644 coverage-fast.xml create mode 100644 tests/unit/test_edge_inference_time_ownership.py diff --git a/coverage-fast.xml b/coverage-fast.xml new file mode 100644 index 00000000..6bcbf6a4 --- /dev/null +++ b/coverage-fast.xml @@ -0,0 +1,5407 @@ + + + + + + /Users/admin/Documents/Ricerca/SCIoT_python_client + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/scripts/analysis/variance_analysis.py b/scripts/analysis/variance_analysis.py index 5cebac79..7462bc5a 100644 --- a/scripts/analysis/variance_analysis.py +++ b/scripts/analysis/variance_analysis.py @@ -11,14 +11,16 @@ from server.communication.request_handler import RequestHandler -def analyze_current_variance(): +def analyze_current_variance(device_id: str): """ - Analyze current variance state from the RequestHandler's detector. + Analyze current variance state for a device. Provides insights and recommendations. + + Note: variance data lives in the running server's memory, so this only + reports anything when called in-process (e.g. from the server itself). """ - - detector = RequestHandler.variance_detector - stats = detector.get_all_stats() + + stats = RequestHandler.device_state.variance_stats(device_id) print("\n" + "=" * 80) print("CURRENT SYSTEM VARIANCE ANALYSIS") @@ -116,14 +118,13 @@ def analyze_current_variance(): print("\n" + "=" * 80) -def compare_layer_pairs(): +def compare_layer_pairs(device_id: str): """ Compare device vs edge performance for each layer. Helps identify optimal split points. """ - - detector = RequestHandler.variance_detector - stats = detector.get_all_stats() + + stats = RequestHandler.device_state.variance_stats(device_id) print("\n" + "=" * 80) print("DEVICE vs EDGE COMPARISON") @@ -174,16 +175,16 @@ def compare_layer_pairs(): print("\n" + "=" * 80) -def export_variance_data(output_file: str = "variance_stats.json"): +def export_variance_data(device_id: str, output_file: str = "variance_stats.json"): """ Export current variance data to JSON for external analysis. - + Args: + device_id: The device whose variance statistics to export output_file: Path to save the JSON file """ - - detector = RequestHandler.variance_detector - stats = detector.get_all_stats() + + stats = RequestHandler.device_state.variance_stats(device_id) output_path = Path(output_file) @@ -200,23 +201,30 @@ def export_variance_data(output_file: str = "variance_stats.json"): print(""" This utility analyzes inference time variance detected by the system. +Variance statistics are per-device and live in the running server's memory, +so these helpers only report data when called in-process. + Usage: from variance_analysis import ( analyze_current_variance, # Full analysis report compare_layer_pairs, # Device vs Edge comparison export_variance_data # Export to JSON ) - - analyze_current_variance() - compare_layer_pairs() - export_variance_data("my_variance_data.json") + + analyze_current_variance("my-device-id") + compare_layer_pairs("my-device-id") + export_variance_data("my-device-id", "my_variance_data.json") """) - - # Run demonstrations if variance data exists + + # Run demonstrations for every device the handler knows about. print("\nRunning analysis on current system state...") - try: - analyze_current_variance() - compare_layer_pairs() - except Exception as e: - print(f"\n⚠️ Could not analyze system variance: {e}") + known_devices = list(RequestHandler.device_profiles) + if not known_devices: + print("\n⚠️ No devices registered in this process.") print(" (System needs to be running with active inference for data)") + for device_id in known_devices: + try: + analyze_current_variance(device_id) + compare_layer_pairs(device_id) + except Exception as e: + print(f"\n⚠️ Could not analyze variance for {device_id}: {e}") diff --git a/src/server/communication/request_handler.py b/src/server/communication/request_handler.py index 7c0c733f..37bccba3 100644 --- a/src/server/communication/request_handler.py +++ b/src/server/communication/request_handler.py @@ -725,7 +725,6 @@ def handle_device_inference_result(self, body, received_timestamp): device_state = RequestHandler.device_state profile = device_state.register(device_id, num_layers=RequestHandler.num_layers) - edge_inference_times = profile["edge_inference_times"] alpha = load_offloading_ema_alpha_config() device_state.update_device_times( @@ -753,7 +752,6 @@ def handle_device_inference_result(self, body, received_timestamp): prediction, edge_layer_times = Edge.run_inference( offloading_layer_index=message_data.offloading_layer_index, offloading_layer_output=message_data.layer_output, - edge_inference_times=edge_inference_times, model_dir=model_dir, model_key=model_key, models_config=models_config, diff --git a/src/server/edge/edge_initialization.py b/src/server/edge/edge_initialization.py index 85fed06a..e7264f0e 100644 --- a/src/server/edge/edge_initialization.py +++ b/src/server/edge/edge_initialization.py @@ -10,7 +10,6 @@ from server.commons import ModelFiles from server.models.model_manager import ModelManager from server.models.model_input_converter import ModelInputConverter -from server.core.variance_detector import VarianceDetector from sciot.config import load_server_config from pathlib import Path @@ -27,9 +26,6 @@ def load_delay_config(): class Edge: - # Class-level variance detector (shared across edge inference calls) - variance_detector = VarianceDetector(window_size=10, variance_threshold=0.15) - # ===================================================== # OPTIMIZATION: Persistent ModelManager cache. # ModelManager instances (and their TFLite interpreter caches) are @@ -123,9 +119,7 @@ def get_valid_offloading_points(model): return valid_points @staticmethod - def _get_model_manager( - model_dir: str, models_config: dict, edge_inference_times: dict - ) -> ModelManager: + def _get_model_manager(model_dir: str, models_config: dict) -> ModelManager: """ OPTIMIZATION: Get or create a cached ModelManager for the given model_dir. The ModelManager (and its TFLite interpreter cache) persists across calls, @@ -137,19 +131,13 @@ def _get_model_manager( Edge._delay_config = load_delay_config() model_manager = ModelManager( - inference_times=edge_inference_times, computation_delay_config=Edge._delay_config, - variance_detector=Edge.variance_detector, models_config=models_config, ) model_h5_path = ModelFiles.get_model_h5_path(model_dir) model_manager.load_model(model_h5_path) Edge._model_managers[model_dir] = model_manager logger.info(f"Cached ModelManager for model_dir={model_dir}") - else: - # Reuse cached manager, but update inference times reference - # so that the EMA-smoothed times stay in sync with RequestHandler - Edge._model_managers[model_dir].inference_times = edge_inference_times return Edge._model_managers[model_dir] @@ -157,13 +145,16 @@ def _get_model_manager( def run_inference( offloading_layer_index: int, offloading_layer_output: np.array, - edge_inference_times: dict, model_dir: str, model_key: str = None, models_config: dict = None, ): """ Versione dinamica di run_inference. + + Returns the prediction and the raw per-layer timings; smoothing them into + the EMA tables is the caller's job (see `DeviceStateManager`). + :param model_key: La chiave del modello (es. 'fomo_96x96' o 'fomo_144x144') :param models_config: Il dizionario completo dei modelli dal setting.yaml """ @@ -174,9 +165,7 @@ def run_inference( # OPTIMIZATION: Reuse cached ModelManager instead of creating a new one each call. # This preserves the TFLite interpreter cache and avoids re-loading the H5 model. - model_manager = Edge._get_model_manager( - model_dir, models_config, edge_inference_times - ) + model_manager = Edge._get_model_manager(model_dir, models_config) # Preparazione indici dei layer predictions = {} @@ -262,12 +251,14 @@ def run_inference( break # ESECUZIONE INFERENZA DINAMICA - t0 = time.time() + # perf_counter, not time.time: these timings are the authoritative + # per-layer costs feeding the EMA and the offloading decision. + t0 = time.perf_counter() # Passiamo il model_key per usare il TFLite interpreter corretto (96 o 144) prediction = model_manager.predict_single_layer( layer_index, start_layer_offset, prediction_data, model_key=model_key ) - t1 = time.time() + t1 = time.perf_counter() measured_times.append(t1 - t0) predictions[layer.name] = prediction @@ -304,9 +295,8 @@ def initialization( # QUI LA MODIFICA: Passiamo models_config invece di model_dir model_manager = ModelManager( - models_config=models_config, + models_config=models_config, computation_delay_config=delay_config, - variance_detector=Edge.variance_detector, ) model_manager.load_model(ModelFiles.get_model_h5_path(model_dir)) diff --git a/src/server/models/model_manager.py b/src/server/models/model_manager.py index c2052fc2..a668db05 100644 --- a/src/server/models/model_manager.py +++ b/src/server/models/model_manager.py @@ -12,12 +12,10 @@ import numpy as np import tensorflow as tf -from server.commons import OffloadingDataFiles from server.commons import ModelFiles from server.logger.log import logger from server.models.model_manager_config import ModelManagerConfig from server.core.delay_simulator import DelaySimulator -from server.core.variance_detector import VarianceDetector class ModelLoadError(RuntimeError): @@ -190,46 +188,27 @@ def load_keras_model_for_inference(model_path: str): def track_inference_time(func): """ - This decorator is used to track the execution time of a function. + Log the execution time of a single-layer prediction. + + This is instrumentation only. The authoritative per-layer timings are the + ones `Edge.run_inference` measures and returns; `DeviceStateManager` owns + smoothing them into the EMA tables with the configured alpha. + :param func: the function to be decorated :return: the decorated function """ @wraps(func) def wrapper(self, layer_id: int, layer_offset: int, *args, **kwargs) -> object: - # Start the timer start_time = time.perf_counter() - # Execute the original function (predict_single_layer) result = func(self, layer_id, layer_offset, *args, **kwargs) - # Calculate the elapsed time elapsed_time = time.perf_counter() - start_time - layer_key = f"layer_{layer_id - layer_offset}" # Use layer_X format to match device times - layer_number = ( - layer_id - layer_offset - ) # Keep the numeric value for variance tracking - # Use exponential moving average to smooth times (alpha=0.2 gives 80% weight to history) - if layer_key in self.inference_times: - alpha = 0.2 # Weight for new measurement - self.inference_times[layer_key] = ( - alpha * elapsed_time + (1 - alpha) * self.inference_times[layer_key] - ) - else: - self.inference_times[layer_key] = elapsed_time - logger.debug( - f"Edge Inference for layer [{layer_number}] took {elapsed_time:.4f} seconds (smoothed: {self.inference_times[layer_key]:.4f}s)" + f"Edge Inference for layer [{layer_id - layer_offset}] " + f"took {elapsed_time:.4f} seconds" ) - # Track variance for edge inference times - if hasattr(self, "variance_detector") and self.variance_detector: - self.variance_detector.add_edge_measurement(layer_number, elapsed_time) - - # OPTIMIZATION: Removed per-layer save_inference_times() call. - # Writing to disk after every single layer prediction (up to 59 times per inference) - # was a major I/O bottleneck. Inference times are tracked in-memory via the - # inference_times dict which is shared with RequestHandler.device_profiles. - return result return wrapper @@ -247,48 +226,22 @@ class ModelManager: model_path: The path to the model. num_layers: The number of layers in the model. model: The model. - inference_times: A dictionary to store the inference times for each layer. """ - # def __init__(self, model_dir: str = ModelManagerConfig.MODEL_DIR_PATH, save_path: str = ModelManagerConfig.SAVE_PATH, - # model_path: str = ModelManagerConfig.MODEL_PATH, - # inference_times: dict = {}, computation_delay_config: dict = None, - # variance_detector: VarianceDetector = None): - # self.model_dir = model_dir # <-- aggiunto - # self.save_path = save_path - - # self.model_path = model_path - # self.num_layers = None - # self.model = None - # # dictionary to store inference times for each layer - # self.inference_times = inference_times - # # cache for TFLite interpreters to avoid recreation overhead - # self._interpreter_cache = {} - # # delay simulator for computation - # self.computation_delay = DelaySimulator(computation_delay_config) - # if self.computation_delay.enabled: - # logger.info(f"Computation delay simulation enabled: {self.computation_delay.get_delay_info()}") - # # variance detector for tracking inference time stability - # self.variance_detector = variance_detector - def __init__( self, models_config: dict = None, save_path: str = ModelManagerConfig.SAVE_PATH, - inference_times: dict = {}, computation_delay_config: dict = None, - variance_detector: VarianceDetector = None, ): # Salviamo l'intera configurazione dei modelli (fomo_96, fomo_144, ecc.) self.models_config = models_config self.save_path = save_path - self.inference_times = inference_times # La cache ora è "vuota" all'inizio e si riempirà per ogni modello self._interpreter_cache = {} self.computation_delay = DelaySimulator(computation_delay_config) - self.variance_detector = variance_detector logger.info("ModelManager dinamico inizializzato.") def get_model_layer(self, layer_id: int) -> tf.keras.layers.Layer: @@ -475,21 +428,3 @@ def predict_single_layer( # internal data buffer, which would cause a RuntimeError on the next invoke(). return np.copy(interpreter.get_tensor(output_details[0]["index"])) - def save_inference_times(self, save_path: str | None = None): - """Save the inference times to a JSON file. - Args: - save_path: The path to save the inference times. - Returns: - None - """ - if save_path is not None: - self.save_path = save_path - self.save_path = ( - self.save_path[:-1] if self.save_path[-1] == "/" else self.save_path - ) - inference_times = self.inference_times - try: - with open(OffloadingDataFiles.data_file_path_edge, "w") as f: - json.dump(inference_times, f, indent=4) - except Exception as e: - logger.error(f"Failed to save inference times: {e}") diff --git a/tests/unit/test_edge_inference_time_ownership.py b/tests/unit/test_edge_inference_time_ownership.py new file mode 100644 index 00000000..17e22f8f --- /dev/null +++ b/tests/unit/test_edge_inference_time_ownership.py @@ -0,0 +1,85 @@ +"""Regression tests: DeviceStateManager is the only owner of the edge EMA. + +Edge per-layer times used to be smoothed twice — once inside +`track_inference_time` with a hard-coded alpha=0.2, and again by the request +handler with the configured alpha (#72). `ModelManager` also fed a second, +never-consulted `VarianceDetector` (#73). +""" + +import inspect + +import pytest + +from server.communication.device_state import DeviceStateManager +from server.edge.edge_initialization import Edge +from server.models.model_manager import ModelManager, track_inference_time + + +class FakeModelManager: + """Minimal stand-in exposing the attributes the decorator may touch.""" + + def __init__(self): + self.calls = [] + + @track_inference_time + def predict_single_layer(self, layer_id, layer_offset, *args, **kwargs): + self.calls.append((layer_id, layer_offset)) + return "prediction" + + +def test_track_inference_time_is_pure_instrumentation(): + manager = FakeModelManager() + + result = manager.predict_single_layer(3, 1) + + assert result == "prediction" + assert manager.calls == [(3, 1)] + # The decorator must not smuggle timing state onto the instance: the + # authoritative timings are the ones run_inference returns. + assert not hasattr(manager, "inference_times") + assert not hasattr(manager, "variance_detector") + + +def test_model_manager_no_longer_owns_timing_or_variance_state(): + signature = inspect.signature(ModelManager.__init__) + + assert "inference_times" not in signature.parameters + assert "variance_detector" not in signature.parameters + assert not hasattr(ModelManager, "save_inference_times") + + +def test_edge_has_no_module_level_variance_detector(): + # The detector here accumulated measurements that no decision ever read. + assert not hasattr(Edge, "variance_detector") + + +def test_run_inference_does_not_take_an_ema_table(): + signature = inspect.signature(Edge.run_inference) + + assert "edge_inference_times" not in signature.parameters + + +def test_edge_ema_is_applied_exactly_once_with_the_configured_alpha(): + manager = DeviceStateManager() + manager.register("dev-1", num_layers=2) + + # Seeded at 0.1; a single EMA pass with alpha=0.5 against a 0.5s measurement + # gives 0.5*0.5 + 0.5*0.1 = 0.3. Double smoothing would land lower. + manager.update_edge_times("dev-1", [0.5], start_layer=0, alpha=0.5) + + times = manager.profiles["dev-1"]["edge_inference_times"] + assert times["layer_0"] == pytest.approx(0.3) + + +def test_configured_alpha_reaches_the_edge_ema(): + # Regression for the hard-coded alpha=0.2 that ignored settings.yaml. + manager = DeviceStateManager() + manager.register("dev-1", num_layers=1) + + manager.update_edge_times("dev-1", [0.5], start_layer=0, alpha=1.0) + + # alpha=1.0 means "trust the new measurement fully"; a hard-coded 0.2 would + # have produced 0.18 instead. + assert manager.profiles["dev-1"]["edge_inference_times"]["layer_0"] == ( + pytest.approx(0.5) + )