From df48d5a77c02e6a5d845d31f4bdb4eb662ba46f8 Mon Sep 17 00:00:00 2001 From: Vittorio Distefano Date: Wed, 4 Nov 2026 22:08:25 +0100 Subject: [PATCH 1/3] fix(termination)!: make time_limit literal and saturating `seconds_spent_limit = 0` (or `minutes_spent_limit = 0`) meant no limit, and `minutes_spent_limit * 60` could overflow. `TerminationConfig::time_limit` is now `None` only when neither field is set, so zero is a limit of zero, and the sum saturates at `Duration::MAX`. BREAKING CHANGE: `seconds_spent_limit = 0` or `minutes_spent_limit = 0` is now a zero time limit instead of no limit. --- crates/solverforge-config/WIREFRAME.md | 2 +- crates/solverforge-config/src/termination.rs | 20 ++++++++-------- .../solverforge-config/src/tests/parsing.rs | 23 +++++++++++++++++++ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/crates/solverforge-config/WIREFRAME.md b/crates/solverforge-config/WIREFRAME.md index 0ebfcb52..5d236104 100644 --- a/crates/solverforge-config/WIREFRAME.md +++ b/crates/solverforge-config/WIREFRAME.md @@ -112,7 +112,7 @@ Derives: `Debug, Clone, Default, Deserialize, Serialize`. | Method | Signature | Note | |--------|-----------|------| -| `time_limit` | `fn(&self) -> Option` | Combines seconds + minutes × 60 | +| `time_limit` | `fn(&self) -> Option` | Seconds + minutes × 60, saturating at `Duration::MAX`; `None` only when neither is set, so `0` is a zero limit | | `unimproved_time_limit` | `fn(&self) -> Option` | Maps unimproved seconds to Duration | ### `DirectorConfig` diff --git a/crates/solverforge-config/src/termination.rs b/crates/solverforge-config/src/termination.rs index ffd057c1..18fad013 100644 --- a/crates/solverforge-config/src/termination.rs +++ b/crates/solverforge-config/src/termination.rs @@ -6,10 +6,10 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, Default, Deserialize, Serialize)] #[serde(rename_all = "snake_case", deny_unknown_fields)] pub struct TerminationConfig { - /// Maximum seconds to spend solving. + /// Maximum seconds to spend solving; added to `minutes_spent_limit`. pub seconds_spent_limit: Option, - /// Maximum minutes to spend solving. + /// Maximum minutes to spend solving; added to `seconds_spent_limit`. pub minutes_spent_limit: Option, /// Target best score to achieve (as string, e.g., "0hard/0soft"). @@ -26,16 +26,16 @@ pub struct TerminationConfig { } impl TerminationConfig { - /// Sum of `seconds_spent_limit` and `minutes_spent_limit`; `None` when the - /// sum is zero. + /// Sum of `seconds_spent_limit` and `minutes_spent_limit`; `None` when + /// neither is set. Zero is a limit of zero; a sum past `Duration::MAX` + /// saturates there. pub fn time_limit(&self) -> Option { - let seconds = - self.seconds_spent_limit.unwrap_or(0) + self.minutes_spent_limit.unwrap_or(0) * 60; - if seconds > 0 { - Some(Duration::from_secs(seconds)) - } else { - None + if self.seconds_spent_limit.is_none() && self.minutes_spent_limit.is_none() { + return None; } + let seconds = Duration::from_secs(self.seconds_spent_limit.unwrap_or(0)); + let minutes = Duration::from_secs(self.minutes_spent_limit.unwrap_or(0)).saturating_mul(60); + Some(seconds.saturating_add(minutes)) } /// `unimproved_seconds_spent_limit` as a `Duration`. diff --git a/crates/solverforge-config/src/tests/parsing.rs b/crates/solverforge-config/src/tests/parsing.rs index fdd39e12..084d1492 100644 --- a/crates/solverforge-config/src/tests/parsing.rs +++ b/crates/solverforge-config/src/tests/parsing.rs @@ -280,3 +280,26 @@ fn score_director_defaults_to_the_constraints_function_with_analysis_enabled() { ); assert!(!set.constraint_match_enabled()); } + +#[test] +fn time_limit_is_literal_and_saturating() { + use crate::TerminationConfig; + use std::time::Duration; + + let limit = |seconds, minutes| { + TerminationConfig { + seconds_spent_limit: seconds, + minutes_spent_limit: minutes, + ..TerminationConfig::default() + } + .time_limit() + }; + assert_eq!(limit(None, None), None); + assert_eq!(limit(Some(0), None), Some(Duration::ZERO)); + assert_eq!(limit(None, Some(0)), Some(Duration::ZERO)); + assert_eq!(limit(Some(30), Some(2)), Some(Duration::from_secs(150))); + assert_eq!( + limit(Some(u64::MAX), Some(u64::MAX)), + Some(Duration::from_secs(u64::MAX).saturating_add(Duration::MAX)) + ); +} From 9a27c3bcd87c59cbefc730a5d90a3cfd69ba426e Mon Sep 17 00:00:00 2001 From: Vittorio Distefano Date: Wed, 4 Nov 2026 22:10:02 +0100 Subject: [PATCH 2/3] fix(termination)!: apply every configured solver termination criterion Solver-level `[termination]` honoured only the first of `best_score_limit`, `step_count_limit`, `unimproved_step_count_limit` and `unimproved_seconds_spent_limit`, paired with the time limit; the others were silently ignored, while phase-level termination combined all of them. `ConfiguredSolverTermination` now holds every configured criterion as its own concrete termination and stops the solve at whichever is reached first. It replaces `AnyTermination` and the tuple `OrTermination`. The candidate trace execution policy lists every installed criterion. BREAKING CHANGE: every configured solver-level termination criterion now applies, combined with OR; a solve with several criteria can stop earlier than before. The `solverforge.execution_policy` trace drops the `criterion` and `criterion_target` attributes and gains one attribute per criterion; `termination_composition` names every installed criterion. --- crates/solverforge-config/WIREFRAME.md | 3 + crates/solverforge-config/src/termination.rs | 3 +- crates/solverforge-solver/WIREFRAME.md | 1 - crates/solverforge-solver/src/run.rs | 60 +-- .../solverforge-solver/src/run/termination.rs | 360 +++++++++--------- .../src/run/weight_modes.rs | 26 +- crates/solverforge-solver/src/run_tests.rs | 166 ++++---- .../src/termination/composite.rs | 69 ---- .../solverforge-solver/src/termination/mod.rs | 2 - .../src/termination/tests.rs | 20 - 10 files changed, 278 insertions(+), 432 deletions(-) delete mode 100644 crates/solverforge-solver/src/termination/composite.rs diff --git a/crates/solverforge-config/WIREFRAME.md b/crates/solverforge-config/WIREFRAME.md index 5d236104..39d3d027 100644 --- a/crates/solverforge-config/WIREFRAME.md +++ b/crates/solverforge-config/WIREFRAME.md @@ -99,6 +99,9 @@ Constructor: `const fn new(max_entries: NonZeroUsize) -> Self`. Derives: `Debug, Clone, Default, Deserialize, Serialize`. +Every configured criterion applies, combined with OR, at solver and phase +level alike: the solve or phase stops at whichever is reached first. + | Field | Type | Note | |-------|------|------| | `seconds_spent_limit` | `Option` | Max seconds | diff --git a/crates/solverforge-config/src/termination.rs b/crates/solverforge-config/src/termination.rs index 18fad013..94d2ce20 100644 --- a/crates/solverforge-config/src/termination.rs +++ b/crates/solverforge-config/src/termination.rs @@ -2,7 +2,8 @@ use std::time::Duration; use serde::{Deserialize, Serialize}; -/// Termination configuration. +/// Termination configuration. Every configured criterion applies, combined +/// with OR: the solve (or phase) stops at whichever is reached first. #[derive(Debug, Clone, Default, Deserialize, Serialize)] #[serde(rename_all = "snake_case", deny_unknown_fields)] pub struct TerminationConfig { diff --git a/crates/solverforge-solver/WIREFRAME.md b/crates/solverforge-solver/WIREFRAME.md index 019b9613..c587e53b 100644 --- a/crates/solverforge-solver/WIREFRAME.md +++ b/crates/solverforge-solver/WIREFRAME.md @@ -384,7 +384,6 @@ src/ │ ├── step_count.rs — StepCountTermination │ ├── best_score.rs — BestScoreTermination │ ├── unimproved.rs — UnimprovedStepCountTermination, UnimprovedTimeTermination -│ ├── composite.rs — OrTermination (tuple impls up to 8) │ └── tests.rs — Tests │ └── realtime/ diff --git a/crates/solverforge-solver/src/run.rs b/crates/solverforge-solver/src/run.rs index 6dd02afd..fc935f6a 100644 --- a/crates/solverforge-solver/src/run.rs +++ b/crates/solverforge-solver/src/run.rs @@ -25,35 +25,22 @@ use crate::runtime::compiler::executor::{ use crate::runtime::compiler::{compile_runtime_graph, CompiledRuntimeExecutor, RuntimeGraphInput}; use crate::runtime_build_error::{RuntimeBuildError, RuntimeBuildResult}; use crate::scope::{ProgressCallback, SolverProgressKind, SolverProgressRef, SolverScope}; -use crate::solver::{NoTermination, Solver}; +use crate::solver::Solver; use crate::stats::{ format_duration, whole_units_per_second, CandidateTraceExecutionPolicy, QualifiedCandidateTraceRunProvenance, }; use crate::termination::{ - BestScoreTermination, OrTermination, StepCountTermination, Termination, TimeTermination, + BestScoreTermination, StepCountTermination, Termination, TimeTermination, UnimprovedStepCountTermination, UnimprovedTimeTermination, }; pub(crate) mod termination; -use termination::build_director_termination; +use termination::{build_solver_termination, ConfiguredSolverTermination}; pub(crate) use termination::{ configured_best_score_limit, configured_execution_policy, parse_configured_termination, }; -/// Monomorphized termination enum for config-driven solver configurations. -/// -/// Avoids repeated branching across termination overloads by capturing the -/// selected termination variant upfront. -pub(crate) enum AnyTermination> { - None(NoTermination), - Default(OrTermination<(TimeTermination,), S, D>), - WithBestScore(OrTermination<(TimeTermination, BestScoreTermination), S, D>), - WithStepCount(OrTermination<(TimeTermination, StepCountTermination), S, D>), - WithUnimprovedStep(OrTermination<(TimeTermination, UnimprovedStepCountTermination), S, D>), - WithUnimprovedTime(OrTermination<(TimeTermination, UnimprovedTimeTermination), S, D>), -} - #[derive(Clone)] pub(crate) struct ChannelProgressCallback { runtime: SolverRuntime, @@ -99,47 +86,6 @@ impl ProgressCallback for ChannelProgressCallback { } } -impl> fmt::Debug for AnyTermination { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::None(_) => write!(f, "AnyTermination::None"), - Self::Default(_) => write!(f, "AnyTermination::Default"), - Self::WithBestScore(_) => write!(f, "AnyTermination::WithBestScore"), - Self::WithStepCount(_) => write!(f, "AnyTermination::WithStepCount"), - Self::WithUnimprovedStep(_) => write!(f, "AnyTermination::WithUnimprovedStep"), - Self::WithUnimprovedTime(_) => write!(f, "AnyTermination::WithUnimprovedTime"), - } - } -} - -impl, ProgressCb: ProgressCallback> - Termination for AnyTermination -where - S::Score: Score, -{ - fn is_terminated(&mut self, solver_scope: &SolverScope) -> bool { - match self { - Self::None(t) => t.is_terminated(solver_scope), - Self::Default(t) => t.is_terminated(solver_scope), - Self::WithBestScore(t) => t.is_terminated(solver_scope), - Self::WithStepCount(t) => t.is_terminated(solver_scope), - Self::WithUnimprovedStep(t) => t.is_terminated(solver_scope), - Self::WithUnimprovedTime(t) => t.is_terminated(solver_scope), - } - } - - fn install_inphase_limits(&self, solver_scope: &mut SolverScope) { - match self { - Self::None(t) => t.install_inphase_limits(solver_scope), - Self::Default(t) => t.install_inphase_limits(solver_scope), - Self::WithBestScore(t) => t.install_inphase_limits(solver_scope), - Self::WithStepCount(t) => t.install_inphase_limits(solver_scope), - Self::WithUnimprovedStep(t) => t.install_inphase_limits(solver_scope), - Self::WithUnimprovedTime(t) => t.install_inphase_limits(solver_scope), - } - } -} - /// Logs the `solve_start` tracing event with the entity count and exactly /// one solve scale: `element_count` for list models or `candidate_count` for /// scalar models. diff --git a/crates/solverforge-solver/src/run/termination.rs b/crates/solverforge-solver/src/run/termination.rs index 0f1915eb..41eb1a8d 100644 --- a/crates/solverforge-solver/src/run/termination.rs +++ b/crates/solverforge-solver/src/run/termination.rs @@ -1,31 +1,24 @@ use super::*; /// Parsed solver termination policy shared by runtime phase assembly and the -/// top-level termination builder. -/// -/// `TerminationConfig` historically chooses the first configured score/work -/// criterion in this order: best score, step count, unimproved steps, -/// unimproved time. A configured time limit is paired with that criterion, or -/// is the policy itself when no other criterion is present. Keeping that -/// precedence here prevents phase assembly from treating an empty or -/// unparsable configuration as a finite solver boundary. +/// top-level termination builder: every configured criterion, combined with +/// OR, so the solve stops at whichever is reached first. #[derive(Clone, Copy)] pub(crate) struct ConfiguredTermination { pub(super) time_limit: Option, - criterion: Option>, -} - -#[derive(Clone, Copy)] -enum ConfiguredTerminationCriterion { - BestScore(Sc), - StepCount(u64), - UnimprovedStepCount(u64), - UnimprovedTime(Duration), + best_score: Option, + step_count: Option, + unimproved_step_count: Option, + unimproved_time: Option, } impl ConfiguredTermination { pub(crate) fn has_effective_limit(&self) -> bool { - self.time_limit.is_some() || self.criterion.is_some() + self.time_limit.is_some() + || self.best_score.is_some() + || self.step_count.is_some() + || self.unimproved_step_count.is_some() + || self.unimproved_time.is_some() } } @@ -51,115 +44,150 @@ where S: PlanningSolution, S::Score: ParseableScore, { - let time_limit = config.and_then(TerminationConfig::time_limit); - let criterion = config.and_then(|config| { - configured_best_score_limit::(config) - .map(ConfiguredTerminationCriterion::BestScore) - .or_else(|| { - config - .step_count_limit - .map(ConfiguredTerminationCriterion::StepCount) - }) - .or_else(|| { - config - .unimproved_step_count_limit - .map(ConfiguredTerminationCriterion::UnimprovedStepCount) - }) - .or_else(|| { - config - .unimproved_time_limit() - .map(ConfiguredTerminationCriterion::UnimprovedTime) - }) - }); ConfiguredTermination { - time_limit, - criterion, + time_limit: config.and_then(TerminationConfig::time_limit), + best_score: config.and_then(configured_best_score_limit::), + step_count: config.and_then(|config| config.step_count_limit), + unimproved_step_count: config.and_then(|config| config.unimproved_step_count_limit), + unimproved_time: config.and_then(TerminationConfig::unimproved_time_limit), } } -/// Builds a termination from config, returning both the termination and the time limit. -pub(crate) fn build_director_termination( - config: &SolverConfig, - default_secs: u64, -) -> (AnyTermination, Option) +/// The solver-wide termination: every criterion of `[termination]`, combined +/// with OR. Each criterion is its own concrete termination; an absent one is +/// never checked. +pub(crate) struct ConfiguredSolverTermination { + time: Option, + best_score: Option>, + step_count: Option, + unimproved_step_count: Option>, + unimproved_time: Option>, +} + +impl fmt::Debug for ConfiguredSolverTermination { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ConfiguredSolverTermination") + .field("time", &self.time) + .field("best_score", &self.best_score.is_some()) + .field("step_count", &self.step_count) + .field("unimproved_step_count", &self.unimproved_step_count) + .field("unimproved_time", &self.unimproved_time.is_some()) + .finish() + } +} + +impl ConfiguredSolverTermination { + /// Whether no criterion is configured, so the solve runs until its phases + /// end or it is cancelled. + #[cfg(test)] + pub(crate) fn is_unbounded(&self) -> bool { + self.time.is_none() + && self.best_score.is_none() + && self.step_count.is_none() + && self.unimproved_step_count.is_none() + && self.unimproved_time.is_none() + } +} + +impl Termination for ConfiguredSolverTermination where S: PlanningSolution, - S::Score: Score + ParseableScore, + S::Score: Score, D: Director, + ProgressCb: ProgressCallback, { - let ConfiguredTermination { - time_limit: configured_time_limit, - criterion, - } = parse_configured_termination::(config.termination.as_ref()); - let fallback_time_limit = Duration::from_secs(default_secs); + fn is_terminated(&mut self, solver_scope: &SolverScope) -> bool { + // Every criterion is checked, so stateful ones (the unimproved + // trackers) observe each step even when another one fires first. + let mut terminated = false; + if let Some(time) = &mut self.time { + terminated |= Termination::::is_terminated(time, solver_scope); + } + if let Some(best_score) = &mut self.best_score { + terminated |= Termination::::is_terminated(best_score, solver_scope); + } + if let Some(step_count) = &mut self.step_count { + terminated |= Termination::::is_terminated(step_count, solver_scope); + } + if let Some(unimproved) = &mut self.unimproved_step_count { + terminated |= Termination::::is_terminated(unimproved, solver_scope); + } + if let Some(unimproved) = &mut self.unimproved_time { + terminated |= Termination::::is_terminated(unimproved, solver_scope); + } + terminated + } - let (termination, effective_time_limit) = match criterion { - Some(ConfiguredTerminationCriterion::BestScore(target)) => { - let effective_time_limit = configured_time_limit.unwrap_or(fallback_time_limit); - let time = TimeTermination::new(effective_time_limit); - ( - AnyTermination::WithBestScore(OrTermination::new(( - time, - BestScoreTermination::new(target), - ))), - Some(effective_time_limit), - ) + fn install_inphase_limits(&self, solver_scope: &mut SolverScope) { + if let Some(time) = &self.time { + Termination::::install_inphase_limits(time, solver_scope); } - Some(ConfiguredTerminationCriterion::StepCount(step_limit)) => { - let effective_time_limit = configured_time_limit.unwrap_or(fallback_time_limit); - let time = TimeTermination::new(effective_time_limit); - ( - AnyTermination::WithStepCount(OrTermination::new(( - time, - StepCountTermination::new(step_limit), - ))), - Some(effective_time_limit), - ) + if let Some(best_score) = &self.best_score { + Termination::::install_inphase_limits(best_score, solver_scope); } - Some(ConfiguredTerminationCriterion::UnimprovedStepCount(unimproved_step_limit)) => { - let effective_time_limit = configured_time_limit.unwrap_or(fallback_time_limit); - let time = TimeTermination::new(effective_time_limit); - ( - AnyTermination::WithUnimprovedStep(OrTermination::new(( - time, - UnimprovedStepCountTermination::::new(unimproved_step_limit), - ))), - Some(effective_time_limit), - ) + if let Some(step_count) = &self.step_count { + Termination::::install_inphase_limits(step_count, solver_scope); } - Some(ConfiguredTerminationCriterion::UnimprovedTime(unimproved_time)) => { - let effective_time_limit = configured_time_limit.unwrap_or(fallback_time_limit); - let time = TimeTermination::new(effective_time_limit); - ( - AnyTermination::WithUnimprovedTime(OrTermination::new(( - time, - UnimprovedTimeTermination::::new(unimproved_time), - ))), - Some(effective_time_limit), - ) + if let Some(unimproved) = &self.unimproved_step_count { + Termination::::install_inphase_limits(unimproved, solver_scope); } - None => configured_time_limit.map_or_else( - || (AnyTermination::None(NoTermination), None), - |limit| { - let time = TimeTermination::new(limit); - ( - AnyTermination::Default(OrTermination::new((time,))), - Some(limit), - ) - }, - ), - }; + if let Some(unimproved) = &self.unimproved_time { + Termination::::install_inphase_limits(unimproved, solver_scope); + } + } +} + +/// Builds the solver-wide termination from config, returning it and the +/// effective time limit. A score or work criterion without a configured time +/// limit is paired with the entrypoint's `default_secs` time guard. +pub(crate) fn build_solver_termination( + config: &SolverConfig, + default_secs: u64, +) -> (ConfiguredSolverTermination, Option) +where + S: PlanningSolution, + S::Score: Score + ParseableScore, +{ + let configured = parse_configured_termination::(config.termination.as_ref()); + let time_limit = effective_time_limit(&configured, default_secs); + ( + ConfiguredSolverTermination { + time: time_limit.map(TimeTermination::new), + best_score: configured.best_score.map(BestScoreTermination::new), + step_count: configured.step_count.map(StepCountTermination::new), + unimproved_step_count: configured + .unimproved_step_count + .map(UnimprovedStepCountTermination::new), + unimproved_time: configured + .unimproved_time + .map(UnimprovedTimeTermination::new), + }, + time_limit, + ) +} - (termination, effective_time_limit) +/// The configured time limit, or the entrypoint fallback when only a score or +/// work criterion is configured. +fn effective_time_limit( + configured: &ConfiguredTermination, + default_secs: u64, +) -> Option { + let has_criterion = configured.best_score.is_some() + || configured.step_count.is_some() + || configured.unimproved_step_count.is_some() + || configured.unimproved_time.is_some(); + configured + .time_limit + .or_else(|| has_criterion.then(|| Duration::from_secs(default_secs))) } -/// Records the termination policy the configured runtime actually installed. +/// Records the termination policy the configured runtime installed: the +/// effective time limit and every configured criterion, all combined with OR. /// -/// This deliberately derives its time guard from `build_termination`'s -/// returned effective limit rather than from the input TOML. In particular, -/// a score/work criterion without an explicit time limit gets the configured -/// entrypoint's fallback guard, and that injected guard is material to both -/// bounded-work and fixed-budget comparisons. +/// The time guard comes from `build_solver_termination`'s returned effective +/// limit rather than from the input TOML: a score or work criterion without +/// a time limit gets the entrypoint's fallback guard, and that injected guard +/// is material to both bounded-work and fixed-budget comparisons. pub(crate) fn configured_execution_policy( config: &SolverConfig, default_secs: u64, @@ -170,85 +198,75 @@ where S::Score: ParseableScore + std::fmt::Display, { let configured = parse_configured_termination::(config.termination.as_ref()); - let configured_time_limit = configured.time_limit; - let criterion = configured.criterion; - let fallback_time_limit = Duration::from_secs(default_secs); - - let time_limit_source = match (configured_time_limit, effective_time_limit) { + let none = || "none".to_string(); + let time_limit_source = match (configured.time_limit, effective_time_limit) { (Some(_), Some(_)) => "configured", - (None, Some(_)) if criterion.is_some() => "configured_entrypoint_fallback", - (None, Some(_)) => "internal", + (None, Some(_)) => "configured_entrypoint_fallback", (_, None) => "not_installed", }; - let mut attributes = vec![ + let mut criteria = Vec::new(); + if effective_time_limit.is_some() { + criteria.push("time"); + } + if configured.best_score.is_some() { + criteria.push("best_score"); + } + if configured.step_count.is_some() { + criteria.push("step_count"); + } + if configured.unimproved_step_count.is_some() { + criteria.push("unimproved_step_count"); + } + if configured.unimproved_time.is_some() { + criteria.push("unimproved_time"); + } + let composition = if criteria.is_empty() { + "unbounded".to_string() + } else { + criteria.join("_or_") + }; + let attributes = vec![ ("entrypoint".to_string(), "configured_runtime".to_string()), ( "configured_time_limit_ns".to_string(), - configured_time_limit.map_or_else(|| "none".to_string(), duration_nanos), + configured.time_limit.map_or_else(none, duration_nanos), ), ( "configured_entrypoint_default_time_limit_ns".to_string(), - duration_nanos(fallback_time_limit), + duration_nanos(Duration::from_secs(default_secs)), ), ( "effective_time_limit_ns".to_string(), - effective_time_limit.map_or_else(|| "none".to_string(), duration_nanos), + effective_time_limit.map_or_else(none, duration_nanos), ), ( "time_limit_source".to_string(), time_limit_source.to_string(), ), + ( + "best_score_limit".to_string(), + configured + .best_score + .map_or_else(none, |score| score.to_string()), + ), + ( + "step_count_limit".to_string(), + configured + .step_count + .map_or_else(none, |limit| limit.to_string()), + ), + ( + "unimproved_step_count_limit".to_string(), + configured + .unimproved_step_count + .map_or_else(none, |limit| limit.to_string()), + ), + ( + "unimproved_time_limit_ns".to_string(), + configured.unimproved_time.map_or_else(none, duration_nanos), + ), + ("termination_composition".to_string(), composition), ]; - - match criterion { - Some(ConfiguredTerminationCriterion::BestScore(target)) => { - attributes.push(("criterion".to_string(), "best_score".to_string())); - attributes.push(("criterion_target".to_string(), target.to_string())); - attributes.push(( - "termination_composition".to_string(), - "time_or_best_score".to_string(), - )); - } - Some(ConfiguredTerminationCriterion::StepCount(limit)) => { - attributes.push(("criterion".to_string(), "step_count".to_string())); - attributes.push(("criterion_target".to_string(), limit.to_string())); - attributes.push(( - "termination_composition".to_string(), - "time_or_step_count".to_string(), - )); - } - Some(ConfiguredTerminationCriterion::UnimprovedStepCount(limit)) => { - attributes.push(("criterion".to_string(), "unimproved_step_count".to_string())); - attributes.push(("criterion_target".to_string(), limit.to_string())); - attributes.push(( - "termination_composition".to_string(), - "time_or_unimproved_step_count".to_string(), - )); - } - Some(ConfiguredTerminationCriterion::UnimprovedTime(limit)) => { - attributes.push(("criterion".to_string(), "unimproved_time".to_string())); - attributes.push(("criterion_target_ns".to_string(), duration_nanos(limit))); - attributes.push(( - "termination_composition".to_string(), - "time_or_unimproved_time".to_string(), - )); - } - None if effective_time_limit.is_some() => { - attributes.push(("criterion".to_string(), "none".to_string())); - attributes.push(( - "termination_composition".to_string(), - "time_only".to_string(), - )); - } - None => { - attributes.push(("criterion".to_string(), "none".to_string())); - attributes.push(( - "termination_composition".to_string(), - "unbounded".to_string(), - )); - } - } - CandidateTraceExecutionPolicy::known("solverforge.execution_policy", attributes) } diff --git a/crates/solverforge-solver/src/run/weight_modes.rs b/crates/solverforge-solver/src/run/weight_modes.rs index d65eaf88..7bf8f6c0 100644 --- a/crates/solverforge-solver/src/run/weight_modes.rs +++ b/crates/solverforge-solver/src/run/weight_modes.rs @@ -19,7 +19,7 @@ type ModeDirector<'d, S, C, M> = ScoreDirector type ModeSession<'d, S, C, M, Runner> = SolveSession< 'static, Runner, - Option>>, + Option>, S, ModeDirector<'d, S, C, M>, ChannelProgressCallback, @@ -77,24 +77,6 @@ where ) } -impl> AnyTermination { - /// The same termination, with its state, checked against director `D2`. - pub(crate) fn retarget>(self) -> AnyTermination { - match self { - Self::None(t) => AnyTermination::None(t), - Self::Default(t) => AnyTermination::Default(OrTermination::new(t.0)), - Self::WithBestScore(t) => AnyTermination::WithBestScore(OrTermination::new(t.0)), - Self::WithStepCount(t) => AnyTermination::WithStepCount(OrTermination::new(t.0)), - Self::WithUnimprovedStep(t) => { - AnyTermination::WithUnimprovedStep(OrTermination::new(t.0)) - } - Self::WithUnimprovedTime(t) => { - AnyTermination::WithUnimprovedTime(OrTermination::new(t.0)) - } - } - } -} - enum Running { Unweighted(A), Weighted(B), @@ -176,7 +158,7 @@ where } Running::Weighted( session.switch_director(ScoreDirector::into_weight_mode::, |termination| { - termination.map(AnyTermination::retarget) + termination }), ) } @@ -186,7 +168,7 @@ where } Running::Unweighted(session.switch_director( ScoreDirector::into_weight_mode::, - |termination| termination.map(AnyTermination::retarget), + |termination| termination, )) } }; @@ -218,7 +200,7 @@ where Runner: Phase, ChannelProgressCallback>, { let (termination, time_limit) = - build_director_termination(self.config, self.default_time_limit_secs); + build_solver_termination::(self.config, self.default_time_limit_secs); let execution_policy = configured_execution_policy::(self.config, self.default_time_limit_secs, time_limit); let mut solver = Solver::new(self.runner) diff --git a/crates/solverforge-solver/src/run_tests.rs b/crates/solverforge-solver/src/run_tests.rs index 001b1a5c..87ec3ec1 100644 --- a/crates/solverforge-solver/src/run_tests.rs +++ b/crates/solverforge-solver/src/run_tests.rs @@ -1,8 +1,7 @@ -use super::{log_solve_start, try_run_solver_with_config_and_search, AnyTermination}; +use super::{build_solver_termination, log_solve_start, try_run_solver_with_config_and_search}; use crate::builder::{RuntimeModel, SearchContext}; use crate::manager::{SolverRuntime, SolverTerminalReason}; use crate::phase::Phase; -use crate::run::termination::build_director_termination; use crate::scope::{ProgressCallback, SolverScope}; use crate::solver::Solver; use crate::stats::{ @@ -230,44 +229,72 @@ fn zero_variable_run_builds_the_normal_graph_and_search_declaration() { assert_eq!(declarations.load(Ordering::SeqCst), 1); } +fn termination_config(termination: solverforge_config::TerminationConfig) -> SolverConfig { + SolverConfig { + termination: Some(termination), + ..SolverConfig::default() + } +} + #[test] fn build_termination_preserves_missing_time_limit_as_unlimited() { - let config = SolverConfig::default(); let (termination, time_limit) = - build_director_termination::>(&config, 180); + build_solver_termination::(&SolverConfig::default(), 180); - assert!(matches!(termination, AnyTermination::None(_))); + assert!(termination.is_unbounded()); assert_eq!(time_limit, None); } #[test] fn build_termination_treats_an_empty_config_as_unlimited() { - let config = SolverConfig { - termination: Some(solverforge_config::TerminationConfig::default()), - ..SolverConfig::default() - }; - let (termination, time_limit) = - build_director_termination::>(&config, 180); + let (termination, time_limit) = build_solver_termination::( + &termination_config(solverforge_config::TerminationConfig::default()), + 180, + ); - assert!(matches!(termination, AnyTermination::None(_))); + assert!(termination.is_unbounded()); assert_eq!(time_limit, None); } #[test] -fn build_termination_returns_fallback_time_for_best_score_limit() { - let config = SolverConfig { - termination: Some(solverforge_config::TerminationConfig { +fn work_criteria_get_the_entrypoint_fallback_time_limit() { + for termination in [ + solverforge_config::TerminationConfig { best_score_limit: Some("0".to_string()), ..Default::default() - }), - ..Default::default() - }; + }, + solverforge_config::TerminationConfig { + step_count_limit: Some(10), + ..Default::default() + }, + solverforge_config::TerminationConfig { + unimproved_step_count_limit: Some(10), + ..Default::default() + }, + solverforge_config::TerminationConfig { + unimproved_seconds_spent_limit: Some(10), + ..Default::default() + }, + ] { + let (termination, time_limit) = + build_solver_termination::(&termination_config(termination), 180); + assert!(!termination.is_unbounded()); + assert_eq!(time_limit, Some(Duration::from_secs(180))); + } +} - let (termination, time_limit) = - build_director_termination::>(&config, 180); +#[test] +fn build_termination_keeps_the_configured_time_limit() { + let (_, time_limit) = build_solver_termination::( + &termination_config(solverforge_config::TerminationConfig { + step_count_limit: Some(10), + seconds_spent_limit: Some(5), + ..Default::default() + }), + 180, + ); - assert!(matches!(termination, AnyTermination::WithBestScore(_))); - assert_eq!(time_limit, Some(Duration::from_secs(180))); + assert_eq!(time_limit, Some(Duration::from_secs(5))); } #[test] @@ -279,10 +306,7 @@ fn config_best_score_limit_stops_active_phase_loop() { }), ..Default::default() }; - let (termination, time_limit) = build_director_termination::< - TestSolution, - ScoreDirector, - >(&config, 180); + let (termination, time_limit) = build_solver_termination::(&config, 180); let descriptor = SolutionDescriptor::new("TestSolution", TypeId::of::()); let director = ScoreDirector::with_descriptor( TestSolution { @@ -311,72 +335,36 @@ fn config_best_score_limit_stops_active_phase_loop() { } #[test] -fn build_termination_returns_fallback_time_for_step_limit() { - let config = SolverConfig { - termination: Some(solverforge_config::TerminationConfig { - step_count_limit: Some(10), - ..Default::default() - }), +fn every_configured_criterion_applies() { + // The step limit (3) comes before the best-score limit (5), which used to + // take precedence and hide it. + let config = termination_config(solverforge_config::TerminationConfig { + best_score_limit: Some("5".to_string()), + step_count_limit: Some(3), ..Default::default() - }; - - let (termination, time_limit) = - build_director_termination::>(&config, 180); - - assert!(matches!(termination, AnyTermination::WithStepCount(_))); - assert_eq!(time_limit, Some(Duration::from_secs(180))); -} - -#[test] -fn build_termination_returns_fallback_time_for_unimproved_step_limit() { - let config = SolverConfig { - termination: Some(solverforge_config::TerminationConfig { - unimproved_step_count_limit: Some(10), - ..Default::default() - }), - ..Default::default() - }; - - let (termination, time_limit) = - build_director_termination::>(&config, 180); - - assert!(matches!(termination, AnyTermination::WithUnimprovedStep(_))); - assert_eq!(time_limit, Some(Duration::from_secs(180))); -} - -#[test] -fn build_termination_returns_fallback_time_for_unimproved_time_limit() { - let config = SolverConfig { - termination: Some(solverforge_config::TerminationConfig { - unimproved_seconds_spent_limit: Some(10), - ..Default::default() - }), - ..Default::default() - }; - - let (termination, time_limit) = - build_director_termination::>(&config, 180); - - assert!(matches!(termination, AnyTermination::WithUnimprovedTime(_))); - assert_eq!(time_limit, Some(Duration::from_secs(180))); -} - -#[test] -fn build_termination_explicit_time_overrides_fallback() { - let config = SolverConfig { - termination: Some(solverforge_config::TerminationConfig { - step_count_limit: Some(10), - seconds_spent_limit: Some(5), - ..Default::default() - }), - ..Default::default() - }; + }); + let (termination, _) = build_solver_termination::(&config, 180); + let descriptor = SolutionDescriptor::new("TestSolution", TypeId::of::()); + let director = ScoreDirector::with_descriptor( + TestSolution { + score: Some(SoftScore::of(0)), + }, + ScoreFromSolutionConstraints, + descriptor, + |_, _| 1, + ); - let (termination, time_limit) = - build_director_termination::>(&config, 180); + let result = Solver::new(IncrementScorePhase { max_score: 10 }) + .with_config(config) + .with_termination(termination) + .solve(director); - assert!(matches!(termination, AnyTermination::WithStepCount(_))); - assert_eq!(time_limit, Some(Duration::from_secs(5))); + assert_eq!( + result.terminal_reason, + SolverTerminalReason::TerminatedByConfig + ); + assert_eq!(result.stats.step_count(), 3); + assert_eq!(result.best_score, SoftScore::of(3)); } #[test] diff --git a/crates/solverforge-solver/src/termination/composite.rs b/crates/solverforge-solver/src/termination/composite.rs deleted file mode 100644 index b200dbe7..00000000 --- a/crates/solverforge-solver/src/termination/composite.rs +++ /dev/null @@ -1,69 +0,0 @@ -//! Composite OR termination condition. -//! -//! Uses macro-generated tuple implementations for zero-erasure architecture. - -use std::fmt::Debug; -use std::marker::PhantomData; - -use solverforge_core::domain::PlanningSolution; -use solverforge_scoring::Director; - -use super::Termination; -use crate::scope::ProgressCallback; -use crate::scope::SolverScope; - -/// Combines multiple terminations with OR logic. -/// -/// Terminates when ANY of the child terminations triggers. -#[derive(Clone)] -pub(crate) struct OrTermination(pub T, PhantomData); - -impl Debug for OrTermination { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("OrTermination").field(&self.0).finish() - } -} - -impl OrTermination { - pub(crate) fn new(terminations: T) -> Self { - Self(terminations, PhantomData) - } -} - -macro_rules! impl_composite_termination { - ($($idx:tt: $T:ident),+) => { - impl Termination for OrTermination<($($T,)+), S, D> - where - S: PlanningSolution, - D: Director, - BestCb: ProgressCallback, - $($T: Termination,)+ - { - fn is_terminated(&mut self, solver_scope: &SolverScope) -> bool { - $( - if self.0.$idx.is_terminated(solver_scope) { - return true; - } - )+ - false - } - - fn install_inphase_limits(&self, solver_scope: &mut SolverScope) { - // Propagate in-phase limits from all child terminations. - // For OR, each child independently may set a limit. - $( - self.0.$idx.install_inphase_limits(solver_scope); - )+ - } - } - }; -} - -impl_composite_termination!(0: T0); -impl_composite_termination!(0: T0, 1: T1); -impl_composite_termination!(0: T0, 1: T1, 2: T2); -impl_composite_termination!(0: T0, 1: T1, 2: T2, 3: T3); -impl_composite_termination!(0: T0, 1: T1, 2: T2, 3: T3, 4: T4); -impl_composite_termination!(0: T0, 1: T1, 2: T2, 3: T3, 4: T4, 5: T5); -impl_composite_termination!(0: T0, 1: T1, 2: T2, 3: T3, 4: T4, 5: T5, 6: T6); -impl_composite_termination!(0: T0, 1: T1, 2: T2, 3: T3, 4: T4, 5: T5, 6: T6, 7: T7); diff --git a/crates/solverforge-solver/src/termination/mod.rs b/crates/solverforge-solver/src/termination/mod.rs index 3185c1dd..35a97d9d 100644 --- a/crates/solverforge-solver/src/termination/mod.rs +++ b/crates/solverforge-solver/src/termination/mod.rs @@ -1,14 +1,12 @@ //! Termination conditions for solver phases. pub(crate) mod best_score; -pub(crate) mod composite; pub(crate) mod step_count; pub(crate) mod time; pub(crate) mod traits; pub(crate) mod unimproved; pub(crate) use best_score::BestScoreTermination; -pub(crate) use composite::OrTermination; pub(crate) use step_count::StepCountTermination; pub(crate) use time::TimeTermination; pub(crate) use traits::Termination; diff --git a/crates/solverforge-solver/src/termination/tests.rs b/crates/solverforge-solver/src/termination/tests.rs index f8e8e16a..28488911 100644 --- a/crates/solverforge-solver/src/termination/tests.rs +++ b/crates/solverforge-solver/src/termination/tests.rs @@ -123,26 +123,6 @@ fn test_unimproved_step_count_termination_with_improvement() { assert!(term.is_terminated(&scope)); // 3 steps since improvement } -#[test] -fn test_or_termination() { - let mut scope = create_scope_with_score(SoftScore::of(-10)); - - // Either: best score >= 0 OR step count >= 3 - let mut term = OrTermination::new(( - BestScoreTermination::new(SoftScore::of(0)), - StepCountTermination::new(3), - )); - - // Neither condition met - assert!(!term.is_terminated(&scope)); - - // Step count condition met - scope.increment_step_count(); - scope.increment_step_count(); - scope.increment_step_count(); - assert!(term.is_terminated(&scope)); -} - #[test] fn test_unimproved_time_termination_no_score() { let scope = create_scope(); From fb8a0e0e91dbbcbd8f762468cefef3fa99acf462 Mon Sep 17 00:00:00 2001 From: Vittorio Distefano Date: Wed, 4 Nov 2026 22:10:12 +0100 Subject: [PATCH 3/3] fix(termination)!: add no hidden time limit A solve with a score or work criterion but no time limit was silently capped at 30 seconds (60 for list models) by `__solverforge_default_time_limit_secs`. The fallback is gone: nothing is added that is not configured, so a score or work criterion without a time limit runs without one. The generated function and the `default_time_limit_secs` parameter of the run entrypoints are removed, and the execution-policy trace no longer records a fallback. Closes #174. BREAKING CHANGE: a solve with `best_score_limit`, `step_count_limit` or an unimproved limit but no `seconds_spent_limit`/`minutes_spent_limit` is no longer capped at 30 or 60 seconds. `try_run_solver_with_config_and_search`, `try_run_weighted_solver_with_config_and_search` and `try_run_dynamic_solver_with_config_parts` lose their `default_time_limit_secs` parameter. The `solverforge.execution_policy` trace drops `configured_entrypoint_default_time_limit_ns`, `effective_time_limit_ns` and `time_limit_source`. --- crates/solverforge-bridge/WIREFRAME.md | 1 - crates/solverforge-bridge/src/runner.rs | 2 - crates/solverforge-config/WIREFRAME.md | 4 +- crates/solverforge-config/src/termination.rs | 4 +- .../runtime/helpers/phase_support.rs | 12 ---- .../src/planning_solution/runtime/solve.rs | 1 - crates/solverforge-solver/public-api.txt | 2 +- .../lifecycle_solutions/pause_and_hooks.rs | 1 - crates/solverforge-solver/src/run.rs | 6 -- .../solverforge-solver/src/run/termination.rs | 57 +++---------------- .../src/run/weight_modes.rs | 12 +--- crates/solverforge-solver/src/run_tests.rs | 29 +++++----- 12 files changed, 29 insertions(+), 102 deletions(-) diff --git a/crates/solverforge-bridge/WIREFRAME.md b/crates/solverforge-bridge/WIREFRAME.md index 88f28d14..04a479eb 100644 --- a/crates/solverforge-bridge/WIREFRAME.md +++ b/crates/solverforge-bridge/WIREFRAME.md @@ -211,7 +211,6 @@ pub fn try_run_dynamic_solver_with_config_parts( entity_count_by_descriptor: fn(&S, usize) -> usize, runtime: SolverRuntime, config: SolverConfig, - default_time_limit_secs: u64, log_scale: fn(&S), qualified_candidate_trace_provenance: Option, model: RuntimeModel, diff --git a/crates/solverforge-bridge/src/runner.rs b/crates/solverforge-bridge/src/runner.rs index d8c2ab75..3556ed94 100644 --- a/crates/solverforge-bridge/src/runner.rs +++ b/crates/solverforge-bridge/src/runner.rs @@ -130,7 +130,6 @@ pub fn try_run_dynamic_solver_with_config_parts( entity_count_by_descriptor: fn(&S, usize) -> usize, runtime: SolverRuntime, config: SolverConfig, - default_time_limit_secs: u64, log_scale: fn(&S), qualified_candidate_trace_provenance: Option, model: RuntimeModel, @@ -156,7 +155,6 @@ where entity_count_by_descriptor, runtime, config, - default_time_limit_secs, log_scale, qualified_candidate_trace_provenance, move |config, descriptor| dynamic_search_declaration(config, descriptor, model), diff --git a/crates/solverforge-config/WIREFRAME.md b/crates/solverforge-config/WIREFRAME.md index 39d3d027..31585482 100644 --- a/crates/solverforge-config/WIREFRAME.md +++ b/crates/solverforge-config/WIREFRAME.md @@ -100,7 +100,9 @@ Constructor: `const fn new(max_entries: NonZeroUsize) -> Self`. Derives: `Debug, Clone, Default, Deserialize, Serialize`. Every configured criterion applies, combined with OR, at solver and phase -level alike: the solve or phase stops at whichever is reached first. +level alike: the solve or phase stops at whichever is reached first. Nothing is +added that is not configured; a score or work criterion without a time limit +runs without a time limit. | Field | Type | Note | |-------|------|------| diff --git a/crates/solverforge-config/src/termination.rs b/crates/solverforge-config/src/termination.rs index 94d2ce20..778caf2a 100644 --- a/crates/solverforge-config/src/termination.rs +++ b/crates/solverforge-config/src/termination.rs @@ -3,7 +3,9 @@ use std::time::Duration; use serde::{Deserialize, Serialize}; /// Termination configuration. Every configured criterion applies, combined -/// with OR: the solve (or phase) stops at whichever is reached first. +/// with OR: the solve (or phase) stops at whichever is reached first. Nothing +/// is added that is not configured; without a time limit there is no time +/// limit. #[derive(Debug, Clone, Default, Deserialize, Serialize)] #[serde(rename_all = "snake_case", deny_unknown_fields)] pub struct TerminationConfig { diff --git a/crates/solverforge-macros/src/planning_solution/runtime/helpers/phase_support.rs b/crates/solverforge-macros/src/planning_solution/runtime/helpers/phase_support.rs index 6ecd8f87..bfbf97c8 100644 --- a/crates/solverforge-macros/src/planning_solution/runtime/helpers/phase_support.rs +++ b/crates/solverforge-macros/src/planning_solution/runtime/helpers/phase_support.rs @@ -241,14 +241,6 @@ pub(super) fn generate_runtime_phase_support( impl #solution_name { #scalar_candidate_count_helper - fn __solverforge_default_time_limit_secs() -> u64 { - if Self::__solverforge_has_list_variable() { - 60 - } else { - 30 - } - } - #[inline] fn __solverforge_has_list_variable() -> bool { false #(|| #has_list_variable_terms)* @@ -294,10 +286,6 @@ pub(super) fn generate_runtime_phase_support( impl #solution_name { #scalar_candidate_count_helper - const fn __solverforge_default_time_limit_secs() -> u64 { - 30 - } - fn __solverforge_log_scale(solution: &Self) { let descriptor = Self::descriptor(); ::solverforge::__internal::log_solve_start( diff --git a/crates/solverforge-macros/src/planning_solution/runtime/solve.rs b/crates/solverforge-macros/src/planning_solution/runtime/solve.rs index a679b1c8..a3ad52a8 100644 --- a/crates/solverforge-macros/src/planning_solution/runtime/solve.rs +++ b/crates/solverforge-macros/src/planning_solution/runtime/solve.rs @@ -76,7 +76,6 @@ pub(super) fn generate_runtime_solve_internal( Self::entity_count, runtime, config, - Self::__solverforge_default_time_limit_secs(), Self::__solverforge_log_scale, #provenance, Self::__solverforge_search_declaration, diff --git a/crates/solverforge-solver/public-api.txt b/crates/solverforge-solver/public-api.txt index 75752378..29c1b9e0 100644 --- a/crates/solverforge-solver/public-api.txt +++ b/crates/solverforge-solver/public-api.txt @@ -665,7 +665,7 @@ fn analyze: pub fn analyze(solution: &S) -> ScoreAnalysis where S: fn bind_scalar_groups: pub fn bind_scalar_groups( groups: Vec>, scalar_slots: &[ScalarVariableSlot], ) -> Vec> fn local_search: pub fn local_search( move_selector: MS, acceptor: A, forager: Fo, ) -> LocalSearchPhase, A, Fo> where S: PlanningSolution, M: Move + 'static, MS: MoveSelector, A: Acceptor, Fo: LocalSearchForager, fn log_solve_start: pub fn log_solve_start( entity_count: usize, element_count: Option, candidate_count: Option, ) -fn try_run_solver_with_config_and_search: pub fn try_run_solver_with_config_and_search( solution: S, constraints: fn(&S) -> C, descriptor: SolutionDescriptor, entity_count_by_descriptor: fn(&S, usize) -> usize, runtime: SolverRuntime, config: SolverConfig, default_time_limit_secs: u64, log_scale: fn(&S), qualified_candidate_trace_provenance: Option, build_search: BuildSearch, ) -> RuntimeBuildResult where S: PlanningSolution + Clone + Send + Sync + 'static, S::Score: Score + Copy + Ord + ParseableScore, C: ConstraintSet, V: Clone + Copy + PartialEq + Eq + Hash + Into + Send + Sync + Debug + 'static, DM: CrossEntityDistanceMeter + Clone + Send + Sync + Debug + 'static, IDM: CrossEntityDistanceMeter + Clone + Send + Sync + Debug + 'static, Declaration: Search, Declaration::Extensions: RuntimeExtensionRegistry, BuildSearch: FnOnce(&SolverConfig, SolutionDescriptor) -> RuntimeBuildResult, +fn try_run_solver_with_config_and_search: pub fn try_run_solver_with_config_and_search( solution: S, constraints: fn(&S) -> C, descriptor: SolutionDescriptor, entity_count_by_descriptor: fn(&S, usize) -> usize, runtime: SolverRuntime, config: SolverConfig, log_scale: fn(&S), qualified_candidate_trace_provenance: Option, build_search: BuildSearch, ) -> RuntimeBuildResult where S: PlanningSolution + Clone + Send + Sync + 'static, S::Score: Score + Copy + Ord + ParseableScore, C: ConstraintSet, V: Clone + Copy + PartialEq + Eq + Hash + Into + Send + Sync + Debug + 'static, DM: CrossEntityDistanceMeter + Clone + Send + Sync + Debug + 'static, IDM: CrossEntityDistanceMeter + Clone + Send + Sync + Debug + 'static, Declaration: Search, Declaration::Extensions: RuntimeExtensionRegistry, BuildSearch: FnOnce(&SolverConfig, SolutionDescriptor) -> RuntimeBuildResult, type RepairProvider: pub type RepairProvider = fn(&S, RepairLimits) -> Vec>; type RuntimeBuildResult: pub type RuntimeBuildResult = Result; type ScalarAssignmentRule: pub type ScalarAssignmentRule = fn(&S, usize, usize, usize, usize) -> bool; diff --git a/crates/solverforge-solver/src/manager/mod_tests_integration/lifecycle_solutions/pause_and_hooks.rs b/crates/solverforge-solver/src/manager/mod_tests_integration/lifecycle_solutions/pause_and_hooks.rs index 2dd39273..079ca8b3 100644 --- a/crates/solverforge-solver/src/manager/mod_tests_integration/lifecycle_solutions/pause_and_hooks.rs +++ b/crates/solverforge-solver/src/manager/mod_tests_integration/lifecycle_solutions/pause_and_hooks.rs @@ -277,7 +277,6 @@ impl Solvable for ZeroVariableLifecycleSolution { zero_variable_entity_count, runtime, solverforge_config::SolverConfig::default(), - 30, zero_variable_log_scale, None, |config, descriptor| { diff --git a/crates/solverforge-solver/src/run.rs b/crates/solverforge-solver/src/run.rs index fc935f6a..4e886091 100644 --- a/crates/solverforge-solver/src/run.rs +++ b/crates/solverforge-solver/src/run.rs @@ -148,7 +148,6 @@ pub fn try_run_solver_with_config_and_search usize, runtime: SolverRuntime, config: SolverConfig, - default_time_limit_secs: u64, log_scale: fn(&S), qualified_candidate_trace_provenance: Option, build_search: BuildSearch, @@ -181,7 +180,6 @@ where entity_count_by_descriptor, runtime, config, - default_time_limit_secs, log_scale, qualified_candidate_trace_provenance, build_search, @@ -196,7 +194,6 @@ fn try_run_solver_with_config_and_search_request usize, runtime: SolverRuntime, config: SolverConfig, - default_time_limit_secs: u64, log_scale: fn(&S), qualified_candidate_trace_provenance: Option, build_search: BuildSearch, @@ -230,7 +227,6 @@ where entity_count_by_descriptor, runtime, config, - default_time_limit_secs, log_scale, qualified_candidate_trace_provenance, move |config, descriptor| { @@ -292,7 +288,6 @@ fn try_run_solver_with_candidate_trace_request( entity_count_by_descriptor: fn(&S, usize) -> usize, runtime: SolverRuntime, config: SolverConfig, - default_time_limit_secs: u64, log_scale: fn(&S), qualified_candidate_trace_provenance: Option, build_runner: BuildRunner, @@ -347,7 +342,6 @@ where entity_count_by_descriptor, runtime, config: &config, - default_time_limit_secs, qualified_candidate_trace_provenance, }, runner, diff --git a/crates/solverforge-solver/src/run/termination.rs b/crates/solverforge-solver/src/run/termination.rs index 41eb1a8d..8a47c5ef 100644 --- a/crates/solverforge-solver/src/run/termination.rs +++ b/crates/solverforge-solver/src/run/termination.rs @@ -138,21 +138,19 @@ where } /// Builds the solver-wide termination from config, returning it and the -/// effective time limit. A score or work criterion without a configured time -/// limit is paired with the entrypoint's `default_secs` time guard. +/// configured time limit. Nothing is added that the configuration does not +/// state: without a time limit there is no time guard. pub(crate) fn build_solver_termination( config: &SolverConfig, - default_secs: u64, ) -> (ConfiguredSolverTermination, Option) where S: PlanningSolution, S::Score: Score + ParseableScore, { let configured = parse_configured_termination::(config.termination.as_ref()); - let time_limit = effective_time_limit(&configured, default_secs); ( ConfiguredSolverTermination { - time: time_limit.map(TimeTermination::new), + time: configured.time_limit.map(TimeTermination::new), best_score: configured.best_score.map(BestScoreTermination::new), step_count: configured.step_count.map(StepCountTermination::new), unimproved_step_count: configured @@ -162,50 +160,21 @@ where .unimproved_time .map(UnimprovedTimeTermination::new), }, - time_limit, + configured.time_limit, ) } -/// The configured time limit, or the entrypoint fallback when only a score or -/// work criterion is configured. -fn effective_time_limit( - configured: &ConfiguredTermination, - default_secs: u64, -) -> Option { - let has_criterion = configured.best_score.is_some() - || configured.step_count.is_some() - || configured.unimproved_step_count.is_some() - || configured.unimproved_time.is_some(); - configured - .time_limit - .or_else(|| has_criterion.then(|| Duration::from_secs(default_secs))) -} - /// Records the termination policy the configured runtime installed: the -/// effective time limit and every configured criterion, all combined with OR. -/// -/// The time guard comes from `build_solver_termination`'s returned effective -/// limit rather than from the input TOML: a score or work criterion without -/// a time limit gets the entrypoint's fallback guard, and that injected guard -/// is material to both bounded-work and fixed-budget comparisons. -pub(crate) fn configured_execution_policy( - config: &SolverConfig, - default_secs: u64, - effective_time_limit: Option, -) -> CandidateTraceExecutionPolicy +/// configured time limit and every configured criterion, all combined with OR. +pub(crate) fn configured_execution_policy(config: &SolverConfig) -> CandidateTraceExecutionPolicy where S: PlanningSolution, S::Score: ParseableScore + std::fmt::Display, { let configured = parse_configured_termination::(config.termination.as_ref()); let none = || "none".to_string(); - let time_limit_source = match (configured.time_limit, effective_time_limit) { - (Some(_), Some(_)) => "configured", - (None, Some(_)) => "configured_entrypoint_fallback", - (_, None) => "not_installed", - }; let mut criteria = Vec::new(); - if effective_time_limit.is_some() { + if configured.time_limit.is_some() { criteria.push("time"); } if configured.best_score.is_some() { @@ -231,18 +200,6 @@ where "configured_time_limit_ns".to_string(), configured.time_limit.map_or_else(none, duration_nanos), ), - ( - "configured_entrypoint_default_time_limit_ns".to_string(), - duration_nanos(Duration::from_secs(default_secs)), - ), - ( - "effective_time_limit_ns".to_string(), - effective_time_limit.map_or_else(none, duration_nanos), - ), - ( - "time_limit_source".to_string(), - time_limit_source.to_string(), - ), ( "best_score_limit".to_string(), configured diff --git a/crates/solverforge-solver/src/run/weight_modes.rs b/crates/solverforge-solver/src/run/weight_modes.rs index 7bf8f6c0..e20cf5cd 100644 --- a/crates/solverforge-solver/src/run/weight_modes.rs +++ b/crates/solverforge-solver/src/run/weight_modes.rs @@ -37,7 +37,6 @@ pub fn try_run_weighted_solver_with_config_and_search usize, runtime: SolverRuntime, config: SolverConfig, - default_time_limit_secs: u64, log_scale: fn(&S), qualified_candidate_trace_provenance: Option, build_search: BuildSearch, @@ -70,7 +69,6 @@ where entity_count_by_descriptor, runtime, config, - default_time_limit_secs, log_scale, qualified_candidate_trace_provenance, build_search, @@ -89,7 +87,6 @@ pub(super) struct ModeSolve<'d, 'c, S: PlanningSolution, C> { pub(super) entity_count_by_descriptor: fn(&S, usize) -> usize, pub(super) runtime: SolverRuntime, pub(super) config: &'c SolverConfig, - pub(super) default_time_limit_secs: u64, pub(super) qualified_candidate_trace_provenance: Option, } @@ -112,7 +109,6 @@ where entity_count_by_descriptor, runtime, config, - default_time_limit_secs, qualified_candidate_trace_provenance, } = solve; let start_weighted = W::WEIGHTED && requires_weighted_mode(&solution); @@ -126,7 +122,6 @@ where let start = SessionStart { runner, config, - default_time_limit_secs, runtime, qualified_candidate_trace_provenance, control, @@ -179,7 +174,6 @@ where struct SessionStart<'c, S: PlanningSolution, Runner> { runner: Runner, config: &'c SolverConfig, - default_time_limit_secs: u64, runtime: SolverRuntime, qualified_candidate_trace_provenance: Option, control: WeightModeControl, @@ -199,10 +193,8 @@ where M: WeightMode, Runner: Phase, ChannelProgressCallback>, { - let (termination, time_limit) = - build_solver_termination::(self.config, self.default_time_limit_secs); - let execution_policy = - configured_execution_policy::(self.config, self.default_time_limit_secs, time_limit); + let (termination, time_limit) = build_solver_termination::(self.config); + let execution_policy = configured_execution_policy::(self.config); let mut solver = Solver::new(self.runner) .with_config(self.config.clone()) .with_candidate_trace_execution_policy(execution_policy) diff --git a/crates/solverforge-solver/src/run_tests.rs b/crates/solverforge-solver/src/run_tests.rs index 87ec3ec1..ddb5b3dd 100644 --- a/crates/solverforge-solver/src/run_tests.rs +++ b/crates/solverforge-solver/src/run_tests.rs @@ -210,7 +210,6 @@ fn zero_variable_run_builds_the_normal_graph_and_search_declaration() { test_entity_count, SolverRuntime::detached(), SolverConfig::default(), - 30, noop_log_scale, None, move |config, descriptor| { @@ -239,7 +238,7 @@ fn termination_config(termination: solverforge_config::TerminationConfig) -> Sol #[test] fn build_termination_preserves_missing_time_limit_as_unlimited() { let (termination, time_limit) = - build_solver_termination::(&SolverConfig::default(), 180); + build_solver_termination::(&SolverConfig::default()); assert!(termination.is_unbounded()); assert_eq!(time_limit, None); @@ -247,17 +246,16 @@ fn build_termination_preserves_missing_time_limit_as_unlimited() { #[test] fn build_termination_treats_an_empty_config_as_unlimited() { - let (termination, time_limit) = build_solver_termination::( - &termination_config(solverforge_config::TerminationConfig::default()), - 180, - ); + let (termination, time_limit) = build_solver_termination::(&termination_config( + solverforge_config::TerminationConfig::default(), + )); assert!(termination.is_unbounded()); assert_eq!(time_limit, None); } #[test] -fn work_criteria_get_the_entrypoint_fallback_time_limit() { +fn work_criteria_install_no_time_limit_of_their_own() { for termination in [ solverforge_config::TerminationConfig { best_score_limit: Some("0".to_string()), @@ -277,22 +275,21 @@ fn work_criteria_get_the_entrypoint_fallback_time_limit() { }, ] { let (termination, time_limit) = - build_solver_termination::(&termination_config(termination), 180); + build_solver_termination::(&termination_config(termination)); assert!(!termination.is_unbounded()); - assert_eq!(time_limit, Some(Duration::from_secs(180))); + assert_eq!(time_limit, None); } } #[test] fn build_termination_keeps_the_configured_time_limit() { - let (_, time_limit) = build_solver_termination::( - &termination_config(solverforge_config::TerminationConfig { + let (_, time_limit) = build_solver_termination::(&termination_config( + solverforge_config::TerminationConfig { step_count_limit: Some(10), seconds_spent_limit: Some(5), ..Default::default() - }), - 180, - ); + }, + )); assert_eq!(time_limit, Some(Duration::from_secs(5))); } @@ -306,7 +303,7 @@ fn config_best_score_limit_stops_active_phase_loop() { }), ..Default::default() }; - let (termination, time_limit) = build_solver_termination::(&config, 180); + let (termination, time_limit) = build_solver_termination::(&config); let descriptor = SolutionDescriptor::new("TestSolution", TypeId::of::()); let director = ScoreDirector::with_descriptor( TestSolution { @@ -343,7 +340,7 @@ fn every_configured_criterion_applies() { step_count_limit: Some(3), ..Default::default() }); - let (termination, _) = build_solver_termination::(&config, 180); + let (termination, _) = build_solver_termination::(&config); let descriptor = SolutionDescriptor::new("TestSolution", TypeId::of::()); let director = ScoreDirector::with_descriptor( TestSolution {