Repository navigation
fix(scorer): filter non-finite distance signals and validate threshold in distance scorers - #558
Conversation
…d in distance scorers
|
REQUIRE_REVIEWER. No verdict issued. This contributor has used their $20 monthly review allowance across their PRs. Next step: @jeqcho, the service did not start a sandbox or make a model call. Review manually; the allowance resets at the start of the next UTC month. This is a limitation or failure of the reviewer, not a finding against the contribution. No approval, merge or closure recommendation was issued. Commit: Budget: this run booked $0.000 for 0 model calls, plus $0.000 sandbox allowance. This revision has used $0.000 of $5.000 across all runs; $0.000 remains under all spending caps. These are conservative ledger amounts, not an invoice. |
SIDDARTHAREDDY8
left a comment
There was a problem hiding this comment.
Two edge cases in the new validation in src/inspect_robots/scorer.py that the added tests do not cover:
-
Overflow escapes both new guards. In
_distances, thefloat(raw)conversion is wrapped inexcept (TypeError, ValueError), butfloat()on a large int (for example a distance recorded as10**400) raisesOverflowError, so one bad signal would still crash the scorer instead of being treated as unobserved. The same applies in_ReachedGoalState.__post_init__:float(self.threshold)on a large int raisesOverflowErrorrather than theTypeError/ValueErrorthe validation otherwise raises. AddingOverflowErrorto the catch in_distances(and converting it in__post_init__) would close that hole. -
The threshold type check is stricter than the distance path, and inconsistent for NumPy scalars.
_distancesaccepts anythingfloat()can coerce, but__post_init__requiresisinstance(threshold, int | float).np.float64passes because it subclassesfloat, whilenp.float32(0.05)(andDecimal) raisesTypeError, even though it would have worked before this change via themin(dists) <= self.thresholdcomparison. Consider anumbers.Realcheck or the same try-float()coercion used for distances, plus a test with anp.float32threshold.
The core NaN/inf filtering in _distances itself looks right, including the explicit bool rejection.
|
@jeqcho please have a look |
Summary
The distance scorers (
min_distance_to_goalandreached_goal_state) consumedistancesignals recorded instep.result.info.NaN/inf): When sensors warm up, track is temporarily lost, or cameras experience occlusion during initial rollout steps, embodiments may recordNaNininfo["distance"]. Because Python'smin([nan, 0.02])returnsnan,reached_goal_stateevaluatesnan <= thresholdasFalse, causing runs that reached the goal to be marked as failed. Inmin_distance_to_goal,Score(value=nan)propagates into downstream evaluation aggregations (such as mean across scenes), turning aggregate benchmark scores intoNaN. Furthermore, ifinfo["distance"]is unconvertible orNone, it raised unhandledTypeErrororValueError.NaN,min_distance_to_goalpreviously returnednaninstead ofinf("no distance signal recorded").reached_goal_state(threshold)silently accepted invalid thresholds, including negative values (which can never be achieved),NaN, and booleans.This change filters
_distancesto finite non-negative values, ignoring missing, non-numeric, non-finite, and negative entries. It also validates thatreached_goal_statethreshold is a finite, non-negative real number.Changes
src/inspect_robots/scorer.py:_distancesto filter out non-finite (NaN,±inf), negative, boolean, and non-numeric values.thresholdin_ReachedGoalStateat initialization to ensure it is a finite non-negative real number.tests/test_scorers.py: Add unit tests forNaNhandling alongside valid distances, filtering of invalid entries, fallback when no distance signals exist, and threshold validation.changelog.d/+distance-scorers-nan-and-threshold-validation.fixed.md: Add changelog fragment.