From 0098beaf25f78167e7ccac18c6cf344dda65fbd8 Mon Sep 17 00:00:00 2001 From: whoisdomi Date: Thu, 17 Sep 2026 17:58:00 -0500 Subject: [PATCH] is small worth it --- selfdrive/modeld/modeld.py | 41 ++++++++++--- selfdrive/modeld/tests/test_usbgpu_helpers.py | 61 ++++++++++++++++++- 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/selfdrive/modeld/modeld.py b/selfdrive/modeld/modeld.py index 1be68efb10..bd1ee09643 100755 --- a/selfdrive/modeld/modeld.py +++ b/selfdrive/modeld/modeld.py @@ -119,6 +119,11 @@ BIG_MODEL_LOADER_CORES = {0, 1, 2, 3} # forever while the small model quietly keeps driving. BIG_MODEL_LOAD_TIMEOUT_SECONDS = 150.0 +# A transient GPU queue wedge should cost frames, not the process: restarting modeld blinds +# openpilot for ~35 s. Keep dying available for a model that is genuinely broken, though -- +# at 20 Hz this is a second of continuous failure. +MAX_CONSECUTIVE_MODEL_FAILURES = 20 + class BigModelLoadCancelled(Exception): """Raised inside the background loader when modeld no longer wants the big model.""" @@ -962,6 +967,11 @@ class BigModelLoader: if self._cancel.is_set(): raise BigModelLoadCancelled("cancelled before the external GPU link check") wait_usbgpu_link() + # Transferring weights over USB legitimately outlasts the run-time watchdog, so raise + # it for the load and put it back in the finally below. This is process-global and + # shared with QCOM, so the window is kept as short as possible: leaving it at 30 s + # meant a wedged *small* model took 30 s to fail instead of 3 s. + _set_hcq_wait_timeout(BIG_MODEL_LOAD_WAIT_TIMEOUT_MS) candidate = ModelState( self.cam_w, self.cam_h, @@ -991,7 +1001,9 @@ class BigModelLoader: with self._lock: self._error = str(exc) or exc.__class__.__name__ finally: - # tinygrad's disk cache handle is thread-local, so this closes only the loader's. + # Restore the short watchdog so a wedged small model fails fast for the rest of the + # drive, and close this thread's tinygrad disk cache handle (it is thread-local). + _set_hcq_wait_timeout(BIG_MODEL_RUN_WAIT_TIMEOUT_MS) _close_tinygrad_disk_cache_connection() @@ -1170,12 +1182,6 @@ def main(demo=False): external_artifact = MODELS_PATH / f"{big_model_id}_driving_tinygrad.pkl" external_artifact_ready = external_model_selected and file_chunked_exists(external_artifact) external_gpu_requested = usbgpu_present_now and (bool(big_model_id) or model_lab_ready) - # The big model is loaded in the background while the small model drives, so the HCQ - # watchdog is set once here and never mutated again: it is process-global and shared by - # the QCOM and AMD devices, so changing it mid-drive would also move the small model's. - if external_gpu_requested: - _set_hcq_wait_timeout(BIG_MODEL_LOAD_WAIT_TIMEOUT_MS) - params.put_bool("UsbGpuPresent", usbgpu_present_now) params.put_bool("UsbGpuCompiled", external_artifact_ready or model_lab_ready) params.put_bool("UsbGpuActive", False) @@ -1324,6 +1330,7 @@ def main(demo=False): frame_id = 0 last_vipc_frame_id = 0 run_count = 0 + consecutive_model_failures = 0 model_transform_main = np.zeros((3, 3), dtype=np.float32) model_transform_extra = np.zeros((3, 3), dtype=np.float32) @@ -1600,12 +1607,24 @@ def main(demo=False): config=model_lab_config, error=model_lab_error, ) - else: - if not external_gpu_active or small_model is None: - raise + elif external_gpu_active and small_model is not None: cloudlog.exception("external GPU model failed, falling back to active small model") model = small_model big_model = None + else: + # Already on the small model, so there is nothing to fall back to. A GPU queue can + # wedge transiently (measured: a QCOM timeline timeout while a background load was + # warming up), and dying here costs ~35 s of restart during which nothing can + # engage. Drop the frame and let the next one retry instead. + cloudlog.exception("model inference failed; dropping this frame and retrying") + consecutive_model_failures += 1 + if consecutive_model_failures > MAX_CONSECUTIVE_MODEL_FAILURES: + raise + # Nothing changed models, so leave the runtime state alone and just skip this frame. + # Keep the frame bookkeeping up to date or the next frame is counted as a drop too. + last_vipc_frame_id = meta_main.frame_id + model_output = None + continue # A failed big model is not retried in this drive, so stop any load still in flight. if big_loader is not None: big_loader.cancel() @@ -1621,6 +1640,8 @@ def main(demo=False): chestnut_state.big = False run_count = 0 model_output = None + else: + consecutive_model_failures = 0 mt2 = time.perf_counter() model_execution_time = mt2 - mt1 diff --git a/selfdrive/modeld/tests/test_usbgpu_helpers.py b/selfdrive/modeld/tests/test_usbgpu_helpers.py index 86046ca2c4..5f73cb2969 100644 --- a/selfdrive/modeld/tests/test_usbgpu_helpers.py +++ b/selfdrive/modeld/tests/test_usbgpu_helpers.py @@ -296,10 +296,11 @@ def test_background_big_model_load_leaves_the_running_model_untouched(monkeypatc calls = [] fake_model_state = _stub_big_model_loader(monkeypatch, calls) # The small model is already driving on these process-global settings, so the background - # load must not touch tinygrad's DEV, its HCQ watchdog, or its shared buffer UOp cache. + # load must not touch tinygrad's DEV or its shared buffer UOp cache. The HCQ watchdog is + # the deliberate exception: see test_hcq_watchdog_is_raised_only_around_the_background_load. + monkeypatch.setattr(modeld, "_set_hcq_wait_timeout", lambda timeout: calls.append(("timeout", timeout))) for name, detail in ( ("tinygrad_dev_config", "runtime must not change tinygrad's process-global DEV"), - ("_set_hcq_wait_timeout", "the background load must not move the running model's HCQ watchdog"), ("_isolate_next_model_artifact_load", "the background load must not evict the running model's buffers"), ): monkeypatch.setattr(modeld, name, lambda *_args, _d=detail: (_ for _ in ()).throw(AssertionError(_d))) @@ -317,8 +318,10 @@ def test_background_big_model_load_leaves_the_running_model_untouched(monkeypatc ("affinity", tuple(sorted(modeld.BIG_MODEL_LOADER_CORES))), ("power", "car-params"), "link", + ("timeout", modeld.BIG_MODEL_LOAD_WAIT_TIMEOUT_MS), ("model", 1928, 1208, True, "big-model", False, "v15"), "warmup", + ("timeout", modeld.BIG_MODEL_RUN_WAIT_TIMEOUT_MS), "close_cache", ] @@ -392,6 +395,60 @@ def test_big_model_warmup_stays_on_the_background_loader(): "promotion must not warm the big model; it runs inside the 20 Hz publish loop" +def test_hcq_watchdog_is_raised_only_around_the_background_load(): + """The HCQ watchdog is process-global and shared with QCOM. + + Leaving it at the 30 s load value for the whole drive meant a wedged *small* model took + 30 s to fail instead of 3 s. Drive 00000ad4: modelV2 stopped at 34.3 s, the loader's + warmup blocked behind the same QCOM queue, and both hit the 30 s timeout together -- + 68 s with no model output. Raise it inside the loader and restore it in the finally. + """ + import ast + from pathlib import Path + + source = (Path(modeld.__file__).with_name("modeld.py")).read_text(encoding="utf-8") + tree = ast.parse(source) + + main_fn = next(n for n in tree.body if isinstance(n, ast.FunctionDef) and n.name == "main") + assert "_set_hcq_wait_timeout" not in ast.dump(main_fn), \ + "main() must not pin the watchdog for the whole drive" + + run = next(n for n in ast.walk(tree) if isinstance(n, ast.FunctionDef) and n.name == "_run") + handler = next(n for n in ast.walk(run) if isinstance(n, ast.Try) and n.finalbody) + assert "BIG_MODEL_RUN_WAIT_TIMEOUT_MS" in ast.dump(ast.Module(body=handler.finalbody, type_ignores=[])), \ + "the loader must restore the short watchdog when it finishes" + + +def test_a_wedged_small_model_drops_frames_instead_of_killing_modeld(): + """Restarting modeld blinds openpilot for ~35 s, which is worse than losing frames. + + Drive 00000ad4 died on `RuntimeError: Wait timeout: 30000 ms!` from the QCOM timeline + while already running the small model, so there was no model to fall back to and the + exception propagated out of main(). + """ + import ast + from pathlib import Path + + source = (Path(modeld.__file__).with_name("modeld.py")).read_text(encoding="utf-8") + main_fn = next(n for n in ast.parse(source).body + if isinstance(n, ast.FunctionDef) and n.name == "main") + handler = next( + h for n in ast.walk(main_fn) if isinstance(n, ast.Try) + for h in n.handlers if "small_model" in ast.dump(h) + ) + # The handler is `if model_lab_active: ... elif external_gpu_active: ... else: ...`, so the + # no-fallback branch is the else of the nested elif. + branch = next(n for n in ast.walk(handler) + if isinstance(n, ast.If) and "external_gpu_active" in ast.dump(n.test)) + assert branch.orelse, "expected an else covering the case with no model to fall back to" + fallback = ast.dump(ast.Module(body=branch.orelse, type_ignores=[])) + assert "consecutive_model_failures" in fallback, "a transient wedge must not be fatal" + assert "Continue" in fallback, "the frame must be skipped rather than the process dying" + # Still die for a model that is genuinely broken rather than looping forever. + assert "Raise" in fallback + assert modeld.MAX_CONSECUTIVE_MODEL_FAILURES <= modeld.ModelConstants.MODEL_FREQ * 2 + + def test_chestnut_telemetry_is_suppressed_while_a_background_load_runs(): """Telemetry shares Chestnut's USB device with the model weight transfer.