diff --git a/docs/vtrees.md b/docs/vtrees.md index 49cee65..436a45d 100644 --- a/docs/vtrees.md +++ b/docs/vtrees.md @@ -45,6 +45,12 @@ caller can read which of those happened: `VtreeBuild::limits` lists the builds that finished, the builds the budget cut short, the time they spent and the candidates never started. +If the budget is already spent when the walk starts and nothing has been built +yet — which happens when preprocessing used it up, or when earlier components +did — the first candidate still gets one attempt under a fixed one-second wall, +and the candidates behind it are reported as never started. That build returns a +tree rather than failing the construction. + `VtreeBuild::construction_ms` reports the broader end-to-end construction wall from the library entry through the finished whole or grafted tree. It includes setup, simple constructors and component grafting that are deliberately outside diff --git a/src/component/mod.rs b/src/component/mod.rs index 7802dfb..6f1f3d0 100644 --- a/src/component/mod.rs +++ b/src/component/mod.rs @@ -521,11 +521,11 @@ fn build_per_component( // between the components by clause count. // // A component whose share is already zero starts expired, and portfolio - // answers that with a construction error — so there is no separate - // deadline check here, and the error propagates straight out of this - // loop (`?` below), aborting the whole multi-component build. Tiny - // components skip this: minfill takes no deadline and cannot fail this - // way. + // answers that by giving its first candidate one short attempt and + // reporting the rest as never started — so there is no separate deadline + // check here, and a budget spent by the earlier components costs the later + // ones the rest of their catalog rather than the build. Tiny components + // skip this: minfill takes no deadline and never consults one. let mut clauses_left: usize = comps.iter().map(|c| c.len()).sum(); for comp_indices in comps { let comp_deadline = limits diff --git a/src/config/mod.rs b/src/config/mod.rs index b6c1949..b8ec559 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -784,10 +784,9 @@ impl RunConfig { } if self.construction_budget == (ConstructionBudget::Deterministic { units: 0 }) { return Err(VitriError::config( - "a deterministic construction budget of 0 work units leaves construction \ - nothing to spend, so no vtree could be built — pass the work a construction \ - should be allowed to do, which ConstructionBudget::for_wall_ms converts from \ - a wall in milliseconds", + "a deterministic construction budget of 0 work units asks construction to do \ + no work at all — pass the work a construction should be allowed to do, which \ + ConstructionBudget::for_wall_ms converts from a wall in milliseconds", )); } if self.candidates == 0 { diff --git a/src/decompose/portfolio/catalog.rs b/src/decompose/portfolio/catalog.rs index cba7442..f225e8c 100644 --- a/src/decompose/portfolio/catalog.rs +++ b/src/decompose/portfolio/catalog.rs @@ -318,11 +318,16 @@ pub(super) struct RunState { /// wall it never reaches. The deadline is otherwise consulted only between /// entries, which cannot stop the one that has already begun — and that is /// the entry which overruns the ceiling. + /// + /// The one exception is the attempt the driver allows when the deadline is + /// already spent and nothing has been built: there the share and the wall + /// are both a fixed short number, because what is left is zero or less. pub(super) cand_wall_ms: Option, - /// Latched once some entry has overrun its own fair share. Until it latches - /// every entry is bounded only by the whole remaining budget; after it - /// latches the remaining FlowCutter builds are additionally tightened to the - /// fair share, and take the tight search with it (see `fc_time_cap_ms` and + /// Latched once some entry has overrun its own fair share, and set outright + /// for the one attempt a spent deadline allows. Until it latches every entry + /// is bounded only by the whole remaining budget; after it latches the + /// remaining FlowCutter builds are additionally tightened to the fair share, + /// and take the tight search with it (see `fc_time_cap_ms` and /// `fc_cap_mode`). pub(super) behind_schedule: bool, pub(super) flowcutter_incidence_td_cache: Option, @@ -479,9 +484,10 @@ impl RunState { /// /// Three sources, and the tightest wins: /// - `cand_wall_ms`, the time actually left in the construction budget when - /// this entry started. Under a deadline this is always armed, the first - /// entry included, which is what makes the budget a ceiling rather than a - /// suggestion. + /// this entry started — or the fixed short wall of the one attempt a spent + /// deadline allows, where the time left is zero or less. Under a deadline + /// this is always armed, the first entry included, which is what makes the + /// budget a ceiling rather than a suggestion. /// - `cand_cap_ms`, this entry's fair share, once `behind_schedule` has /// latched. That is the scheduling tightening the latch has always /// applied; it no longer decides whether a cap exists at all. @@ -503,7 +509,8 @@ impl RunState { /// Tightness changes what the search considers, not only when it stops (see /// [`WallCapMode`]), so it is keyed on the two conditions that mean the /// build is already in the regime where finishing beats searching: - /// - `behind_schedule` — some entry has already overrun its fair share; + /// - `behind_schedule` — some entry has already overrun its fair share, or + /// this is the one attempt a spent deadline allows; /// - `flowcutter_cap_ms` — the projected large-component cap, whose whole /// purpose is to cut a grinding `flowcutter-primal` short. /// diff --git a/src/decompose/portfolio/driver.rs b/src/decompose/portfolio/driver.rs index 1d84078..c06a268 100644 --- a/src/decompose/portfolio/driver.rs +++ b/src/decompose/portfolio/driver.rs @@ -1,7 +1,9 @@ //! The portfolio driver: run the catalog, select a winner, publish the result. //! -//! A construction budget that leaves nothing built is a hard error: no -//! fallback stands between an exhausted budget and +//! A construction budget that leaves nothing built is a hard error. A budget +//! already spent when the walk starts is not that case: the entry the walk +//! stops at gets one attempt under a fixed short wall, and only a budget under +//! which that attempt also produces nothing reaches //! `Err(VitriError::construction(..))`. //! //! **Determinism:** what a portfolio build produces is a function of the @@ -48,6 +50,15 @@ use super::catalog::{ gate_hypergraph_bisect, outspent, work_ms_since, }; +/// The wall one catalog entry gets when the construction deadline is already +/// spent and nothing has been built. +/// +/// It is a fixed number rather than a share of what is left, because what is +/// left is zero or less. Short enough that a build already over its budget does +/// not go far past it, and long enough for the first entry — an anytime cutter +/// under a timed budget — to return a decomposition. +const LAST_ATTEMPT_MS: i64 = 1_000; + /// One build's wall report: a build that left candidates unstarted is the /// truncated one, and a build that walked the whole catalog is the complete /// one. Stated here rather than at the call site so the rule can be asked @@ -326,18 +337,42 @@ pub(crate) fn vtree_from_portfolio( } // A deadline already passed on entry — common once a multi-component build - // has spent its budget on earlier components — skips the whole catalog on - // the first iteration, so construction fails outright. + // has spent its budget on earlier components — would skip the whole catalog + // on the first iteration and fail the construction outright. A candidate + // that could have been built is worth more than the deadline it misses, so + // the entry the loop stopped at gets one attempt under a fixed short wall + // when nothing has been built yet; the rest are skipped either way. let mut skipped: Vec<&'static str> = Vec::new(); + let mut last_attempt = false; for (i, c) in catalog.iter().enumerate() { if inp.out_of_time() { - skipped.extend(catalog[i..].iter().map(|c| c.name)); - break; + // Both, because which of the two a built candidate lands in depends + // on the mode: plain selection adopts into `best`, projected + // selection collects into `cands` and chooses at the end. + if run.best.vtree.is_none() && run.cands.is_empty() { + diag!( + "[portfolio] deadline spent with nothing built; {} gets {LAST_ATTEMPT_MS}ms", + c.name, + ); + last_attempt = true; + } else { + skipped.extend(catalog[i..].iter().map(|c| c.name)); + break; + } + } + if last_attempt { + run.cand_cap_ms = Some(LAST_ATTEMPT_MS); + run.cand_wall_ms = Some(LAST_ATTEMPT_MS); + // The regime where finishing beats searching, which is what this + // attempt is: the wall is the whole budget it has. + run.behind_schedule = true; + } else { + run.cand_cap_ms = inp.fair_share_ms(catalog.len() - i); + // The hard bound: whatever is still left of the whole construction + // budget. `out_of_time` above has already ruled out a non-positive + // one. + run.cand_wall_ms = inp.remaining_ms().map(|r| r.max(1)); } - run.cand_cap_ms = inp.fair_share_ms(catalog.len() - i); - // The hard bound: whatever is still left of the whole construction - // budget. `out_of_time` above has already ruled out a non-positive one. - run.cand_wall_ms = inp.remaining_ms().map(|r| r.max(1)); // Where this entry's slice starts, on the construction clock: what the // latch below decides — whether the entries behind this one search less // patiently — is a decision about which tree comes out, so it is @@ -354,6 +389,12 @@ pub(crate) fn vtree_from_portfolio( if open && let Some(built) = (c.build)(&inp, &mut run) { run.fold(&inp, c, built); } + // One attempt is all a spent deadline buys, whether or not it produced + // anything: the entries behind it are skipped. + if last_attempt { + skipped.extend(catalog[i + 1..].iter().map(|c| c.name)); + break; + } if run .cand_cap_ms .is_some_and(|cap| (work_ms_since(slice_start) as i64) > cap) diff --git a/src/decompose/portfolio/tests/driver.rs b/src/decompose/portfolio/tests/driver.rs index 70faae0..33f671c 100644 --- a/src/decompose/portfolio/tests/driver.rs +++ b/src/decompose/portfolio/tests/driver.rs @@ -138,34 +138,46 @@ fn budget_fixture() -> crate::cnf::CnfFormula { crate::tests::circuit_fixture::multiplier() } -/// SPENT BUDGET IS A HARD ERROR: a deadline that has ALREADY passed on entry -/// skips every catalog candidate, and the build fails outright rather than -/// handing back a degraded vtree. +/// A SPENT BUDGET STILL BUILDS ONE CANDIDATE: a deadline that has ALREADY +/// passed on entry leaves no share for anything, but a tree the caller can use +/// is worth more than the deadline it misses. The first catalog entry runs +/// under a fixed short wall and every entry behind it is reported as never +/// started, which is how a caller tells this tree from a complete one. +/// +/// The spent deadline is constructed, not waited for: an `Instant` already in +/// the past is past on entry on any machine, so the case under test is reached +/// without timing anything. #[test] -fn expired_deadline_is_a_construction_error() { +fn an_expired_deadline_still_builds_the_first_candidate() { use std::time::{Duration, Instant}; let formula = budget_fixture(); let limits = BuildLimits { deadline: Some(Instant::now() - Duration::from_secs(1)), ..BuildLimits::default() }; - // Matched by hand rather than `.expect_err()`: `VtreeArtifacts` (the `Ok` - // side) carries an `Arc` and does not derive `Debug`, which - // `.expect_err()`'s bound would otherwise require adding just for this test. - match vtree_from_portfolio( + let built = vtree_from_portfolio( &formula, 150_000, 15, Reading::default(), &SelectionCtx::plain(), &limits, - ) { - Ok(_) => panic!("an already-spent deadline must fail construction, not build a vtree"), - Err(e) => assert!( - matches!(e, crate::error::VitriError::Construction { .. }), - "expected a construction error, got {e:?}", - ), - } + ) + .expect("a spent deadline must still hand back a vtree"); + assert_eq!( + built.vtree.num_leaves(), + formula.num_vars, + "the tree must cover the formula", + ); + assert_eq!( + built.limits.truncated_builds, 1, + "a build that left catalog entries unstarted is the truncated one", + ); + let behind_the_first: Vec = catalog().iter().skip(1).map(|c| c.name.into()).collect(); + assert_eq!( + built.limits.skipped, behind_the_first, + "one attempt is all a spent deadline buys: every entry behind it is never started", + ); } /// NO BEHAVIOR DRIFT: a deadline far beyond what construction needs must @@ -213,6 +225,17 @@ fn generous_deadline_matches_no_deadline() { unbounded.vtree.to_vtree_text(), "a generous budget changed the constructed vtree", ); + // The other side of the fallback above: with time left on entry the walk is + // the ordinary fair-share one, so nothing is skipped and no entry is cut + // down to the one-attempt wall. + assert!( + bounded.limits.skipped.is_empty(), + "a budget with time left must walk the whole catalog", + ); + assert_eq!( + bounded.limits.complete_builds, 1, + "a build that walked the whole catalog is the complete one", + ); } /// Tiny dummy ScoredCandidate (the vtree is never inspected by select_peak_band). diff --git a/src/tests/bundle/components/candidates.rs b/src/tests/bundle/components/candidates.rs index b83d126..14665a4 100644 --- a/src/tests/bundle/components/candidates.rs +++ b/src/tests/bundle/components/candidates.rs @@ -171,18 +171,18 @@ fn what_the_run_config_allows_reaches_the_portfolio() { "the run config's candidate width must reach the portfolio", ); - // The same config with an already-spent deadline: the construction that - // succeeded above can now only fail by obeying it. Matched by hand rather - // than `expect_err`: the `Ok` side does not derive `Debug`. + // The same config with an already-spent deadline. The build still returns a + // vtree — the portfolio gives its first candidate one short attempt rather + // than skipping the whole catalog — but it obeys the deadline by leaving the + // rest of the catalog unstarted, which a complete build never reports. let spent = RunConfig { deadline: Some(std::time::Instant::now()), ..candidates_config(1) }; - match build_vtree(&formula, &spent, &SelectionCtx::plain()) { - Ok(_) => panic!("a spent deadline on the run config must bound the construction"), - Err(err) => assert!( - matches!(err, crate::error::VitriError::Construction { .. }), - "expected a construction error, got {err:?}", - ), - } + let bounded = build_vtree(&formula, &spent, &SelectionCtx::plain()) + .expect("a spent deadline must still hand back a vtree"); + assert!( + !bounded.limits.skipped.is_empty(), + "a spent deadline on the run config must bound the construction", + ); } diff --git a/src/tests/bundle/run.rs b/src/tests/bundle/run.rs index 4daa48c..0b3bbc5 100644 --- a/src/tests/bundle/run.rs +++ b/src/tests/bundle/run.rs @@ -286,10 +286,11 @@ fn a_refuted_run_writes_the_bundle_and_names_no_vtree() { /// construction budget that cannot build anything never reaches the run. /// /// One work unit buys no construction at all: the budget is spent at the -/// instant construction would start, every portfolio entry is skipped, and -/// nothing is built — which the irreducible instance below shows is a hard -/// error. The refuted instance takes the same configuration and succeeds, -/// because construction is never asked for a vtree over it. +/// instant construction would start. The irreducible instance below shows what +/// that costs — the portfolio gives its first candidate one short attempt and +/// leaves the rest of the catalog unstarted. The refuted instance takes the +/// same configuration and reports no construction at all, because construction +/// is never asked for a vtree over it. #[test] fn a_refuted_run_answers_before_construction_can_be_asked_for_a_vtree() { let config = RunConfig { @@ -298,11 +299,14 @@ fn a_refuted_run_answers_before_construction_can_be_asked_for_a_vtree() { }; let (formula, meta) = parse(IRREDUCIBLE_5); - let err = run(&formula, &meta, &config, &SelectionCtx::plain()) - .expect_err("one work unit must leave the portfolio nothing to build with"); + let produced = run(&formula, &meta, &config, &SelectionCtx::plain()) + .expect("one work unit still buys the first candidate its one attempt"); + let RunVtree::Built(built) = &produced.vtree else { + panic!("the irreducible instance reaches construction and is built over"); + }; assert!( - matches!(err, VitriError::Construction { .. }), - "the exhausted construction budget must be what fails, got: {err:?}", + !built.limits.skipped.is_empty(), + "a construction budget spent at entry leaves the rest of the catalog unstarted", ); let (formula, meta) = parse(REFUTED); diff --git a/src/tests/component.rs b/src/tests/component.rs index 2c142f7..dcf89b6 100644 --- a/src/tests/component.rs +++ b/src/tests/component.rs @@ -346,9 +346,9 @@ fn a_variable_no_clause_names_still_gets_exactly_one_leaf() { assert_covers_all_vars(&built.vtree, formula.num_vars, "the graft"); } -/// The tiny-component construction takes no deadline, so a build that starts -/// with none left still produces a vtree — the complement of the larger -/// components, for which a spent deadline is a construction error. +/// The tiny-component construction takes no deadline at all, so a build that +/// starts with none left still produces a vtree, and by the min-fill shortcut +/// rather than by the portfolio's one short attempt. #[test] fn a_tiny_component_builds_even_with_no_budget_left() { let formula = chain_components(&[5, 6]); @@ -375,18 +375,21 @@ fn a_tiny_component_builds_even_with_no_budget_left() { } /// The construction budget at the layer a caller actually uses: a build handed -/// a deadline that has ALREADY passed fails with a construction error, on the -/// single-component path and on the split alike. The complement of the tiny -/// component above, whose min-fill shortcut takes no deadline at all. +/// a deadline that has ALREADY passed still returns a vtree, on the +/// single-component path and on the split alike. The portfolio gives its first +/// candidate one short attempt instead of skipping the whole catalog, so a +/// spent budget costs the caller the rest of the catalog rather than the tree. /// /// The single chain can be small, because the deadline is spent before /// construction starts and nothing about the formula's shape is ever reached. -/// The pair has to be 32 variables each: at or under the tiny threshold a -/// component takes the min-fill shortcut and cannot fail this way. Only the -/// first of the two ever runs, since the deadline is one absolute budget shared -/// across the whole split. +/// The pair has to be over 32 variables each: at or under the tiny threshold a +/// component takes the min-fill shortcut, which never consults the deadline and +/// so would not exercise this at all. The two differ in size so that neither is +/// served from the identical-component cache — both are constructed, which is +/// what makes the count below the number of attempts taken. The deadline is one +/// absolute budget shared across the split, so both start past it. #[test] -fn expired_vtree_deadline_fails_construction() { +fn an_expired_vtree_deadline_still_builds_a_vtree() { // An already-gone run deadline: the construction deadline the entry point // derives from it is gone too. let expired = RunConfig { @@ -394,30 +397,27 @@ fn expired_vtree_deadline_fails_construction() { ..RunConfig::default() }; - // Single component. Matched by hand rather than `.expect_err()`: `VtreeBuild` - // (the `Ok` side) carries an `Arc` and does not derive `Debug`, which - // `.expect_err()`'s bound would otherwise require adding just for this test. let formula = chain_components(&[9]); - match build_vtree(&formula, &expired, &SelectionCtx::plain()) { - Ok(_) => panic!("an already-spent deadline must fail construction, not build a vtree"), - Err(err) => assert!( - matches!(err, crate::error::VitriError::Construction { .. }), - "expected a construction error, got {err:?}", - ), - } - - // Two independent components, both big enough to reach portfolio: the - // deadline is absolute and shared, so the first component already starts - // past it, and the whole build fails before the second component is ever - // reached. - let two = chain_components(&[32, 32]); - let err2 = match build_vtree(&two, &expired, &SelectionCtx::plain()) { - Ok(_) => panic!("an already-spent deadline must fail a multi-component build too"), - Err(err) => err, - }; + let built = build_vtree(&formula, &expired, &SelectionCtx::plain()) + .expect("a spent deadline must still hand back a vtree"); + assert_covers_all_vars(&built.vtree, formula.num_vars, "the single-component build"); assert!( - matches!(err2, crate::error::VitriError::Construction { .. }), - "expected a construction error, got {err2:?}", + !built.limits.skipped.is_empty(), + "the candidates behind the one attempt must be reported as never started", + ); + + let two = chain_components(&[32, 33]); + let grafted = build_vtree(&two, &expired, &SelectionCtx::plain()) + .expect("a spent deadline must still hand back a grafted vtree"); + assert_covers_all_vars(&grafted.vtree, two.num_vars, "the graft"); + assert_eq!( + grafted.components.as_ref().map(Vec::len), + Some(2), + "two independent chains are two components", + ); + assert_eq!( + grafted.limits.truncated_builds, 2, + "each component's build takes its own attempt and leaves the rest of the catalog unstarted", ); } diff --git a/tests/cli/success_path.rs b/tests/cli/success_path.rs index 0840f33..7569a03 100644 --- a/tests/cli/success_path.rs +++ b/tests/cli/success_path.rs @@ -356,22 +356,21 @@ fn a_budget_hint_is_accepted() { assert!(out.join(VTREE_NAME).exists()); } -/// A budget spent before vtree construction even starts is a hard failure, not -/// a degraded run: `--budget-ms 0` sets the whole-run deadline to the instant -/// the process started, so it has already passed by the time preprocessing hands -/// off to vtree construction. The run must exit 1 (a construction failure, not -/// a bad invocation) and leave the output directory untouched — no partial -/// bundle, because nothing is written until after the vtree is built. +/// A budget spent before vtree construction even starts is a degraded run +/// rather than a hard failure: `--budget-ms 0` sets the whole-run deadline to +/// the instant the process started, so it has already passed by the time +/// preprocessing hands off to vtree construction. The portfolio gives its first +/// candidate one short attempt instead of skipping the whole catalog, so the run +/// exits 0 and writes the bundle it was asked for. #[test] -fn a_spent_budget_fails_construction_and_writes_nothing() { +fn a_spent_budget_still_builds_a_vtree_and_writes_its_bundle() { let t = Scratch::new("spent-budget"); let input = t.file("in.cnf", IRREDUCIBLE_5); let out = t.out("bundle"); - let run = run(&[s(&input), "-o", s(&out), "--budget-ms", "0"]).exit(1); - run.assert_stderr("every candidate failed"); + run(&[s(&input), "-o", s(&out), "--budget-ms", "0"]).exit(0); assert!( - !out.exists(), - "the output directory must not be created when construction fails", + out.join(VTREE_NAME).exists(), + "a spent budget must still leave the caller a vtree", ); }