You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up from the #1089 review (its "finding 8", plus the AOD capacity gap it left open). Both concern Push and Rotate (P&R) as NoHome's RANKED mover selection uses it (#1083), and both change the planner's API, so they belong together.
1. Rebuild the planner's per-stage state once, not per candidate
RANKED plans every candidate target in a CZ stage, up to max_mover_candidates (64), with a separate one-shot call: solve_with_engine(… Strategy::PushRotate …) → solve_push_rotate(index, initial, target, blocked, budget). Each call rebuilds its planner state from scratch:
LaneGraph::build(index, blocked): walks every lane of the architecture, drops blocked sites, sorts and deduplicates the adjacency lists.
Decomposition::build(graph, occupancy(start)): connected components, subgraphs, and per-component empty-site counts.
Mapping start and target onto graph vertices.
Target assignment, planning, scheduling into AOD shots, and the replay check.
Only the target half of step 3 and step 4 depend on the candidate. The lane graph depends only on the architecture and blocked, and the decomposition only on the graph and the starting occupancy. Both are identical for every candidate in a stage, yet they're rebuilt up to 64 times.
Proposal: a reusable planner, e.g. PushRotatePlanner::new(index, blocked, initial) and then .plan(target) per candidate, with solve_push_rotate kept as a thin wrapper. This is a planner API change: today's public entry points take the whole problem in one call.
Expected effect: compile time only. Graph and decomposition are deterministic, so every plan and result stays identical, and the behaviour net and benchmark baselines should not move. The #1089 review measured RANKED at about 14% more compile time than RULE. How much of that is this rebuilding, as opposed to the second routing solve for the rule's candidate and the planning itself, is unmeasured, so profile first: time one RANKED stage split into graph build, decomposition, planning and routing.
2. Enforce the AOD capacity in the planner
ArchSpec.aod_capacity (#1061) caps the source columns and rows one AOD shot may use. Every search generator honours it. P&R does not:
The scheduler ignores it.push_rotate/schedule.rs packs the largest ready batch into one rectangle without consulting a capacity, so Strategy::PushRotate can return a shot wider than the cap. This is documented on LaneIndex::aod_capacity as a known gap.
The fallback discards instead of fixing. Since feat(search)!: land the search-crate refactor #1089, finish_with_push_rotate checks each layer with LaneIndex::admits_shot and drops an over-cap plan, keeping the search's own verdict. That keeps capped results legal, but on a capped architecture the fallback cannot recover a stage whose P&R plan has a wide shot.
Direct use isn't checked at all. Neither the replay check (search/verify.rs) nor LaneIndex::check_lanes / check_move_set checks the capacity. A caller using Strategy::PushRotate directly, or a Rust caller of solve_push_rotate, can get an over-cap plan back with no error.
RANKED's score can be optimistic. NoHome ranks candidates by P&R plan length. On a capped architecture an uncapped plan can report fewer layers than the capped router could achieve, which skews the ranking. The router's plans are still capped, so results stay legal.
Proposal:
Scheduler: give the scheduler the capacity, and split a batch that exceeds it into shots that fit. With item 1's planner object, the capacity is naturally a planner parameter.
Validators: check it in the validators too. admits_shot in check_lanes, or in the replay check, would make an over-cap plan a validation error everywhere rather than silently accepted.
Fallback: then drop, or downgrade to a debug assertion, the fallback's discard.
Impact today: neither shipped Gemini spec sets aod_capacity, so nothing in the current pipeline or benchmarks hits this. It matters once a capacity is set: #1064 sets it from the v2 ArchBuilder, and user specs may set it.
Suggested order
Profile the RANKED stage cost from item 1.
Introduce the planner object, which also makes a natural home for the capacity.
Thread the capacity through the scheduler and validators, with tests on a capped spec: a P&R plan never exceeds the cap, and the fallback recovers a capped stage.
Follow-up from the #1089 review (its "finding 8", plus the AOD capacity gap it left open). Both concern Push and Rotate (P&R) as NoHome's
RANKEDmover selection uses it (#1083), and both change the planner's API, so they belong together.1. Rebuild the planner's per-stage state once, not per candidate
RANKEDplans every candidate target in a CZ stage, up tomax_mover_candidates(64), with a separate one-shot call:solve_with_engine(… Strategy::PushRotate …)→solve_push_rotate(index, initial, target, blocked, budget). Each call rebuilds its planner state from scratch:LaneGraph::build(index, blocked): walks every lane of the architecture, drops blocked sites, sorts and deduplicates the adjacency lists.Decomposition::build(graph, occupancy(start)): connected components, subgraphs, and per-component empty-site counts.Only the target half of step 3 and step 4 depend on the candidate. The lane graph depends only on the architecture and
blocked, and the decomposition only on the graph and the starting occupancy. Both are identical for every candidate in a stage, yet they're rebuilt up to 64 times.Proposal: a reusable planner, e.g.
PushRotatePlanner::new(index, blocked, initial)and then.plan(target)per candidate, withsolve_push_rotatekept as a thin wrapper. This is a planner API change: today's public entry points take the whole problem in one call.Expected effect: compile time only. Graph and decomposition are deterministic, so every plan and result stays identical, and the behaviour net and benchmark baselines should not move. The #1089 review measured
RANKEDat about 14% more compile time thanRULE. How much of that is this rebuilding, as opposed to the second routing solve for the rule's candidate and the planning itself, is unmeasured, so profile first: time oneRANKEDstage split into graph build, decomposition, planning and routing.2. Enforce the AOD capacity in the planner
ArchSpec.aod_capacity(#1061) caps the source columns and rows one AOD shot may use. Every search generator honours it. P&R does not:push_rotate/schedule.rspacks the largest ready batch into one rectangle without consulting a capacity, soStrategy::PushRotatecan return a shot wider than the cap. This is documented onLaneIndex::aod_capacityas a known gap.finish_with_push_rotatechecks each layer withLaneIndex::admits_shotand drops an over-cap plan, keeping the search's own verdict. That keeps capped results legal, but on a capped architecture the fallback cannot recover a stage whose P&R plan has a wide shot.search/verify.rs) norLaneIndex::check_lanes/check_move_setchecks the capacity. A caller usingStrategy::PushRotatedirectly, or a Rust caller ofsolve_push_rotate, can get an over-cap plan back with no error.RANKED's score can be optimistic. NoHome ranks candidates by P&R plan length. On a capped architecture an uncapped plan can report fewer layers than the capped router could achieve, which skews the ranking. The router's plans are still capped, so results stay legal.Proposal:
admits_shotincheck_lanes, or in the replay check, would make an over-cap plan a validation error everywhere rather than silently accepted.Impact today: neither shipped Gemini spec sets
aod_capacity, so nothing in the current pipeline or benchmarks hits this. It matters once a capacity is set: #1064 sets it from the v2ArchBuilder, and user specs may set it.Suggested order
RANKEDstage cost from item 1.