fix: false go verdicts - #28
Merged
Merged
Conversation
Charging a whole leg at its vertical phase power understated descent energy: supplying descent_power_w cut mission energy by 36% and turned INFEASIBLE into FEASIBLE. Legs are now split by phase time. max_segment_length_m defaults to 500 m so a leg is sampled along its length rather than at its departure end, and usable_capacity_curve derating now reaches the RTH and divert margins.
The check used a two-point line in degree space. A 40 km leg at 60N bows about 54 m off that chord, so a zone in the gap was flown through and reported clear; across the antimeridian the line wrapped the wrong way round the globe. Legs now come from the shared sampler with longitudes unwrapped. Also stops out-of-window required zones gating altitude, and fixes the transposed (lon, lat) unpack in the SORA route-AGL walk.
Deleting any of these left the suite green, and each one only makes a verdict more permissive: obstacle uncertainty_m, the ground-risk and landing-zone sampling-gap terms, and the RTH min-groundspeed abort.
An unbuffered figure was rendered as "Mission iGRC" with nothing to separate it from a buffered assessment, which on the same route can be two risk classes higher. The report now states the buffer and warns when it is zero.
A scenario is only FAILED when an assertion actively fails, so SKIPPED and UNSUPPORTED outcomes left it PASSED. A divert check is skipped when the mission declares no lost_link_policy, so the contingency went unevaluated and still reported a pass. Inconclusive assertions are now missing evidence, which blocks GO.
Readiness was computed in the envelope but recomputed or ignored elsewhere, so one run could report no_go in JSON while the CLI exited 0 and the checklist printed GO. Batch consulted only ScenarioStatus, so a mission the estimator called INFEASIBLE reported PASSED. All four surfaces now call scenario_readiness(), and --engineering-only is threaded into batch dispatch instead of being permanently implied.
Declaring a resource system hands the battery and RTH gates to resource_link, which budgeted nameplate: a 170 Wh pack delivering 93.5 Wh was budgeted at 170 Wh and INFEASIBLE became SUCCESS. A leg's stored wind comes from the horizontal integration, which ends early when the climb outlasts the ground track, so wind limits were checked against the departure-end wind. Legs are now sampled over their full duration and altitude band.
Proximity was an intersection test, which is empty unless the geometries actually cross. Zero-radius obstacles were invisible under the default clearance, and a vertical leg built a zero-length line that intersects nothing, so a 75 m mast 18 m from the pad reported feasible. It is now a distance test. Vertical clearance also interpolated altitude linearly, sitting above the flown profile; it now takes the lower endpoint. Adds OBSTACLE_ZERO_FEATURES and OBSTACLE_KEEP_OUT_NOT_CONFIGURED.
This was the only wind-aware solver that never gated its wind triangle, so a divert needing 70.8 deg of crab against a 35 deg limit passed. Energy charged transit only and never read the zone altitude, so a 400 m descent cost nothing, and the whole Dubins path was charged at the straight-line groundspeed. Both gates now fire, the terminal descent is budgeted, and the path is bounded by the harshest heading the turn sweeps.
The readiness gate only rejected iGRC > 7 and never checked the population assessment buffer, so a centerline-only figure counted as ground-risk evidence. A landing_zone_unavailable event naming an unconfigured zone removed nothing, so the run came back unperturbed while the report echoed the id back as closed. Unmatched ids now fail closed.
A calibration overrides vehicle performance, but provenance enumerated a fixed set of inputs, so a calibrated run and the base run were byte-identical in provenance. It now has its own entry. input_schema_versions advertised population-grid.v1 while the loader only accepts v2, and omitted obstacles and calibration entirely. The test compared the CLI to the same constants, so the drift was invisible. Bumps the envelope to estimator-envelope.v10.
Missions are preloaded to enumerate their assets, so one unreadable mission aborted the whole batch before any run executed. The failing run is now an ERROR row naming the run id and the file. A leftover .tmp from an interrupted run blocked reruns into the same --output-dir forever; our own scratch files are cleaned up instead. migrate had no catch-all and raised TypeError on a non-string airspace.class. Bounds dt_s so a tiny step cannot make the propagation timeline unbounded.
render_sora_markdown discarded everything but the assessment, so the filed artifact named neither the tool version, the SORA edition, the population data vintage nor the input digests. The envelope also omitted the terrain asset that gates the maximum-AGL verification, so the assessment could not be reproduced from it.
Metadata was written with Python repr(), so a source name or reference containing quotes produced a file the loader could not read back. Also splits the forward-flight mass exponent out of the hover one: transit scaled by a hardcoded 0.5, so induced_power_mass_exponent never reached cruise. The new cruise_power_mass_exponent defaults to 0.5 and keeps existing results.
A single missing SRTM cell aborted the whole fetch, so the documented
"fetch your own area" command died on our own coordinates and left the
output directory empty. Adds --void-policy {fail,interpolate} and lets
fetch_all warn and continue.
The bundled Alpine terrain had five voids written as 0.0 - sea level in
the Alps - now filled from the nearest sampled elevation. Corrects the
example README, which also claimed the bundled wind grid exercises
spatiotemporal interpolation when only its 10 m band holds real values.
adapters.cli registers every command at import time, so importing a single command first left the module half-initialised and raised ImportError. 18 command modules and adapters.preflight were affected. Exit codes, output formats and error helpers move to a leaf adapters.cli_contract that commands import instead; adapters.cli re-exports them unchanged. validate now refuses a calibration fitted from the trace being validated: a model tuned on a flight reproduces that flight, so the agreement was never evidence.
The wind grid held real values only in its 10 m band, on an axis expressed above ground while the provider queries metres AMSL. The route flies near 550 m AMSL, so every sample clamped to the grid edge and the flagship example demonstrated the wind model on a null input: worst wind 0.00 m/s, now 3.55 m/s. Terrain is re-fetched with --void-policy interpolate; SRTM voids over the lakes had been written as 0.0, sea level in the Alps. Corrects the README coverage table - the terrain grid spans lon 8.15-8.45, not 8.1-8.4 - and the reproduce commands.
The distribution installed five generic top-level names - main, adapters, estimator, schemas, scripts - and pip does not detect file conflicts, so any co-installed distribution shipping one of them silently overwrote ours, and ours overwrote theirs. Installing a second package with a schemas/ directory broke bvlos-sim outright, and uninstalling it again did not repair the install. Imports gain a prefix: "from estimator import ..." becomes "from bvlos_sim.estimator import ...". Console entrypoints are unchanged and python -m bvlos_sim now works. CI and the suite assert the wheel exposes exactly one top-level name.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test Plan
uv run pytestpassesuv run ruff check .passesContract Impact
CONTRIBUTING.md(Public contracts)