diff --git a/crates/solverforge-config/src/acceptor.rs b/crates/solverforge-config/src/acceptor.rs index cd90871b..485a44a9 100644 --- a/crates/solverforge-config/src/acceptor.rs +++ b/crates/solverforge-config/src/acceptor.rs @@ -31,7 +31,7 @@ pub enum AcceptorConfig { #[serde(rename_all = "snake_case", deny_unknown_fields)] pub struct StepCountingHillClimbingConfig { /// Steps a non-improving move is accepted for after the last improvement. - /// Omitted: `100`. + /// Omitted: `100`. Explicit `0` is invalid. pub step_count_limit: Option, } @@ -76,7 +76,8 @@ pub struct SimulatedAnnealingConfig { /// Hard-regression policy. Omitted: `temperature_controlled`. pub hard_regression_policy: Option, - /// Sampled auto-calibration settings used when level_temperatures is omitted. + /// Sampled auto-calibration settings used when `level_temperatures` is + /// omitted. Setting both is invalid. pub calibration: Option, } @@ -129,6 +130,6 @@ pub struct DiversifiedLateAcceptanceConfig { #[serde(rename_all = "snake_case", deny_unknown_fields)] pub struct GreatDelugeConfig { /// Fraction of the initial score's magnitude the water level rises per - /// step. Omitted: `0.001`. + /// step. Omitted: `0.001`. Must be finite and greater than 0. pub water_level_increase_ratio: Option, } diff --git a/crates/solverforge-solver/src/builder/acceptor/tests.rs b/crates/solverforge-solver/src/builder/acceptor/tests.rs index 51691fe0..5527b853 100644 --- a/crates/solverforge-solver/src/builder/acceptor/tests.rs +++ b/crates/solverforge-solver/src/builder/acceptor/tests.rs @@ -2,9 +2,9 @@ use super::*; use crate::builder::local_search_policy::CompiledAcceptor; use crate::builder::local_search_policy::ConfigValueError; use solverforge_config::{ - AcceptorConfig, DiversifiedLateAcceptanceConfig, HardRegressionPolicyConfig, + AcceptorConfig, DiversifiedLateAcceptanceConfig, GreatDelugeConfig, HardRegressionPolicyConfig, LateAcceptanceConfig, SimulatedAnnealingCalibrationConfig, SimulatedAnnealingConfig, - TabuSearchConfig, + StepCountingHillClimbingConfig, TabuSearchConfig, }; use solverforge_core::score::{HardSoftScore, SoftScore}; use std::any::Any; @@ -297,3 +297,47 @@ fn test_acceptor_builder_late_acceptance_rejects_zero_size() { ); } } + +#[test] +fn step_counting_hill_climbing_rejects_a_zero_step_count_limit() { + let config = AcceptorConfig::StepCountingHillClimbing(StepCountingHillClimbingConfig { + step_count_limit: Some(0), + }); + let error = build_acceptor::(&config).expect_err("zero is rejected"); + assert_eq!(error.field(), "step_count_limit"); + assert_eq!(error.message(), "must be greater than 0"); + + let config = AcceptorConfig::StepCountingHillClimbing(StepCountingHillClimbingConfig { + step_count_limit: Some(1), + }); + assert!(build_acceptor::(&config).is_ok()); +} + +#[test] +fn great_deluge_rejects_a_ratio_that_does_not_rise() { + for ratio in [0.0, -0.1, f64::NAN, f64::INFINITY] { + let config = AcceptorConfig::GreatDeluge(GreatDelugeConfig { + water_level_increase_ratio: Some(ratio), + }); + let error = build_acceptor::(&config).expect_err("invalid ratio"); + assert_eq!(error.field(), "water_level_increase_ratio"); + assert!(error + .message() + .starts_with("must be finite and greater than 0")); + } + let config = AcceptorConfig::GreatDeluge(GreatDelugeConfig { + water_level_increase_ratio: Some(0.01), + }); + assert!(build_acceptor::(&config).is_ok()); +} + +#[test] +fn simulated_annealing_rejects_calibration_beside_level_temperatures() { + let config = AcceptorConfig::SimulatedAnnealing(SimulatedAnnealingConfig { + level_temperatures: Some(vec![1.0]), + calibration: Some(SimulatedAnnealingCalibrationConfig::default()), + ..Default::default() + }); + let error = build_acceptor::(&config).expect_err("exclusive keys"); + assert_eq!(error.field(), "calibration"); +} diff --git a/crates/solverforge-solver/src/builder/local_search_policy.rs b/crates/solverforge-solver/src/builder/local_search_policy.rs index ecb94a0f..4ff07693 100644 --- a/crates/solverforge-solver/src/builder/local_search_policy.rs +++ b/crates/solverforge-solver/src/builder/local_search_policy.rs @@ -128,7 +128,16 @@ impl CompiledForager { fn validate_acceptor(config: &AcceptorConfig, level_count: usize) -> Result<(), ConfigValueError> { match config { - AcceptorConfig::HillClimbing {} | AcceptorConfig::StepCountingHillClimbing(_) => Ok(()), + AcceptorConfig::HillClimbing {} => Ok(()), + AcceptorConfig::StepCountingHillClimbing(counting) => { + if counting.step_count_limit == Some(0) { + return Err(ConfigValueError::new( + "step_count_limit", + "must be greater than 0", + )); + } + Ok(()) + } AcceptorConfig::TabuSearch(tabu) => validate_tabu_search(tabu), AcceptorConfig::SimulatedAnnealing(annealing) => { validate_simulated_annealing(annealing, level_count) @@ -139,7 +148,13 @@ fn validate_acceptor(config: &AcceptorConfig, level_count: usize) -> Result<(), AcceptorConfig::DiversifiedLateAcceptance(late) => { positive("late_acceptance_size", late.late_acceptance_size) } - AcceptorConfig::GreatDeluge(_) => Ok(()), + AcceptorConfig::GreatDeluge(deluge) => match deluge.water_level_increase_ratio { + Some(ratio) if !(ratio.is_finite() && ratio > 0.0) => Err(ConfigValueError::new( + "water_level_increase_ratio", + format!("must be finite and greater than 0, found {ratio}"), + )), + _ => Ok(()), + }, } } @@ -180,6 +195,12 @@ fn validate_simulated_annealing( for &temperature in temperatures { non_negative("level_temperatures", Some(temperature))?; } + if config.calibration.is_some() { + return Err(ConfigValueError::new( + "calibration", + "cannot be combined with `level_temperatures`, which set the temperatures calibration would measure", + )); + } Ok(()) } None => validate_calibration(config.calibration.as_ref()),