diff --git a/crates/solverforge-config/WIREFRAME.md b/crates/solverforge-config/WIREFRAME.md index 361e1269..32ce35f3 100644 --- a/crates/solverforge-config/WIREFRAME.md +++ b/crates/solverforge-config/WIREFRAME.md @@ -633,7 +633,8 @@ Enum: `Sequential`, `RoundRobin`, `RotatingRoundRobin`, `Random`, and ### `UnionWeighting` Enum: `Equal` (default), `Fixed`, and `CandidateCount`. `Fixed` consumes the -explicit `weights` vector; `CandidateCount` derives scheduling weight from the +explicit `weights` vector; a zero weight disables its child (never opened, not +counted in the union size), and at least one weight must be positive; `CandidateCount` derives scheduling weight from the children's declared candidate counts. ### `CartesianProductConfig` diff --git a/crates/solverforge-config/src/move_selector/composite.rs b/crates/solverforge-config/src/move_selector/composite.rs index c1fba9ad..1dc4fb91 100644 --- a/crates/solverforge-config/src/move_selector/composite.rs +++ b/crates/solverforge-config/src/move_selector/composite.rs @@ -54,8 +54,9 @@ pub struct UnionMoveSelectorConfig { #[serde(default)] pub weighting: UnionWeighting, /// One weight per child for `weighting = "fixed"`; a zero weight disables - /// that child. Weights other than `1` need `random` or `stratified_random` - /// order. Default: empty. + /// that child, which is then never opened nor counted in the union's + /// size. At least one weight must be positive. Weights other than `1` + /// need `random` or `stratified_random` order. Default: empty. #[serde(default)] pub weights: Vec, /// Child selectors. diff --git a/crates/solverforge-solver/src/builder/selector/types/composite/state.rs b/crates/solverforge-solver/src/builder/selector/types/composite/state.rs index abb36c9e..ae395e84 100644 --- a/crates/solverforge-solver/src/builder/selector/types/composite/state.rs +++ b/crates/solverforge-solver/src/builder/selector/types/composite/state.rs @@ -172,8 +172,11 @@ where context, resolve_union_weights(*weighting, weights, &sizes), ); - for &child in children { - self.open_node(child, resources, director, child_context); + // A disabled child is never scheduled, so it is never opened. + for (index, &child) in children.iter().enumerate() { + if union_child_enabled(*weighting, weights, index) { + self.open_node(child, resources, director, child_context); + } } CursorNode::Union(union) } @@ -226,9 +229,16 @@ where } => self .selector_size(*selector, director) .min(*selected_count_limit), - SelectorCompositionNode::Union { children, .. } => children + SelectorCompositionNode::Union { + weighting, + weights, + children, + .. + } => children .iter() - .map(|&child| self.selector_size(child, director)) + .enumerate() + .filter(|&(index, _)| union_child_enabled(*weighting, weights, index)) + .map(|(_, &child)| self.selector_size(child, director)) .sum(), SelectorCompositionNode::Cartesian { left, right, .. } => self .selector_size(*left, director) @@ -406,3 +416,13 @@ where self.selector_index_at(self.selector.root(), id) } } + +/// Whether a union child takes part: every child does, except one a fixed +/// weighting gives weight `0`, which is disabled. +fn union_child_enabled( + weighting: solverforge_config::UnionWeighting, + weights: &[u64], + index: usize, +) -> bool { + !(weighting == solverforge_config::UnionWeighting::Fixed && weights[index] == 0) +} diff --git a/crates/solverforge-solver/src/builder/selector/types/composite/tests.rs b/crates/solverforge-solver/src/builder/selector/types/composite/tests.rs index 3834a7cf..6e076792 100644 --- a/crates/solverforge-solver/src/builder/selector/types/composite/tests.rs +++ b/crates/solverforge-solver/src/builder/selector/types/composite/tests.rs @@ -483,3 +483,6 @@ fn arena_rejects_shared_mutable_child_nodes() { }); let _ = SelectorCompositionState::<_, _, _, _, ()>::new(graph); } + +#[path = "tests/union_weights.rs"] +mod union_weights; diff --git a/crates/solverforge-solver/src/builder/selector/types/composite/tests/union_weights.rs b/crates/solverforge-solver/src/builder/selector/types/composite/tests/union_weights.rs new file mode 100644 index 00000000..7bdfc9c5 --- /dev/null +++ b/crates/solverforge-solver/src/builder/selector/types/composite/tests/union_weights.rs @@ -0,0 +1,93 @@ +// A zero fixed union weight disables its child. + +use super::*; + +fn leaf(graph: &mut Graph, entity: usize, len: usize) -> CompositionNodeId { + graph.push(SelectorCompositionNode::Flat(CustomFlat { + entity, + values: (1..=len).map(|value| value as i64).collect(), + })) +} + +fn observe_graph(graph: &Graph, context: MoveStreamContext) -> Vec<(usize, i64)> { + let mut director = create_nqueens_director(&[0, 0, 0]); + let state = new_stream_state(graph); + let mut cursor = open_cursor_with_owned_stream_state(graph, state, &mut (), &director, context); + observe(&mut cursor, &mut director) + .into_iter() + .map(|(_, entity, row)| (entity, row)) + .collect() +} + +#[test] +fn a_disabled_child_produces_no_candidates() { + for seed in [0, 1, 17, 999] { + let context = MoveStreamContext::new(7, seed, None); + let mut graph = Graph::new(); + let children = vec![ + leaf(&mut graph, 0, 2), + leaf(&mut graph, 1, 3), + leaf(&mut graph, 2, 4), + ]; + graph.push(SelectorCompositionNode::Union { + selection_order: UnionSelectionOrder::StratifiedRandom, + weighting: UnionWeighting::Fixed, + weights: vec![2, 0, 1], + children, + }); + let seen = observe_graph(&graph, context); + assert_eq!(seen.len(), 2 + 4, "{seen:?}"); + assert!(seen.iter().all(|(entity, _)| *entity != 1), "{seen:?}"); + } +} + +#[test] +fn a_disabled_child_is_not_counted_in_the_union_size() { + // The outer union weighs its children by candidate count. Its inner + // union disables a three-candidate child, so it must weigh like a union + // of the two-candidate child alone. + for seed in [0, 1, 17, 999, 4242] { + let context = MoveStreamContext::new(3, seed, None); + + let mut with_disabled = Graph::new(); + let inner_children = vec![ + leaf(&mut with_disabled, 0, 2), + leaf(&mut with_disabled, 1, 3), + ]; + let inner = with_disabled.push(SelectorCompositionNode::Union { + selection_order: UnionSelectionOrder::StratifiedRandom, + weighting: UnionWeighting::Fixed, + weights: vec![1, 0], + children: inner_children, + }); + let other = leaf(&mut with_disabled, 2, 8); + with_disabled.push(SelectorCompositionNode::Union { + selection_order: UnionSelectionOrder::StratifiedRandom, + weighting: UnionWeighting::CandidateCount, + weights: vec![], + children: vec![inner, other], + }); + + let mut without = Graph::new(); + let only = leaf(&mut without, 0, 2); + let inner = without.push(SelectorCompositionNode::Union { + selection_order: UnionSelectionOrder::StratifiedRandom, + weighting: UnionWeighting::Fixed, + weights: vec![1], + children: vec![only], + }); + let other = leaf(&mut without, 2, 8); + without.push(SelectorCompositionNode::Union { + selection_order: UnionSelectionOrder::StratifiedRandom, + weighting: UnionWeighting::CandidateCount, + weights: vec![], + children: vec![inner, other], + }); + + assert_eq!( + observe_graph(&with_disabled, context), + observe_graph(&without, context), + "seed {seed}" + ); + } +} diff --git a/crates/solverforge-solver/src/heuristic/selector/decorator/vec_union.rs b/crates/solverforge-solver/src/heuristic/selector/decorator/vec_union.rs index cafd4200..aa225df2 100644 --- a/crates/solverforge-solver/src/heuristic/selector/decorator/vec_union.rs +++ b/crates/solverforge-solver/src/heuristic/selector/decorator/vec_union.rs @@ -44,8 +44,8 @@ pub(crate) fn resolve_union_weights( "fixed union weight count must match child count" ); assert!( - fixed.iter().all(|weight| *weight > 0), - "fixed union weights must all be positive" + fixed.iter().any(|weight| *weight > 0), + "fixed union weights need at least one positive weight" ); fixed.to_vec() } diff --git a/crates/solverforge-solver/src/runtime/compiler/selector_tree.rs b/crates/solverforge-solver/src/runtime/compiler/selector_tree.rs index 64f892fa..d0a1b5d7 100644 --- a/crates/solverforge-solver/src/runtime/compiler/selector_tree.rs +++ b/crates/solverforge-solver/src/runtime/compiler/selector_tree.rs @@ -286,8 +286,10 @@ where { Some("fixed union weight count must match selector count") } - solverforge_config::UnionWeighting::Fixed if union.weights.contains(&0) => { - Some("fixed union weights must all be positive") + solverforge_config::UnionWeighting::Fixed + if union.weights.iter().all(|weight| *weight == 0) => + { + Some("fixed union weights need at least one positive weight; zero disables a child") } solverforge_config::UnionWeighting::CandidateCount if !union.weights.is_empty() => { Some("candidate_count union weighting does not accept explicit weights") diff --git a/crates/solverforge-solver/src/runtime/compiler/tests/invalid_config.rs b/crates/solverforge-solver/src/runtime/compiler/tests/invalid_config.rs index 205ec92e..a6efe39c 100644 --- a/crates/solverforge-solver/src/runtime/compiler/tests/invalid_config.rs +++ b/crates/solverforge-solver/src/runtime/compiler/tests/invalid_config.rs @@ -142,3 +142,32 @@ fn unparsable_best_score_limits_fail_compilation_with_the_key_path() { }; validate_best_score_limits::(&valid).expect("valid limits compile"); } + +fn fixed_union(weights: Vec) -> MoveSelectorConfig { + use solverforge_config::{UnionMoveSelectorConfig, UnionSelectionOrder, UnionWeighting}; + MoveSelectorConfig::UnionMoveSelector(UnionMoveSelectorConfig { + selection_order: UnionSelectionOrder::StratifiedRandom, + weighting: UnionWeighting::Fixed, + weights, + selectors: vec![ + MoveSelectorConfig::ListChangeMoveSelector(ListChangeMoveConfig::default()), + MoveSelectorConfig::ListRuinMoveSelector(ListRuinMoveSelectorConfig::default()), + ], + }) +} + +#[test] +fn a_zero_fixed_union_weight_disables_its_child() { + compile(PrecedenceShape::Absent, fixed_union(vec![1, 0])) + .expect("a zero weight disables the child instead of failing"); +} + +#[test] +fn fixed_union_weights_that_are_all_zero_are_rejected() { + let error = compile(PrecedenceShape::Absent, fixed_union(vec![0, 0])) + .expect_err("a union without a live child is rejected"); + assert!( + error.to_string().contains("at least one positive weight"), + "{error}" + ); +}