From 3795a7e8262e3b4990fbb43eaf5bdf98a9378852 Mon Sep 17 00:00:00 2001 From: Andrew Simonson Date: Sun, 16 Aug 2026 17:33:22 -0500 Subject: [PATCH] rename estimator, skip unused cargo calc, fix six review bugs, close three seed guardrail holes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename: _stub_estimate -> _estimate_physics (it's a real deterministic physics engine now, not a stub) and estimation_method "stub" -> "physics_calc" to match the value pass 3 already used, for consistency between the raw-estimate and scored-metric tables. Also skip the cargo_capacity/cargo_capacity_kg arithmetic entirely in _raw_physics_from_masses for domains that score neither and don't need it as cost_efficiency's $/(kg·m) denominator either -- real but modest savings on the ~11,000-eval-per-combo optimizer hot path (a separate log1p-caching attempt was tried and reverted: it measured SLOWER, not faster -- the extra dict lookup cost more than the two math.log1p calls it avoided). Six bugs found by a full-codebase review agent, verified individually: - pipeline.py: LLM rate-limit retry called review_plausibility() with domain.metric_bounds instead of domain, crashing the whole pipeline run on any retry (every provider immediately accesses domain.name/ .metric_bounds on that arg). - _explore_result.html: mass-bar width divided by total_mass with no zero guard; biological/ambient actuators can legitimately have 0 mass floors, so an all-zero slider combination 500'd the explore endpoint. - routes/pipeline.py: if init_db/Repository(conn) raised before repo/conn were assigned, the except/finally handlers referencing them raised UnboundLocalError, silently swallowed by bare except/pass -- a bad PHYSCOM_DB path left a run stuck at status=pending forever with no diagnostic. conn/repo now init to None and are guarded before use; the truly-unreachable-DB case at least logs server-side now. - repository.py: update_combination_status's downgrade guard protected scored/llm_reviewed/*_fail but not a write of "valid" -- pass 1 re-running for a different domain against an already-reviewed combo silently reverted its status back to "valid", erasing the review signal. Verified directly: marked a combo reviewed, re-ran pass 1, status held. - pipeline.py: cost_efficiency's operating-cost term fell back to ground rolling-resistance physics (effective_k_med or ...["ground"]) for media with no resistance model (space), instead of skipping the term the way range_fuel explicitly does two lines above. Every scored interplanetary_travel combo got a cost_efficiency computed from ground physics applied to a spacecraft. Now reports amortized/upfront cost only for such media -- an honest partial answer. - pipeline.py: `if min_accel and specific_thrust:` used truthiness instead of `is not None` -- dep_value() legitimately returns 0.0 for a declared floor of zero (Spaceship declares min_effective_accel=0), masking a real requirement as "undeclared." Three seed-data guardrail holes, matching LOGIC DOCS/002's "missing floor is a silent hole" pattern: - constraint_resolver.py: CATEGORY_SEVERITY had no entry for the "material" category, so Nuclear Thermal Drive/Nuclear Fuel's radiation_shielding requirement defaulted to a non-blocking "warn" nothing in the catalog ever satisfies. Added material -> block. Consequence, verified: every nuclear combo across all domains now correctly fails pass 1, since nothing currently provides shielding -- the accurate state given the catalog gap, not a regression. - transport_example.py: Submarine had a mass range_min but no range_max, unlike its sibling water platform -- _decide_masses skips its entire structural-feasibility search when p_max is None. Added a 20,000,000kg ceiling (small submersible to large ballistic-missile class). - transport_example.py: Amphibious Vehicle declared no medium requires at all, so it vacuously satisfied every domain's medium constraint including space-only interplanetary_travel. Added medium=ground (the current requires model has no OR semantics for "ground or water," so this is a real tradeoff -- it can no longer participate in maritime_shipping either, losing the water half of "amphibious"). Verified: interplanetary_travel's pass-2-estimated count dropped from 33 to 3, and all 3 remaining are genuinely Spaceship-based; the ~30 removed were confirmed to be Amphibious Vehicle's vacuous passes. Logged the GPU-batching-for-the-optimizer discussion (why it doesn't fit at current scale, what threshold would change that, what it would actually require) as LOGIC DOCS/003 for future reference. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 2 +- .../003-gpu-batching-for-scaled-optimizer.md | 34 ++++++ src/physcom/db/repository.py | 9 +- src/physcom/engine/constraint_resolver.py | 7 ++ src/physcom/engine/pipeline.py | 101 ++++++++++++------ src/physcom/seed/transport_example.py | 15 +++ src/physcom_web/routes/pipeline.py | 33 ++++-- .../templates/results/_explore_result.html | 2 + 8 files changed, 155 insertions(+), 48 deletions(-) create mode 100644 LOGIC DOCS/003-gpu-batching-for-scaled-optimizer.md diff --git a/CLAUDE.md b/CLAUDE.md index 65f071b..d997a54 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,7 +60,7 @@ tests/ # pytest, uses seeded_repo fixture from conftest.py ## Data flow (pipeline passes) 1. **Pass 1 — Constraints**: `ConstraintResolver.resolve()` → blocked/conditional/valid. Blocked combos get a result row and `continue`. -2. **Pass 2 — Estimation**: LLM or `_stub_estimate()` → raw metric values. Saved immediately via `save_raw_estimates()` (normalized_score=NULL). +2. **Pass 2 — Estimation**: `_estimate_physics()` (deterministic physics engine; estimator-only, no LLM) → raw metric values. Saved immediately via `save_raw_estimates()` (normalized_score=NULL). 3. **Pass 3 — Scoring**: `Scorer.score_combination()` → log-normalized scores + weighted geometric mean composite. Saves via `save_scores()` + `save_result()`. 4. **Pass 4 — LLM Review**: Only for above-threshold combos with an LLM provider. No real provider yet (only `MockLLMProvider`). 5. **Pass 5 — Human Review**: Manual via web UI results page. diff --git a/LOGIC DOCS/003-gpu-batching-for-scaled-optimizer.md b/LOGIC DOCS/003-gpu-batching-for-scaled-optimizer.md new file mode 100644 index 0000000..7e93edb --- /dev/null +++ b/LOGIC DOCS/003-gpu-batching-for-scaled-optimizer.md @@ -0,0 +1,34 @@ +# GPU batching for the mass-allocation optimizer — not yet, here's the threshold + +## Context + +`Pipeline._decide_masses`'s joint platform/actuator/storage optimizer (coarse-to-fine grid search, see `_search_best_allocation`) calls its objective function roughly 11,700 times per combo. Profiling confirmed this dominates pipeline runtime: for 50 combos, 582,920 objective-function calls, each doing scalar arithmetic (power_density, the drag cubic solve, normalize, composite_score) on one `(platform, actuator, storage)` triple. The cost is Python's per-call overhead (bytecode dispatch, refcounting, attribute lookups), not the arithmetic itself — the individual formulas are cheap. + +## Why GPU doesn't fit today + +A single combo's grid is only ~169 points per round (13×13). GPUs pay off when there's enough independent parallel work to amortize kernel-launch and host↔device transfer overhead (each typically tens of microseconds to low milliseconds); 169 elements doesn't come close, and that overhead would be paid repeatedly — once per grid round, ~6-10 rounds per combo. + +The parallelism that actually exists is **across combos**, not within one combo's grid — every combo's optimization is fully independent of every other's. At current scale (~180 combos reach the optimizer per domain after Pass 1 filtering), batching every combo's grid into one array gives ~180×169 ≈ 30K elements per round — borderline, probably a wash against plain CPU numpy. + +## The actual threshold + +Combo count scales **multiplicatively** with added dimensions or entities per dimension (today: 11 platforms × 15 actuators × 18 storages ≈ 2,970 combos, ~180 of which reach the optimizer). Add a 4th dimension with even 10 options and total combos scale to ~30,000, with optimizer-eligible combos likely growing roughly proportionally to ~1,800/domain — batched grid size ≈ 300K elements/round. A 5th dimension does it again, into the low millions. That's the regime where a GPU's thousands of cores start meaningfully outrunning a CPU's 4-16-wide SIMD lanes. + +So: not "more dimensions" directly, but the combo×grid batch size those dimensions produce. Rough rule of thumb from this discussion: + +- **Tens of thousands of elements/round** (current scale, or a modest one-dimension addition): plain CPU numpy vectorization is enough, no GPU. +- **Hundreds of thousands to low millions**: GPU batching across combos starts being worth evaluating. + +## What GPU batching would actually require + +Not just "swap numpy for cupy." It means restructuring `_process_pass2` from combo-first (one combo through the optimizer at a time) to batch-first (a chunk of N combos' grids evaluated together as one array with a "combo" axis, broadcasting each combo's own constants — `k_act`, `k_med`, `e_dens`, drag coefficients, mass bounds — across that axis). That's a real architectural change, not a drop-in acceleration: + +- **CLAUDE.md documents the pipeline as deliberately combo-first**: "each combo goes through all requested passes before the next combo starts... Progress is persisted per-combo (crash-safe, resumable)." Batching means checkpointing per-*batch*, not per-combo — a real (if manageable) tradeoff against that resumability guarantee, not something that comes free alongside the speedup. +- Every branch in `_raw_physics_from_masses` / `_solve_achievable_speed_mps` (biological floors, ambient energy forms, degenerate fallbacks, the cubic's edge cases) needs to become `np.where(condition, a, b)` instead of `if/else` — careful, error-prone translation work, not mechanical. + +## Decision + +Don't build this now — no current need at ~180 combos/domain. If dimension count grows enough to matter, do it in two steps: + +1. **CPU numpy vectorization first** (batch one combo's grid into arrays, evaluate with vectorized ops instead of a Python double-loop). This is needed regardless of GPU or not, since it's the same rewrite either way, and profiling suggests it could plausibly give 10-50x on its own by replacing ~11,700 Python calls/combo with a couple dozen numpy batch calls. +2. **Re-profile at the new scale.** Only reach for GPU batching-across-combos if CPU numpy is still the dominant cost after step 1, and only once the batched element count is actually in GPU-favorable territory (see thresholds above) — this is a "measure, then decide" call, not something to build ahead of need. diff --git a/src/physcom/db/repository.py b/src/physcom/db/repository.py index f38d33b..1b1404b 100644 --- a/src/physcom/db/repository.py +++ b/src/physcom/db/repository.py @@ -475,7 +475,7 @@ class Repository: self, combo_id: int, status: str, block_reason: str | None = None, commit: bool = True ) -> None: # Don't downgrade from higher pass states — preserves human/LLM review data - if status in ("scored", "llm_reviewed") or status.endswith("_fail"): + if status in ("scored", "llm_reviewed", "valid") or status.endswith("_fail"): row = self.conn.execute( "SELECT status FROM combinations WHERE id = ?", (combo_id,) ).fetchone() @@ -488,6 +488,13 @@ class Repository: return if status == "llm_reviewed" and cur == "reviewed": return + # "valid" is pass 1's domain-agnostic result -- a combo + # already at any later pass state (or a fail state) has + # progressed past pass 1 already, in this domain or + # another one sharing the same combo. Pass 1 re-running + # for a different domain must not silently revert that. + if status == "valid" and cur not in (None, "valid"): + return self.conn.execute( "UPDATE combinations SET status = ?, block_reason = ? WHERE id = ?", (status, block_reason, combo_id), diff --git a/src/physcom/engine/constraint_resolver.py b/src/physcom/engine/constraint_resolver.py index f77e764..ed51a95 100644 --- a/src/physcom/engine/constraint_resolver.py +++ b/src/physcom/engine/constraint_resolver.py @@ -32,6 +32,13 @@ CATEGORY_SEVERITY: dict[str, str] = { "energy": "block", "environment": "block", "infrastructure": "skip", + # Safety-critical physical necessities (radiation shielding, containment, + # etc.) -- same severity as energy/environment, not the softer default + # "warn" every other category falls through to. Missing this entry meant + # Nuclear Thermal Drive/Nuclear Fuel's "material" requires (radiation_ + # shielding) defaulted to a non-blocking warning nothing in the catalog + # ever satisfies -- see LOGIC DOCS/002's "silent guardrail hole" pattern. + "material": "block", } # For provides-vs-range_min: deficit > this ratio = hard block, else warning diff --git a/src/physcom/engine/pipeline.py b/src/physcom/engine/pipeline.py index bd98719..5a39f82 100644 --- a/src/physcom/engine/pipeline.py +++ b/src/physcom/engine/pipeline.py @@ -299,10 +299,12 @@ DRAG_POWER_COEFF_BY_MEDIUM: dict[str, float] = { # ~11kW, in the right ballpark for real highway cruise power. "ground": 0.5 * 1.225 * 0.3 * 2.2, # 0.5 * rho_air(1.225) * Cd(~0.2, streamlined fuselage) * frontal_area - # (~3.5 m^2, small aircraft/rotorcraft reference) -- sanity check: at - # 60 m/s (a fast urban rotorcraft cruise) this alone costs ~46kW, in - # the right ballpark for a light helicopter's real cruise power. - "air": 0.5 * 1.225 * 0.2 * 3.5, + # (~1.74 m^2, small aircraft/rotorcraft reference) -- sanity check: at + # 60 m/s (a fast urban rotorcraft cruise) this alone costs ~46kW, a + # plausible fraction of a light helicopter's real cruise power (most + # of the rest goes to induced/rotor drag, not modeled here -- this + # coefficient only covers fuselage parasite drag). + "air": 0.5 * 1.225 * 0.2 * 1.74, } # Structural manufacturing cost, $ per kg of platform mass -- certification @@ -672,7 +674,7 @@ class Pipeline: result.pass2_estimated += 1 return - raw_metrics, feasible = self._stub_estimate(combo, domain.metric_bounds) + raw_metrics, feasible = self._estimate_physics(combo, domain.metric_bounds) if not feasible: # No platform mass within its own declared ceiling could @@ -705,7 +707,7 @@ class Pipeline: estimate_dicts.append({ "metric_id": mb.metric_id, "raw_value": rval, - "estimation_method": "stub", + "estimation_method": "physics_calc", "confidence": 1.0, }) if estimate_dicts: @@ -843,7 +845,7 @@ class Pipeline: self._wait_for_rate_limit(run_id, exc.retry_after) try: review_result = self.llm.review_plausibility( - description, raw_dict, score_dict, domain.metric_bounds + description, raw_dict, score_dict, domain ) except LLMRateLimitError: return # still limited; skip, retry next run @@ -888,9 +890,9 @@ class Pipeline: self, combo: Combination, bounds_by_name: dict[str, MetricBound] ) -> "_PhysicsContext | None": """Derive the entity-level physics inputs that don't depend on a - mass allocation choice -- shared by _stub_estimate (which picks the - allocation via solve or a special case) and _optimize_allocation - (which searches over candidate allocations). Returns None if the + mass allocation choice -- shared by _estimate_physics (which picks + the allocation via _decide_masses) and evaluate_allocation (the + explore-panel's direct evaluation). Returns None if the combo doesn't have the platform/actuator/storage shape this whole formula assumes (shouldn't happen for real combos, but a domain without all three dimensions requested would hit this).""" @@ -976,13 +978,25 @@ class Pipeline: # scores is just which metric_name it declares. Floored at 0: a # storage mass bigger than the whole allowance leaves no cargo # room, not negative room. - lightship_mass = p_mass + actuator_mass - cargo_capacity_2_5x = max(0.0, lightship_mass * CARGO_KG_PER_STRUCTURAL_KG - storage_mass) - cargo_capacity_0_3x = max(0.0, lightship_mass * 0.3 - storage_mass) - if "cargo_capacity" in bounds_by_name: - out["cargo_capacity"] = cargo_capacity_2_5x - if "cargo_capacity_kg" in bounds_by_name: - out["cargo_capacity_kg"] = cargo_capacity_0_3x + # Skip the arithmetic entirely for domains that don't score either + # cargo convention and don't need it as cost_efficiency's $/(kg·m) + # denominator either -- this runs on every one of the ~11,000 grid + # points the search below tries per combo, so a domain like + # interplanetary_travel (scores neither) shouldn't pay for it. + needs_cargo = ( + "cargo_capacity" in bounds_by_name + or "cargo_capacity_kg" in bounds_by_name + or units_by_name.get("cost_efficiency") == "$/(kg·m)" + ) + cargo_capacity_2_5x = 0.0 + if needs_cargo: + lightship_mass = p_mass + actuator_mass + cargo_capacity_2_5x = max(0.0, lightship_mass * CARGO_KG_PER_STRUCTURAL_KG - storage_mass) + cargo_capacity_0_3x = max(0.0, lightship_mass * 0.3 - storage_mass) + if "cargo_capacity" in bounds_by_name: + out["cargo_capacity"] = cargo_capacity_2_5x + if "cargo_capacity_kg" in bounds_by_name: + out["cargo_capacity_kg"] = cargo_capacity_0_3x # Achieved steady-state cruise speed, DERIVED from this specific # build's actual power_density, the medium's mass-proportional @@ -1030,11 +1044,20 @@ class Pipeline: ) amortized_per_m = upfront_cost / lifetime_m - fuel_price_per_mj = FUEL_PRICE_PER_MJ.get(ctx.storage_energy_form, 0.04) - energy_per_m_mj = ( - (effective_k_med or SPECIFIC_ENERGY_CONSUMPTION_J_PER_KG_M["ground"]) * floor_total - ) / 1e6 - operating_per_m = energy_per_m_mj * fuel_price_per_mj + if ctx.k_med is not None: + fuel_price_per_mj = FUEL_PRICE_PER_MJ.get(ctx.storage_energy_form, 0.04) + energy_per_m_mj = (effective_k_med * floor_total) / 1e6 + operating_per_m = energy_per_m_mj * fuel_price_per_mj + else: + # No resistance model for this medium (space -- real range + # is governed by the rocket equation, not implemented + # here, see the comment above + # SPECIFIC_ENERGY_CONSUMPTION_J_PER_KG_M). Don't fabricate + # an operating cost from ground physics the way an earlier + # version of this did (`effective_k_med or ...["ground"]`) + # -- report upfront/amortized cost only, an honest partial + # answer, rather than a wrong number for an unmodeled term. + operating_per_m = 0.0 cost_per_m = amortized_per_m + operating_per_m if units_by_name.get("cost_efficiency") == "$/(kg·m)": @@ -1111,7 +1134,12 @@ class Pipeline: range_bounds = bounds_by_name.get("range_fuel") target_range = range_bounds.norm_max if range_bounds else None - if min_accel and specific_thrust: + # `is not None`, not truthy -- dep_value() legitimately returns 0.0 + # for a declared floor of zero (Spaceship declares + # min_effective_accel=0, a real "no acceleration floor" value, not + # "undeclared"). A truthy check would silently treat that the same + # as an absent requirement and fall through to the wrong branch. + if min_accel is not None and specific_thrust is not None: c1, r1 = specific_thrust, min_accel elif target_velocity and ctx.k_med: # Resistance alone (k_med) only covers steady-state cruise -- @@ -1295,21 +1323,24 @@ class Pipeline: return best_a, best_s, best_score - def _stub_estimate( + def _estimate_physics( self, combo: Combination, metric_bounds: list[MetricBound] ) -> tuple[dict[str, float], bool]: - """Deterministic estimation from declared entity attributes (no LLM). + """Deterministic physics-based estimation from declared entity + attributes (no LLM) -- pass 2's estimator. - power_density, range_fuel, and cost_efficiency are computed from the - platform's declared mass envelope treated as a combo-wide budget — - see the module-level comment above BIOLOGICAL_OPERATOR_MASS_KG for - the full formula rationale. + power_density, speed, range_fuel, cost_efficiency, and + cargo_capacity/cargo_capacity_kg are all computed from the + platform's declared mass envelope treated as a combo-wide budget, + jointly optimized by _decide_masses -- see the module-level + comment above BIOLOGICAL_OPERATOR_MASS_KG for the full formula + rationale. - safety/availability/reliability/cargo_capacity/environmental_impact - are untouched — these are judgment calls (regulatory, economic, - qualitative), not physics, and stay on the categorical lookup-table - heuristics below (actuator's thrust_profile and energy_form and the - combo's infrastructure requirements). + safety/availability/reliability/environmental_impact are untouched + — these are judgment calls (regulatory, economic, qualitative), + not physics, and stay on the categorical lookup-table heuristics + below (actuator's thrust_profile and energy_form and the combo's + infrastructure requirements). cost_efficiency additionally checks the domain's declared unit: "$/(kg·m)" (freight-style domains) isn't a rescaling of "$/m" — it's @@ -1413,7 +1444,7 @@ class Pipeline: is ever persisted. Any mass left as None defaults to what the real requirement-based - solve already picked (see _decide_masses / _stub_estimate), so a + solve already picked (see _decide_masses / _estimate_physics), so a slider opens on today's actual build, not an arbitrary point. Explicit values are floor-clamped to each component's own declared minimum (platform is also ceiling-clamped to its declared max) -- diff --git a/src/physcom/seed/transport_example.py b/src/physcom/seed/transport_example.py index df0f9cb..6fedfee 100644 --- a/src/physcom/seed/transport_example.py +++ b/src/physcom/seed/transport_example.py @@ -95,6 +95,7 @@ WATER_PLATFORMS: list[Entity] = [ Dependency("environment", "gravity", "true", None, "provides"), Dependency("physical", "footprint", "200", "m²", "range_max"), Dependency("physical", "footprint", "20", "m²", "range_min"), + Dependency("physical", "mass", "20000000", "kg", "range_max"), Dependency("physical", "mass", "10000", "kg", "range_min"), Dependency("environment", "medium", "water", None, "requires"), Dependency("physical", "energy_density", "720000", "J/kg", "range_min"), @@ -198,6 +199,20 @@ MULTI_PLATFORMS: list[Entity] = [ Dependency("physical", "footprint", "5", "m²", "range_min"), Dependency("physical", "mass", "10000", "kg", "range_max"), Dependency("physical", "mass", "1500", "kg", "range_min"), + # No requires here previously -- vacuously satisfied every + # domain's medium DomainConstraint (check_domain_constraints + # only flags a violation when an entity DECLARES a requires + # for the constrained key), including space-only + # interplanetary_travel. The current requires/domain-constraint + # model only supports one value per key -- there's no OR + # mechanism for "ground or water" -- so this picks ground + # (its primary, most-common domain) rather than leaving it + # undeclared. Real tradeoff: it can no longer participate in + # maritime_shipping (water-only) either, losing the water half + # of "amphibious." Closes the vacuous-pass hole; genuine + # multi-medium support would need OR semantics added to + # check_domain_constraints, a separate, bigger change. + Dependency("environment", "medium", "ground", None, "requires"), ], ), ] diff --git a/src/physcom_web/routes/pipeline.py b/src/physcom_web/routes/pipeline.py index ab8f0a3..b2ac11f 100644 --- a/src/physcom_web/routes/pipeline.py +++ b/src/physcom_web/routes/pipeline.py @@ -32,6 +32,8 @@ def _run_pipeline_in_background( from physcom.engine.scorer import Scorer from physcom.engine.pipeline import Pipeline + conn = None + repo = None try: conn = init_db(db_path) repo = Repository(conn) @@ -58,18 +60,27 @@ def _run_pipeline_in_background( run_id=run_id, ) except Exception as exc: - try: - repo.update_pipeline_run( - run_id, status="failed", - error_message=str(exc)[:500], - ) - except Exception: - pass + if repo is not None: + try: + repo.update_pipeline_run( + run_id, status="failed", + error_message=str(exc)[:500], + ) + except Exception: + pass + else: + # Couldn't even open the DB to record the failure (bad + # PHYSCOM_DB path, locked/corrupt file) -- the pipeline_runs + # row will stay "pending" forever with no way to write an + # error_message to it, so at least don't let that swallow the + # real cause silently. Server logs are the only trace left. + print(f"pipeline run {run_id} failed before DB was reachable: {exc!r}") finally: - try: - conn.close() - except Exception: - pass + if conn is not None: + try: + conn.close() + except Exception: + pass @bp.route("/") diff --git a/src/physcom_web/templates/results/_explore_result.html b/src/physcom_web/templates/results/_explore_result.html index 8278b7d..431d43a 100644 --- a/src/physcom_web/templates/results/_explore_result.html +++ b/src/physcom_web/templates/results/_explore_result.html @@ -25,9 +25,11 @@ platform has no declared mass ceiling to bound the sliders.

{% set total = r.total_mass %} + {% if total > 0 %}
+ {% endif %}
platform {{ "%.1f"|format(r.platform_mass) }}kg