drop safety/availability from scoring, holistic p4 rating, phase-parallel pipeline
safety and availability don't reduce to physics formulas the way power_density/range_fuel/cost_efficiency do -- they're judgment calls (risk assessment, infrastructure prevalence), and running them through the same log-normalize() built for physical quantities produced incoherent results: safety's raw value is already a "0-1" score, and normalizing it again turned 0.6 into an unexplainable 0.678 that even the LLM reviewing it could only cite, never justify (see combo 1540). Removed both from domain_metric_weights (safety from 4 domains, availability from urban_commuting) and renormalized the remaining weights to sum to 1.0. Pass 4 now produces one holistic RATING (LOW/MEDIUM/HIGH) alongside the existing VERDICT, with safety and accessibility folded in as qualitative considerations feeding that single judgment rather than scored separately -- not a checklist of independent numbers. New qualitative_rating column, filterable in the results UI. Also added domain name/description to the review prompt so the LLM judges a metric like range against what the domain actually needs (urban_commuting: 1-50km) instead of generic real-world expectations for the platform category -- confirmed live on a combo where phi4 had called a 396km range "limited" by comparing to typical aircraft rather than a domain that needs 1-50km. Pass 2 is estimator-only now -- self.llm is never consulted there, reserved entirely for pass 4. Restructured Pipeline.run() from combo-first to phase-parallel: each pass now runs to completion across every combo before the next pass starts, rather than walking each combo through all four passes before the next combo. This surfaced a real bug: domain- blocked combos (status stays "valid" by design, not "_fail") were slipping past a naive status-based skip guard and getting silently re-processed by pass 2. Fixed with a shared dead-combo check that catches both generic failures and domain blocks correctly. Also fixes a results-page display bug found while reviewing a live combo: the per-metric "position" bar showed raw distance from norm_min without inverting for lower_is_better metrics, so an excellent cost score (near the good end) rendered as a ~0%, near-empty bar -- looked bad next to its own 0.99 normalized score. Validated live against phi4 (real Ollama calls, not mocked): full-domain phase-parallel run (2,970 combos, estimator-only p2, 1.6s) followed by a real pass-4 run (111 reviewed, 11m, 0 crashes, 0 null ratings). Two tests that relied on the old LLM-driven pass 2 to force deterministic outcomes were updated to test pass 4's verdict-wiring directly instead. All 100 tests pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -335,28 +335,34 @@ def test_p3_fail_below_threshold(seeded_repo):
|
||||
|
||||
|
||||
def test_p4_fail_implausible(seeded_repo):
|
||||
"""Combos deemed implausible by LLM should get p4_fail status."""
|
||||
"""Combos deemed implausible by LLM should get p4_fail status.
|
||||
|
||||
Pass 2 is estimator-only now (never calls the LLM), so there's no way
|
||||
to force every combo's raw estimates toward a controlled low/high value
|
||||
the way MockLLMProvider's default_estimates used to. Force the pass-4
|
||||
verdict directly instead -- what's under test here is pipeline.py's
|
||||
wiring of review_plausibility's return value to status/counters, not
|
||||
MockLLMProvider's avg-based heuristic.
|
||||
"""
|
||||
from physcom.llm.providers.mock import MockLLMProvider
|
||||
|
||||
class AlwaysImplausibleLLM(MockLLMProvider):
|
||||
def review_plausibility(self, description, raw_metrics, normalized_scores, domain):
|
||||
return ("Always implausible for testing.", False)
|
||||
|
||||
repo = seeded_repo
|
||||
domain = repo.get_domain("urban_commuting")
|
||||
|
||||
resolver = ConstraintResolver()
|
||||
scorer = Scorer(domain)
|
||||
# Low estimates → normalized scores avg <= 0.5 → MockLLMProvider returns (text, False)
|
||||
# Use threshold=0.0 so no combo gets p3_fail and all reach pass 4
|
||||
mock_llm = MockLLMProvider(default_estimates={
|
||||
"power_density": 0.1, "cost_efficiency": 0.1, "safety": 0.1,
|
||||
"availability": 0.1, "range_fuel": 0.1,
|
||||
})
|
||||
pipeline = Pipeline(repo, resolver, scorer, llm=mock_llm)
|
||||
pipeline = Pipeline(repo, resolver, scorer, llm=AlwaysImplausibleLLM())
|
||||
|
||||
result = pipeline.run(
|
||||
domain, ["platform", "actuator", "energy_storage"],
|
||||
score_threshold=0.0, passes=[1, 2, 3, 4],
|
||||
)
|
||||
|
||||
# With low normalized scores (avg <= 0.5), reviewed combos should be p4_fail
|
||||
assert result.pass4_failed > 0
|
||||
assert result.pass4_reviewed == 0
|
||||
|
||||
@@ -367,20 +373,23 @@ def test_p4_fail_implausible(seeded_repo):
|
||||
|
||||
|
||||
def test_p4_pass_plausible(seeded_repo):
|
||||
"""Combos deemed plausible by LLM should get llm_reviewed status."""
|
||||
"""Combos deemed plausible by LLM should get llm_reviewed status.
|
||||
|
||||
See test_p4_fail_implausible on why the verdict is forced directly
|
||||
rather than via controlled pass-2 estimates.
|
||||
"""
|
||||
from physcom.llm.providers.mock import MockLLMProvider
|
||||
|
||||
class AlwaysPlausibleLLM(MockLLMProvider):
|
||||
def review_plausibility(self, description, raw_metrics, normalized_scores, domain):
|
||||
return ("Always plausible for testing.", True)
|
||||
|
||||
repo = seeded_repo
|
||||
domain = repo.get_domain("urban_commuting")
|
||||
|
||||
resolver = ConstraintResolver()
|
||||
scorer = Scorer(domain)
|
||||
# High estimates → avg > 0.5 → MockLLMProvider returns (text, True)
|
||||
mock_llm = MockLLMProvider(default_estimates={
|
||||
"power_density": 500.0, "cost_efficiency": 5e-4, "safety": 0.6,
|
||||
"availability": 0.7, "range_fuel": 200000.0,
|
||||
})
|
||||
pipeline = Pipeline(repo, resolver, scorer, llm=mock_llm)
|
||||
pipeline = Pipeline(repo, resolver, scorer, llm=AlwaysPlausibleLLM())
|
||||
|
||||
result = pipeline.run(
|
||||
domain, ["platform", "actuator", "energy_storage"],
|
||||
|
||||
Reference in New Issue
Block a user