diff --git a/openpilot/sunnypilot/models/NAVIGATION_MODEL.md b/openpilot/sunnypilot/models/NAVIGATION_MODEL.md index ce10bda231..884d35b3b1 100644 --- a/openpilot/sunnypilot/models/NAVIGATION_MODEL.md +++ b/openpilot/sunnypilot/models/NAVIGATION_MODEL.md @@ -21,6 +21,11 @@ storage. Missing device dependencies produce an error; this feature does not install system libraries. The compiler log is `Paths.model_root()/rl_nav_80eedcdd_build.log`. An interrupted verified download can be reused on the next attempt; incomplete downloads are never accepted. +If a completed compilation fails protocol validation, its artifact, descriptor +and `validation_failure.json` are retained in the single +`Paths.model_root()/.rl_nav_failed_build` directory for diagnosis. A later rejected +build replaces that directory. It is outside normal model registration and is +never automatically selected or reused as a validated model. ## Exact checkpoint and build diff --git a/openpilot/sunnypilot/models/build_navigation_model.py b/openpilot/sunnypilot/models/build_navigation_model.py index 8d8de00b3a..8378966d88 100644 --- a/openpilot/sunnypilot/models/build_navigation_model.py +++ b/openpilot/sunnypilot/models/build_navigation_model.py @@ -19,12 +19,38 @@ from openpilot.sunnypilot.models.navigation_model import (MANIFEST_NAME, SOURCE_ COMPILE_ENV = {'DEBUG': '1', 'DEV': 'USB+AMD:LLVM', 'FRAME_DEV': 'CPU', 'FLOAT16': '1', 'GMMU': '0', 'TC_OPT': '2', 'TC_MIN_GLOBALS': '32'} COMPILER_SHA256 = '0ff7415619aa267914f858c24b3f3c32583f75aceebd89ec4f56e70e9daf32ac' +FAILED_BUILD_DIRECTORY = '.rl_nav_failed_build' class PreparationCancelled(RuntimeError): pass +def preserve_failed_build(temporary, root, reason): + """Keep one completed rejected build for inspection, outside the selector root.""" + diagnostic = {'reason': reason, 'sourceOnnxSha256': SOURCE_ONNX_SHA256, 'compilerSha256': COMPILER_SHA256, + 'registered': False, 'automaticReuseAllowed': False} + try: + descriptor = json.loads((temporary / MANIFEST_NAME).read_text()) + artifact = temporary / descriptor['artifact']['fileName'] + diagnostic['artifact'] = descriptor['artifact'] | {'bytes': artifact.stat().st_size} + from openpilot.sunnypilot.modeld_v2.helpers import read_parts + _parts, extras = read_parts(str(artifact)) + metadata = {key: extras.get(key) for key in ('input_specs', 'output_specs', 'metadata')} + encoded = json.dumps(metadata, default=repr) + diagnostic['compiledMetadata'] = metadata if len(encoded) <= 65536 else {'truncated': True, 'head': encoded[:65536]} + except Exception as error: + diagnostic['diagnosticReadError'] = str(error)[:2048] + (temporary / 'validation_failure.json').write_text(json.dumps(diagnostic, indent=2, default=repr) + '\n') + failed = root / FAILED_BUILD_DIRECTORY + if failed.is_symlink(): + failed.unlink() + elif failed.exists(): + shutil.rmtree(failed) + temporary.replace(failed) + return failed + + def check_allowed(params, queued): if not params.get_bool('IsOffroad'): raise PreparationCancelled('Stopped because the device is not confirmed offroad. Park before preparing again.') @@ -145,7 +171,9 @@ def prepare(root, basedir, params, hardware, *, queued=False, onnx=None, report= output.rename(temporary / name) (temporary / MANIFEST_NAME).write_text(json.dumps(descriptor, indent=2) + '\n') if installed_bundle(temporary) is None: - raise RuntimeError('Compiled model failed its plain-format/navigation protocol checks. No model was registered.') + reason = 'Compiled model failed its plain-format/navigation protocol checks.' + failed = preserve_failed_build(temporary, root, reason) + raise RuntimeError(f'{reason} No model was registered. Completed build and validation details kept in {failed.name}; see the build log.') check() (temporary / name).replace(root / name) check() diff --git a/openpilot/sunnypilot/models/navigation_model.py b/openpilot/sunnypilot/models/navigation_model.py index 2749e1ac1f..deeeade35b 100644 --- a/openpilot/sunnypilot/models/navigation_model.py +++ b/openpilot/sunnypilot/models/navigation_model.py @@ -67,6 +67,9 @@ def compatible_metadata(extras): if tuple(outputs[name]) != shape or tuple(actual[0]) != shape or np.dtype(actual[1]) != dtype or 'AMD' not in actual[2]: return False encoded = extras['metadata']['metadata']['output_slices'] + # Upstream ONNX metadata uses wrapped Base64. Permit only ASCII whitespace + # around its encoded bytes; keep strict decoding and restricted unpickling. + encoded = encoded.translate(str.maketrans('', '', ' \t\r\n\v\f')) slices = _SliceMetadata(io.BytesIO(base64.b64decode(encoded, validate=True))).load() required = {'plan': slice(917, 1907), 'action': slice(2062, 2066), 'desire_state': slice(2054, 2062), 'desire_pred': slice(855, 887)} diff --git a/openpilot/sunnypilot/models/tests/fixtures/rl_nav_80eedcdd_metadata.json b/openpilot/sunnypilot/models/tests/fixtures/rl_nav_80eedcdd_metadata.json new file mode 100644 index 0000000000..1f1d5e722f --- /dev/null +++ b/openpilot/sunnypilot/models/tests/fixtures/rl_nav_80eedcdd_metadata.json @@ -0,0 +1,264 @@ +{ + "provenance": { + "sourceRepository": "commaai/openpilot", + "sourceRef": "afbf2e547e9319e2eb9f515e1e4330af63705cec", + "sourceModelPath": "openpilot/selfdrive/modeld/models/big_driving_supercombo.onnx", + "officialOnnxSha256": "1d18c42ea4cd95c8bffab4635844b78b8e2d4e25b9de44a9c5ac2e0961371a2e", + "officialOnnxBytes": 814591106, + "sourceCheckpoint": "cf8f6120-49ee-48f8-b2d7-d663a4708271/56320/80eedcdd-f97b-46d0-9b94-780685292acf/12864", + "extractedIoMetadataSha256": "10b55d548b2f662f214c5e7c52c5e7864b18675b295ac51064661dd207e93e39", + "scope": "Graph declarations and literal metadata only; no model weights or executable pickle." + }, + "input_specs": { + "new_img": [ + [ + 2, + 6, + 128, + 256 + ], + "uint8", + "AMD" + ], + "desire": [ + [ + 8 + ], + "float32", + "AMD" + ], + "traffic_convention": [ + [ + 1, + 2 + ], + "float32", + "AMD" + ], + "action_t": [ + [ + 1, + 2 + ], + "float32", + "AMD" + ], + "nav_map": [ + [ + 1, + 1, + 256, + 256 + ], + "float32", + "AMD" + ], + "navigation": [ + [ + 1, + 47 + ], + "float32", + "AMD" + ], + "state_img_q": [ + [ + 2, + 5, + 6, + 128, + 256 + ], + "uint8", + "AMD" + ], + "state_desire_q": [ + [ + 132, + 1, + 8 + ], + "float32", + "AMD" + ], + "state_feat_q": [ + [ + 128, + 1, + 16384 + ], + "float32", + "AMD" + ], + "state_map_features_q": [ + [ + 128, + 1, + 64 + ], + "float32", + "AMD" + ], + "state_navigation_features_q": [ + [ + 128, + 1, + 64 + ], + "float32", + "AMD" + ] + }, + "output_specs": { + "outputs": [ + [ + 1, + 18580 + ], + "float32", + "AMD" + ], + "next_state_img_q": [ + [ + 2, + 5, + 6, + 128, + 256 + ], + "uint8", + "AMD" + ], + "next_state_desire_q": [ + [ + 132, + 1, + 8 + ], + "float32", + "AMD" + ], + "next_state_feat_q": [ + [ + 128, + 1, + 16384 + ], + "float32", + "AMD" + ], + "next_state_map_features_q": [ + [ + 128, + 1, + 64 + ], + "float32", + "AMD" + ], + "next_state_navigation_features_q": [ + [ + 128, + 1, + 64 + ], + "float32", + "AMD" + ] + }, + "metadata": { + "metadata": { + "output_slices": "gASV0gEAAAAAAAB9lCiMCmxhbmVfbGluZXOUjAhidWlsdGluc5SMBXNsaWNllJOUSwBNEAJOh5RS\nlIwPbGFuZV9saW5lc19wcm9ilGgETRACTRgCToeUUpSMCnJvYWRfZWRnZXOUaARNGAJNIANOh5RS\nlIwEbWV0YZRoBE0gA01XA06HlFKUjAtkZXNpcmVfcHJlZJRoBE1XA013A06HlFKUjARwb3NllGgE\nTXcDTYMDToeUUpSMFndpZGVfZnJvbV9kZXZpY2VfZXVsZXKUaARNgwNNiQNOh5RSlIwOcm9hZF90\ncmFuc2Zvcm2UaARNiQNNlQNOh5RSlIwEcGxhbpRoBE2VA01zB06HlFKUjARsZWFklGgETXMHTQMI\nToeUUpSMCWxlYWRfcHJvYpRoBE0DCE0GCE6HlFKUjAxkZXNpcmVfc3RhdGWUaARNBghNDghOh5RS\nlIwGYWN0aW9ulGgETQ4ITRIIToeUUpSMDGhpZGRlbl9zdGF0ZZRoBE0SCE0SSE6HlFKUjAxtYXBf\nZmVhdHVyZXOUaARNEkhNUkhOh5RSlIwTbmF2aWdhdGlvbl9mZWF0dXJlc5RoBE1SSE2SSE6HlFKU\njANwYWSUaARNkkhNlEhOh5RSlHUu\n", + "model_checkpoint": "cf8f6120-49ee-48f8-b2d7-d663a4708271/56320/80eedcdd-f97b-46d0-9b94-780685292acf/12864" + }, + "input_shapes": { + "new_img": [ + 2, + 6, + 128, + 256 + ], + "desire": [ + 8 + ], + "traffic_convention": [ + 1, + 2 + ], + "action_t": [ + 1, + 2 + ], + "nav_map": [ + 1, + 1, + 256, + 256 + ], + "navigation": [ + 1, + 47 + ], + "state_img_q": [ + 2, + 5, + 6, + 128, + 256 + ], + "state_desire_q": [ + 132, + 1, + 8 + ], + "state_feat_q": [ + 128, + 1, + 16384 + ], + "state_map_features_q": [ + 128, + 1, + 64 + ], + "state_navigation_features_q": [ + 128, + 1, + 64 + ] + }, + "output_shapes": { + "outputs": [ + 1, + 18580 + ], + "next_state_img_q": [ + 2, + 5, + 6, + 128, + 256 + ], + "next_state_desire_q": [ + 132, + 1, + 8 + ], + "next_state_feat_q": [ + 128, + 1, + 16384 + ], + "next_state_map_features_q": [ + 128, + 1, + 64 + ], + "next_state_navigation_features_q": [ + 128, + 1, + 64 + ] + } + } +} diff --git a/openpilot/sunnypilot/models/tests/test_navigation_model.py b/openpilot/sunnypilot/models/tests/test_navigation_model.py index e03e50573f..5cb6595a91 100644 --- a/openpilot/sunnypilot/models/tests/test_navigation_model.py +++ b/openpilot/sunnypilot/models/tests/test_navigation_model.py @@ -1,12 +1,17 @@ """Local selector entries require an installed verified target artifact.""" import ast import base64 +import builtins import hashlib import importlib.util import json +import io +import mmap from pathlib import Path import pickle import sys +import struct +import subprocess from tempfile import TemporaryDirectory from types import SimpleNamespace import unittest @@ -132,6 +137,87 @@ class LocalNavigationModelTests(unittest.TestCase): self.assertIsNone(nav.installed_bundle(self.root)) +def real_navigation_metadata(): + fixture = json.loads((SOURCE / 'tests/fixtures/rl_nav_80eedcdd_metadata.json').read_text()) + for name, value in fixture['input_specs'].items(): + fixture['input_specs'][name] = (tuple(value[0]), nav.np.dtype(value[1]).str, value[2]) + for name, value in fixture['output_specs'].items(): + fixture['output_specs'][name] = (tuple(value[0]), nav.np.dtype(value[1]).name, value[2]) + for kind in ('input_shapes', 'output_shapes'): + fixture['metadata'][kind] = {name: tuple(shape) for name, shape in fixture['metadata'][kind].items()} + return fixture + + +def plain_io_functions(): + """Execute the exact plain writer/reader definitions, with no GPU imports.""" + path = SOURCE.parents[0] / 'modeld_v2/helpers.py' + text = path.read_text() if path.exists() else subprocess.check_output( + ['git', '-C', str(SOURCE.parents[2]), 'show', 'HEAD:openpilot/sunnypilot/modeld_v2/helpers.py'], text=True) + names = {'Blob', '_MetadataUnpickler', '_relocate', 'write_bundle', 'read_parts'} + constants = {'MAGIC', 'ALIGNMENT', 'METADATA_TYPES'} + selected = [] + for node in ast.parse(text).body: + if isinstance(node, (ast.FunctionDef, ast.ClassDef)) and node.name in names: + selected.append(node) + elif isinstance(node, ast.Assign) and any(isinstance(t, ast.Name) and t.id in constants for t in node.targets): + selected.append(node) + namespace = {'hashlib': hashlib, 'builtins': builtins, 'pickle': pickle, 'struct': struct, 'json': json, + 'io': io, 'mmap': mmap} + exec(compile(ast.Module(body=selected, type_ignores=[]), str(path), 'exec'), namespace) + return namespace['write_bundle'], namespace['read_parts'] + + +class RealNavigationMetadataTests(unittest.TestCase): + def test_exact_official_wrapped_metadata_survives_actual_plain_roundtrip(self): + fixture = real_navigation_metadata() + self.assertEqual(fixture['provenance']['officialOnnxSha256'], nav.SOURCE_ONNX_SHA256) + self.assertEqual(fixture['provenance']['officialOnnxBytes'], nav.SOURCE_ONNX_BYTES) + self.assertEqual(fixture['metadata']['metadata']['output_slices'].count('\n'), 9) + write_bundle, read_parts = plain_io_functions() + with TemporaryDirectory() as directory: + path = Path(directory) / 'real_metadata_plain_fixture.pkl' + # An empty, non-executable run part is sufficient to test the actual format. + write_bundle(path, {'run': ({'calls': [], 'buffers': []}, b'')}, fixture) + parts, decoded = read_parts(path) + self.assertIn('run', parts) + self.assertEqual(decoded, fixture) + self.assertTrue(nav.compatible_metadata(decoded)) + (Path(directory) / nav.MANIFEST_NAME).write_text(json.dumps({'version': 1, 'target': 'chestnut', + 'sourceOnnxSha256': nav.SOURCE_ONNX_SHA256, + 'artifact': {'fileName': path.name, 'sha256': nav.file_sha256(path)}})) + with patch.dict(sys.modules, {'openpilot.sunnypilot.modeld_v2.helpers': SimpleNamespace(read_parts=read_parts)}): + self.assertIsNotNone(nav.installed_bundle(directory)) + + def test_ascii_wrapping_is_allowed_but_corrupt_base64_is_rejected(self): + fixture = real_navigation_metadata() + original = fixture['metadata']['metadata']['output_slices'] + compact = ''.join(original.split()) + fixture['metadata']['metadata']['output_slices'] = ' \t\r\n'.join(compact[i:i+16] for i in range(0, len(compact), 16)) + self.assertTrue(nav.compatible_metadata(fixture)) + for encoded in (compact[:8] + '!' + compact[8:], compact[:-1], compact[:8] + '\u00a0' + compact[8:]): + with self.subTest(encoded=encoded[:20]): + fixture['metadata']['metadata']['output_slices'] = encoded + self.assertFalse(nav.compatible_metadata(fixture)) + + def test_wrapping_does_not_permit_executable_or_wrong_slice_metadata(self): + class ExecutableMetadata: + def __reduce__(self): + return builtins.eval, ('1 + 1',) + fixture = real_navigation_metadata() + payload = base64.encodebytes(pickle.dumps(ExecutableMetadata())).decode('ascii') + fixture['metadata']['metadata']['output_slices'] = payload + with patch.object(builtins, 'eval', side_effect=AssertionError('Metadata must never execute')) as execute: + self.assertFalse(nav.compatible_metadata(fixture)) + execute.assert_not_called() + original = real_navigation_metadata()['metadata']['metadata']['output_slices'] + slices = nav._SliceMetadata(io.BytesIO(base64.b64decode(original))).load() + slices['action'] = slice(2062, 2070) + fixture['metadata']['metadata']['output_slices'] = base64.encodebytes(pickle.dumps(slices)).decode('ascii') + self.assertFalse(nav.compatible_metadata(fixture)) + fixture['metadata']['metadata']['output_slices'] = base64.encodebytes(pickle.dumps([])).decode('ascii') + self.assertFalse(nav.compatible_metadata(fixture)) + + class CatalogHookTests(unittest.TestCase): def test_cached_offline_model_is_selectable_without_changing_remote_cache(self): tree = ast.parse((SOURCE / 'fetcher.py').read_text()) diff --git a/openpilot/sunnypilot/models/tests/test_navigation_preparation.py b/openpilot/sunnypilot/models/tests/test_navigation_preparation.py index 05f5d39e2c..6c0cac6a62 100644 --- a/openpilot/sunnypilot/models/tests/test_navigation_preparation.py +++ b/openpilot/sunnypilot/models/tests/test_navigation_preparation.py @@ -168,6 +168,26 @@ class BuildLifecycleTests(unittest.TestCase): self.run_build(lambda _path: None) self.assertFalse((self.root / nav.MANIFEST_NAME).exists()) + def test_rejected_completed_build_is_preserved_once_without_normal_registration(self): + failed = self.root / builder.FAILED_BUILD_DIRECTORY + for attempt in range(2): + if attempt: + (failed / 'old-build-marker').write_text('must be replaced') + with self.assertRaisesRegex(RuntimeError, 'Completed build and validation details kept'): + self.run_build(lambda _path: None) + descriptor = json.loads((failed / nav.MANIFEST_NAME).read_text()) + artifact = failed / descriptor['artifact']['fileName'] + self.assertEqual(artifact.read_bytes(), b'target-plain-model') + self.assertEqual(nav.file_sha256(artifact), descriptor['artifact']['sha256']) + diagnostic = json.loads((failed / 'validation_failure.json').read_text()) + self.assertFalse(diagnostic['registered']) + self.assertFalse(diagnostic['automaticReuseAllowed']) + self.assertEqual(diagnostic['artifact']['sha256'], descriptor['artifact']['sha256']) + self.assertFalse((failed / 'old-build-marker').exists()) + self.assertFalse((self.root / nav.MANIFEST_NAME).exists()) + self.assertIsNone(nav.installed_bundle(self.root)) + self.assertEqual([path.name for path in self.root.glob('.rl_nav_failed*')], [builder.FAILED_BUILD_DIRECTORY]) + def test_onroad_before_publication_never_registers(self): def process(*args, **kwargs): result = self.process(*args, **kwargs)