Fix the debugging searches that pruned work they never checked

coreai_torch/debugging/benchmarker.py

@@ -123,6 +123,10 @@ class _BenchmarkerState(Enum):
     """Benchmark completed."""
 
 
+_NO_INTERVAL = 0
+"""Interval id standing for "this begin event opened nothing"."""
+
+
 @dataclass(frozen=True)
 class Statistics:
     """Statistical summary of measurements."""
@@ -522,11 +526,6 @@ def _timing_annotation_callback(
     figure to give, and annotating each one repeated a single measurement as many
     times as the dispatch had members.
 
-    Grouped rather than indexed, because a representative is not unique. Two
-    dispatches regularly cover the same operation, and ``{timing.op_id: timing}``
-    kept only the last -- 22 annotations from 34 dispatches on one model, silently,
-    with the survivor chosen by insertion order.
-
     Args:
         timings: The dispatches to report.
 
@@ -1302,7 +1301,7 @@ def __init__(
         self._timings: dict[tuple[int, tuple[int, ...]], list[float]] = defaultdict(
             list
         )
-        self._interval_counter = 0
+        self._interval_counter = _NO_INTERVAL + 1
         self._debug_info_records: list[DebugInfoRecord] = []
         # Maps a compiled op identifier (odix_id, delegate_id) to the list of
         # coreai op IDs fused into it. Keyed on the same pair carried by a runtime
@@ -1386,7 +1385,7 @@ def _reset_state(self: Self) -> None:
             self._intervals.clear()
             self._timings.clear()
             self._pending_durations.clear()
-            self._interval_counter = 0
+            self._interval_counter = _NO_INTERVAL + 1
 
     def _wait_for_intervals_to_complete(self: Self, timeout_s: float = 10.0) -> None:
         """
@@ -1563,23 +1562,24 @@ def _on_log_event_begin(self: Self, event: LogEvent) -> int:
             event: LogEvent from the profiler
 
         Returns:
-            Interval ID for tracking this event
+            Interval ID for tracking this event, or :data:`_NO_INTERVAL` when this
+            event is not being timed.
 
         """
         # Only process events when actively running benchmark
         if self._state != _BenchmarkerState.RUNNING:
-            return 0  # Return dummy interval_id
+            return _NO_INTERVAL
 
         # Only process inference phase events
         phase = _LogEventPhase(event.phase)
         if phase != _LogEventPhase.INFERENCE:
-            return 0  # Return dummy interval_id for non-inference events
+            return _NO_INTERVAL
 
         # Opening an interval for the host-timestamped twin would pool it with the
         # hardware measurement of the same encoder; its end event then finds no open
         # interval and is ignored.
         if _is_gpu_interval(event) and not _is_hardware_timestamped(event):
-            return 0
+            return _NO_INTERVAL
 
         with self._lock:
             interval_id = self._interval_counter
@@ -1602,6 +1602,12 @@ def _on_log_event_end(self: Self, event: LogEvent, interval_id: int) -> None:
             interval_id: Interval ID from the begin callback
 
         """
+        # Nothing was opened for this event, so there is nothing to close. Checked
+        # before the lookup: the sentinel must never reach `self._intervals`, where it
+        # would find whichever interval happened to be open under that id.
+        if interval_id == _NO_INTERVAL:
+            return
+
         # Only process events when actively running benchmark
         if self._state != _BenchmarkerState.RUNNING:
             return
@@ -1819,8 +1825,11 @@ async def benchmark(
             logger.debug("Warmup run (untimed)")
             await function(nd_inputs)
 
-            # Transition to RUNNING state to start collecting timing data
-            self._state = _BenchmarkerState.RUNNING
+            # Transition to RUNNING state to start collecting timing data. Written
+            # under the lock the callbacks take, so the transition is ordered against
+            # their reads rather than racing them.
+            with self._lock:
+                self._state = _BenchmarkerState.RUNNING
 
             for i in range(num_runs):
                 logger.debug("Benchmark run %d/%d", i + 1, num_runs)
@@ -1833,7 +1842,8 @@ async def benchmark(
             await asyncio.to_thread(self._wait_for_intervals_to_complete)
 
             # All intervals have closed; stop collecting and build the result.
-            self._state = _BenchmarkerState.COMPLETED
+            with self._lock:
+                self._state = _BenchmarkerState.COMPLETED
 
             result = self._create_result()
             logger.info(

coreai_torch/debugging/comparator.py

@@ -417,8 +417,13 @@ def __init__(
             source: Source debug graph containing computation graph and inspector
             target: Target debug graph containing computation graph and inspector
             id_map: Mapping from source node IDs to target node IDs
-            strategy: Search strategy to use on source graph. Defaults to bisection (batch_size=10)
+            strategy: Search strategy to use on source graph. Defaults to
+                :class:`~coreai_torch.debugging.search_strategy.ExhaustiveStrategy`,
+                which checks every operation in one batch; see the note below for why
+                that rather than bisection.
             show_progress: Whether to show progress bar during comparison (default: True)
+            exclude_ops: Torch operation names treated as deliberate exclusions when
+                explaining what the id_map dropped.
 
         """
         self.source = source
@@ -890,11 +895,23 @@ def _validation_result_to_status(vr: SearchStrategy.ValidationResult) -> Status:
     def _status_to_validation_result(
         status: Status,
     ) -> SearchStrategy.ValidationResult:
-        """Convert Status to ValidationResult."""
+        """Project a Status onto what a search strategy acts on.
+
+        Lossy by design, but the loss has to preserve one distinction: whether the
+        operation was ever a candidate. EXCLUDED was projected to UNKNOWN along with
+        everything else, and a strategy narrows on UNKNOWN -- so a graph opening with
+        placeholders and `aten.view`s, both EXCLUDED, narrowed a level-order search to
+        depth 0 before a single value was compared.
+        """
         if status == Comparator.Status.PASS:
             return SearchStrategy.ValidationResult.PASS
         elif status == Comparator.Status.FAIL:
             return SearchStrategy.ValidationResult.FAIL
+        elif status == Comparator.Status.EXCLUDED:
+            # Never a candidate: a policy exclusion, or a node that computes nothing.
+            # Reporting it as unverified would have the search hunt for a fault in an
+            # operation there was never anything to check.
+            return SearchStrategy.ValidationResult.SKIPPED
         elif status == Comparator.Status.SHAPE_AMBIGUOUS:
             # Unverified, not clean: the values were never compared. Reporting it as PASS
             # would let a search prune a subgraph on the strength of a comparison that
@@ -1513,7 +1530,9 @@ async def create_comparator_for_programs(
         target_program: AIProgram (target compiled model)
         target_entry_point: Name of the coreai.graph in target program
         inspector_type: Type of inspector for the target program
-        strategy: Search strategy for source graph (defaults to bisection)
+        strategy: Search strategy for source graph. Defaults to `ExhaustiveStrategy`,
+                  which checks every operation in one batch and reports every
+                  divergence; see :class:`Comparator` for why that rather than bisection.
         use_caching: Whether to use caching inspectors (default: True)
         exclude_ops: Frozenset of torch operation names to exclude from comparison.
                      Defaults to _DEFAULT_EXCLUDED_OPS which includes view/reshape

coreai_torch/debugging/compute_plan.py

@@ -162,7 +162,7 @@ class ComputeDevice(Enum):
     """Unknown compute device."""
 
     @classmethod
-    def __missing__(cls, value: object) -> "ComputeDevice":
+    def _missing_(cls, value: object) -> "ComputeDevice":
         """Return UNKNOWN for unrecognized device values."""
         return cls.UNKNOWN