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 0ebfcb52..31585482 100644 --- a/crates/solverforge-config/WIREFRAME.md +++ b/crates/solverforge-config/WIREFRAME.md @@ -99,6 +99,11 @@ 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. Nothing is +added that is not configured; a score or work criterion without a time limit +runs without a time limit. + | Field | Type | Note | |-------|------|------| | `seconds_spent_limit` | `Option` | Max seconds | @@ -112,7 +117,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..778caf2a 100644 --- a/crates/solverforge-config/src/termination.rs +++ b/crates/solverforge-config/src/termination.rs @@ -2,14 +2,17 @@ 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. 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 { - /// 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 +29,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)) + ); +} 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/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/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 6dd02afd..4e886091 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. @@ -202,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, @@ -235,7 +180,6 @@ where entity_count_by_descriptor, runtime, config, - default_time_limit_secs, log_scale, qualified_candidate_trace_provenance, build_search, @@ -250,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, @@ -284,7 +227,6 @@ where entity_count_by_descriptor, runtime, config, - default_time_limit_secs, log_scale, qualified_candidate_trace_provenance, move |config, descriptor| { @@ -346,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, @@ -401,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 0f1915eb..8a47c5ef 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,204 +44,186 @@ 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); - - 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 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); } - 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) = &mut self.best_score { + terminated |= Termination::::is_terminated(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) = &mut self.step_count { + terminated |= Termination::::is_terminated(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) = &mut self.unimproved_step_count { + terminated |= Termination::::is_terminated(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) = &mut self.unimproved_time { + terminated |= Termination::::is_terminated(unimproved, solver_scope); + } + terminated + } - (termination, 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); + } + if let Some(best_score) = &self.best_score { + Termination::::install_inphase_limits(best_score, solver_scope); + } + if let Some(step_count) = &self.step_count { + Termination::::install_inphase_limits(step_count, solver_scope); + } + if let Some(unimproved) = &self.unimproved_step_count { + Termination::::install_inphase_limits(unimproved, solver_scope); + } + if let Some(unimproved) = &self.unimproved_time { + Termination::::install_inphase_limits(unimproved, solver_scope); + } + } } -/// Records the termination policy the configured runtime actually installed. -/// -/// 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. -pub(crate) fn configured_execution_policy( +/// Builds the solver-wide termination from config, returning it and the +/// 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, - effective_time_limit: Option, -) -> CandidateTraceExecutionPolicy +) -> (ConfiguredSolverTermination, Option) where S: PlanningSolution, - S::Score: ParseableScore + std::fmt::Display, + S::Score: Score + ParseableScore, { 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); + ( + ConfiguredSolverTermination { + 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 + .unimproved_step_count + .map(UnimprovedStepCountTermination::new), + unimproved_time: configured + .unimproved_time + .map(UnimprovedTimeTermination::new), + }, + configured.time_limit, + ) +} - 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) => "not_installed", +/// Records the termination policy the configured runtime installed: the +/// 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 mut criteria = Vec::new(); + if configured.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 mut attributes = vec![ + 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), + ), + ( + "best_score_limit".to_string(), + configured + .best_score + .map_or_else(none, |score| score.to_string()), ), ( - "configured_entrypoint_default_time_limit_ns".to_string(), - duration_nanos(fallback_time_limit), + "step_count_limit".to_string(), + configured + .step_count + .map_or_else(none, |limit| limit.to_string()), ), ( - "effective_time_limit_ns".to_string(), - effective_time_limit.map_or_else(|| "none".to_string(), duration_nanos), + "unimproved_step_count_limit".to_string(), + configured + .unimproved_step_count + .map_or_else(none, |limit| limit.to_string()), ), ( - "time_limit_source".to_string(), - time_limit_source.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..e20cf5cd 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, @@ -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,31 +69,12 @@ where entity_count_by_descriptor, runtime, config, - default_time_limit_secs, log_scale, qualified_candidate_trace_provenance, build_search, ) } -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), @@ -107,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, } @@ -130,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); @@ -144,7 +122,6 @@ where let start = SessionStart { runner, config, - default_time_limit_secs, runtime, qualified_candidate_trace_provenance, control, @@ -176,7 +153,7 @@ where } Running::Weighted( session.switch_director(ScoreDirector::into_weight_mode::, |termination| { - termination.map(AnyTermination::retarget) + termination }), ) } @@ -186,7 +163,7 @@ where } Running::Unweighted(session.switch_director( ScoreDirector::into_weight_mode::, - |termination| termination.map(AnyTermination::retarget), + |termination| termination, )) } }; @@ -197,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, @@ -217,10 +193,8 @@ where M: WeightMode, Runner: Phase, ChannelProgressCallback>, { - let (termination, time_limit) = - build_director_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 001b1a5c..ddb5b3dd 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::{ @@ -211,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| { @@ -230,44 +228,70 @@ 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()); - 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(), + )); - 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_install_no_time_limit_of_their_own() { + 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)); + assert!(!termination.is_unbounded()); + assert_eq!(time_limit, None); + } +} - 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() + }, + )); - 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 +303,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); let descriptor = SolutionDescriptor::new("TestSolution", TypeId::of::()); let director = ScoreDirector::with_descriptor( TestSolution { @@ -311,72 +332,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); + 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();