From bb5d3b03e9b08db56b6632f2038d326320363ee3 Mon Sep 17 00:00:00 2001 From: Carlo van Driesten Date: Wed, 11 Nov 2026 12:52:28 +0100 Subject: [PATCH] fix(eval): resolve carla_cosim benchmarks and lint for unresolved names Selecting the shipped carla_cosim benchmark crashed with UnboundLocalError: the cosim branch of load_benchmark_config appended to resolved_benchmarks, which is not assigned until after the loop it sits in. The parsed cosim entries now accumulate in their own list and are appended to the normalized replay benchmarks, so a selection may mix both kinds. The bug is the kind a linter finds for free, so the ruff hook that has been commented out since it was added is enabled, scoped to exactly that class of defect: names that do not resolve and imports that are never used. The repo has 104 other violations under ruff's full default set, most of them E402 import placement, and fixing those is neither this change nor obviously desirable; a deliberately narrow select keeps the hook green today and lets the set widen later alongside the fixes a wider set would demand. Enabling it removed eight genuinely unused imports and required noqa markers on the four import-availability probes that exist precisely to be unused. tests/unit_tests/test_terminate_on_goal.py called pytest.main without importing pytest, so its __main__ entry point had never worked. --- .pre-commit-config.yaml | 4 ++-- data_utils/download_womd_data.py | 2 +- notebooks/notebook_utils.py | 1 - pufferlib/__init__.py | 4 ++-- pufferlib/ocean/__init__.py | 4 ++-- pufferlib/ocean/cosim/carla_cosim.py | 2 -- .../ocean/evaluation_utils/evaluation_utils.py | 4 +++- .../ocean/evaluation_utils/wosac/estimators.py | 1 - pufferlib/pufferl.py | 2 +- ruff.toml | 5 +++++ scripts/eval/analyze_nuplan_cosim.py | 1 - tests/unit_tests/test_cosim_config_contract.py | 18 ++++++++++++++++++ tests/unit_tests/test_map_metrics.py | 3 --- tests/unit_tests/test_terminate_on_goal.py | 1 + 14 files changed, 35 insertions(+), 17 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d1b138a240..5013845e03 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,8 +11,8 @@ repos: - repo: https://github.com/astral-sh/ruff-pre-commit rev: v0.12.12 hooks: - # - id: ruff - # args: [ --fix ] + - id: ruff-check + args: [ --fix ] - id: ruff-format # Apply clang format - repo: https://github.com/pre-commit/mirrors-clang-format diff --git a/data_utils/download_womd_data.py b/data_utils/download_womd_data.py index 4100c0106a..63bc724bf3 100644 --- a/data_utils/download_womd_data.py +++ b/data_utils/download_womd_data.py @@ -1,5 +1,5 @@ import os -from huggingface_hub import snapshot_download, login +from huggingface_hub import snapshot_download import threading import time diff --git a/notebooks/notebook_utils.py b/notebooks/notebook_utils.py index 05a7091ef3..362368e917 100644 --- a/notebooks/notebook_utils.py +++ b/notebooks/notebook_utils.py @@ -6,7 +6,6 @@ import yaml from pufferlib.ocean.drive.drive import Drive -from pufferlib.ocean.drive import binding from pufferlib.ocean.torch import Drive as DrivePolicy from pufferlib.pufferl import load_config diff --git a/pufferlib/__init__.py b/pufferlib/__init__.py index 95f71d92db..cd9c91bb09 100644 --- a/pufferlib/__init__.py +++ b/pufferlib/__init__.py @@ -15,8 +15,8 @@ sys.stdout = open(os.devnull, "w") sys.stderr = open(os.devnull, "w") try: - import gymnasium - import pygame + import gymnasium # noqa: F401 + import pygame # noqa: F401 except ImportError: pass sys.stdout.close() diff --git a/pufferlib/ocean/__init__.py b/pufferlib/ocean/__init__.py index 55822f8a57..1a9ff8df0d 100644 --- a/pufferlib/ocean/__init__.py +++ b/pufferlib/ocean/__init__.py @@ -1,11 +1,11 @@ from .environment import * try: - import torch + import torch # noqa: F401 except ImportError: pass else: - from .torch import Policy + from .torch import Policy # noqa: F401 try: from .torch import Recurrent diff --git a/pufferlib/ocean/cosim/carla_cosim.py b/pufferlib/ocean/cosim/carla_cosim.py index 9e07b93d6a..ff9120a06b 100644 --- a/pufferlib/ocean/cosim/carla_cosim.py +++ b/pufferlib/ocean/cosim/carla_cosim.py @@ -23,9 +23,7 @@ """ import argparse -import math import xml.etree.ElementTree as ET -from pathlib import Path import numpy as np diff --git a/pufferlib/ocean/evaluation_utils/evaluation_utils.py b/pufferlib/ocean/evaluation_utils/evaluation_utils.py index 7ac45692df..90b4774e6d 100644 --- a/pufferlib/ocean/evaluation_utils/evaluation_utils.py +++ b/pufferlib/ocean/evaluation_utils/evaluation_utils.py @@ -135,12 +135,13 @@ def load_benchmark_config(config_path, selected_names, map_dir_override=None, nu raise pufferlib.APIUsageError(f"Unknown benchmarks: {', '.join(missing_names)}") selected_benchmark_configs = [] + cosim_benchmarks = [] for name in selected_names: benchmark = configured_benchmarks[name] if benchmark.get("simulation_mode") == "carla_cosim": from pufferlib.ocean.evaluation_utils.cosim_evaluator import parse_cosim_benchmark - resolved_benchmarks.append(parse_cosim_benchmark(name, benchmark)) + cosim_benchmarks.append(parse_cosim_benchmark(name, benchmark)) continue if num_scenarios_override is not None: benchmark = {**benchmark, "num_scenarios": num_scenarios_override} @@ -154,6 +155,7 @@ def load_benchmark_config(config_path, selected_names, map_dir_override=None, nu "benchmark_config", validate_resources=False, ) + resolved_benchmarks.extend(cosim_benchmarks) return environment_config, resolved_benchmarks diff --git a/pufferlib/ocean/evaluation_utils/wosac/estimators.py b/pufferlib/ocean/evaluation_utils/wosac/estimators.py index bee1860c9a..0e90c770d4 100644 --- a/pufferlib/ocean/evaluation_utils/wosac/estimators.py +++ b/pufferlib/ocean/evaluation_utils/wosac/estimators.py @@ -2,7 +2,6 @@ import numpy as np import matplotlib.pyplot as plt -from typing import Optional np.set_printoptions(suppress=True) diff --git a/pufferlib/pufferl.py b/pufferlib/pufferl.py index 588a32e260..e054327bc1 100644 --- a/pufferlib/pufferl.py +++ b/pufferlib/pufferl.py @@ -56,7 +56,7 @@ try: - from pufferlib import _C + from pufferlib import _C # noqa: F401 except ImportError: raise ImportError( "Failed to import C/CUDA advantage kernel. If you have non-default PyTorch, try installing with --no-build-isolation" diff --git a/ruff.toml b/ruff.toml index f109b17cb5..94dc33b1e1 100644 --- a/ruff.toml +++ b/ruff.toml @@ -1,6 +1,11 @@ line-length = 120 target-version = "py310" +[lint] +# Narrow, zero-false-positive set: names that do not resolve and imports that +# are never used. Widen only together with the fixes a wider set demands. +select = ["E9", "F401", "F811", "F821", "F822"] + [lint.isort] lines-after-imports = 2 split-on-trailing-comma = false diff --git a/scripts/eval/analyze_nuplan_cosim.py b/scripts/eval/analyze_nuplan_cosim.py index 8c86b65e6b..da2e2132f6 100644 --- a/scripts/eval/analyze_nuplan_cosim.py +++ b/scripts/eval/analyze_nuplan_cosim.py @@ -26,7 +26,6 @@ from nuplan.common.actor_state.state_representation import Point2D # noqa: E402 from nuplan.common.maps.maps_datatypes import SemanticMapLayer # noqa: E402 -from nuplan.planning.simulation.simulation_log import SimulationLog # noqa: E402 sys.path.insert(0, str(Path(__file__).resolve().parent)) import nuplan_log_remap # noqa: E402 logs written on another machine (cluster paths, planner classes) diff --git a/tests/unit_tests/test_cosim_config_contract.py b/tests/unit_tests/test_cosim_config_contract.py index 08179accfd..399f63d96b 100644 --- a/tests/unit_tests/test_cosim_config_contract.py +++ b/tests/unit_tests/test_cosim_config_contract.py @@ -196,5 +196,23 @@ def test_defaults_only_fill_gaps(self): self.assertEqual(kwargs["num_goals"], 3) +class TestCosimBenchmarkResolution(unittest.TestCase): + def test_shipped_carla_cosim_benchmark_resolves(self): + """Selecting a cosim benchmark must return it alongside the replay ones. + + The cosim branch of load_benchmark_config appended to the result list + before that list existed, so every selection containing carla_cosim + died with UnboundLocalError. + """ + from pufferlib.ocean.evaluation_utils.evaluation_utils import load_benchmark_config + + benchmark_yaml = REPO_ROOT / "pufferlib" / "config" / "evaluation" / "benchmark.yaml" + _, benchmarks = load_benchmark_config(str(benchmark_yaml), "carla_cosim") + + self.assertEqual([b["name"] for b in benchmarks], ["carla_cosim"]) + self.assertEqual(benchmarks[0]["simulation_mode"], "carla_cosim") + self.assertEqual(benchmarks[0]["route_ids"], [0, 1, 2]) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit_tests/test_map_metrics.py b/tests/unit_tests/test_map_metrics.py index d3b2a2c5c6..3ea0d50432 100644 --- a/tests/unit_tests/test_map_metrics.py +++ b/tests/unit_tests/test_map_metrics.py @@ -4,7 +4,6 @@ import numpy as np import matplotlib.pyplot as plt from matplotlib.patches import Rectangle -from matplotlib.transforms import Affine2D import torch from pufferlib.ocean.evaluation_utils.wosac import map_metric_features @@ -130,8 +129,6 @@ def plot_test_cases(): inner_y = [1, 3, 3, 1, 1] ax.plot(inner_x, inner_y, "b-", linewidth=2) # Fill road area - from matplotlib.patches import Polygon - outer_poly = np.array([[0, 0], [4, 0], [4, 4], [0, 4]]) inner_poly = np.array([[1, 1], [1, 3], [3, 3], [3, 1]]) ax.fill(outer_x, outer_y, alpha=0.2, color="green") diff --git a/tests/unit_tests/test_terminate_on_goal.py b/tests/unit_tests/test_terminate_on_goal.py index de6b3ce99b..8ce925b167 100644 --- a/tests/unit_tests/test_terminate_on_goal.py +++ b/tests/unit_tests/test_terminate_on_goal.py @@ -20,6 +20,7 @@ import os import numpy as np +import pytest from pufferlib.ocean.drive.drive import Drive