From 37d8185bb077b96ed0b0611465b778f80fca5292 Mon Sep 17 00:00:00 2001 From: Xiwei Pan Date: Fri, 18 Sep 2026 01:03:54 +0800 Subject: [PATCH 1/6] Share flow model validation across input boundaries --- src/models/graph/integral_flow_bundles.rs | 175 +++++++++--------- .../graph/integral_flow_homologous_arcs.rs | 133 ++++++++----- .../graph/integral_flow_with_multipliers.rs | 140 +++++++------- .../models/graph/integral_flow_bundles.rs | 48 +++++ .../graph/integral_flow_homologous_arcs.rs | 49 +++++ .../graph/integral_flow_with_multipliers.rs | 43 +++++ 6 files changed, 384 insertions(+), 204 deletions(-) diff --git a/src/models/graph/integral_flow_bundles.rs b/src/models/graph/integral_flow_bundles.rs index a9f99f931..61336c0a6 100644 --- a/src/models/graph/integral_flow_bundles.rs +++ b/src/models/graph/integral_flow_bundles.rs @@ -24,6 +24,7 @@ inventory::submit! { /// Integral Flow with Bundles (Garey & Johnson ND36). #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "IntegralFlowBundlesData")] pub struct IntegralFlowBundles { graph: DirectedGraph, source: usize, @@ -33,6 +34,30 @@ pub struct IntegralFlowBundles { requirement: i64, } +#[derive(Deserialize)] +struct IntegralFlowBundlesData { + graph: DirectedGraph, + source: usize, + sink: usize, + bundles: Vec>, + bundle_capacities: Vec, + requirement: i64, +} + +impl TryFrom for IntegralFlowBundles { + type Error = crate::registry::ConstructionError; + fn try_from(data: IntegralFlowBundlesData) -> Result { + Self::try_new( + data.graph, + data.source, + data.sink, + data.bundles, + data.bundle_capacities, + data.requirement, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct IntegralFlowBundlesCreateSpec { #[create(codec = "arc-list")] @@ -67,57 +92,14 @@ impl TryFrom for IntegralFlowBundles { if count < inferred { return Err("num_vertices is too small".into()); } - if spec.source >= count || spec.sink >= count { - return Err("source and sink must be valid vertices".into()); - } - if spec.source == spec.sink { - return Err("source and sink must be distinct".into()); - } - if spec.bundles.len() != spec.bundle_capacities.len() { - return Err("bundles length must match bundle_capacities length".into()); - } - if spec.requirement == 0 { - return Err("requirement must be positive".into()); - } - let mut covered = vec![false; spec.arcs.len()]; - let mut upper = vec![i64::MAX; spec.arcs.len()]; - for (i, (bundle, &capacity)) in spec.bundles.iter().zip(&spec.bundle_capacities).enumerate() - { - if capacity == 0 { - return Err(format!("bundle capacity {i} must be positive").into()); - } - let mut seen = BTreeSet::new(); - for &arc in bundle { - if arc >= spec.arcs.len() { - return Err(format!("bundle {i} arc is out of range").into()); - } - if !seen.insert(arc) { - return Err(format!("bundle {i} contains duplicate arc").into()); - } - covered[arc] = true; - upper[arc] = upper[arc].min(capacity); - } - } - for (arc, &is_covered) in covered.iter().enumerate() { - if !is_covered { - return Err(format!("arc {arc} must belong to a bundle").into()); - } - if usize::try_from(upper[arc]) - .ok() - .and_then(|v| v.checked_add(1)) - .is_none() - { - return Err(format!("arc {arc} upper bound is too large").into()); - } - } - Ok(Self { - graph: DirectedGraph::new(count, spec.arcs), - source: spec.source, - sink: spec.sink, - bundles: spec.bundles, - bundle_capacities: spec.bundle_capacities, - requirement: spec.requirement, - }) + Self::try_new( + DirectedGraph::new(count, spec.arcs), + spec.source, + spec.sink, + spec.bundles, + spec.bundle_capacities, + spec.requirement, + ) } } @@ -131,24 +113,36 @@ impl IntegralFlowBundles { bundle_capacities: Vec, requirement: i64, ) -> Self { + Self::try_new(graph, source, sink, bundles, bundle_capacities, requirement) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + graph: DirectedGraph, + source: usize, + sink: usize, + bundles: Vec>, + bundle_capacities: Vec, + requirement: i64, + ) -> Result { let num_vertices = graph.num_vertices(); let num_arcs = graph.num_arcs(); - assert!( - source < num_vertices, - "source ({source}) >= num_vertices ({num_vertices})" - ); - assert!( - sink < num_vertices, - "sink ({sink}) >= num_vertices ({num_vertices})" - ); - assert!(source != sink, "source and sink must be distinct"); - assert_eq!( - bundles.len(), - bundle_capacities.len(), - "bundles length must match bundle_capacities length" - ); - assert!(requirement > 0, "requirement must be positive"); + if !(source < num_vertices) { + return Err(format!("source ({source}) >= num_vertices ({num_vertices})").into()); + } + if !(sink < num_vertices) { + return Err(format!("sink ({sink}) >= num_vertices ({num_vertices})").into()); + } + if source == sink { + return Err("source and sink must be distinct".into()); + } + if bundles.len() != bundle_capacities.len() { + return Err("bundles length must match bundle_capacities length".into()); + } + if requirement <= 0 { + return Err("requirement must be positive".into()); + } let mut arc_covered = vec![false; num_arcs]; let mut arc_upper_bounds = vec![i64::MAX; num_arcs]; @@ -156,48 +150,49 @@ impl IntegralFlowBundles { for (bundle_index, (bundle, &capacity)) in bundles.iter().zip(&bundle_capacities).enumerate() { - assert!( - capacity > 0, - "bundle capacity at index {bundle_index} must be positive" - ); + if !(capacity > 0) { + return Err( + format!("bundle capacity at index {bundle_index} must be positive").into(), + ); + } let mut seen = BTreeSet::new(); for &arc_index in bundle { - assert!( - arc_index < num_arcs, - "bundle {bundle_index} references arc {arc_index}, but num_arcs is {num_arcs}" - ); - assert!( - seen.insert(arc_index), - "bundle {bundle_index} contains duplicate arc index {arc_index}" - ); + if !(arc_index < num_arcs) { + return Err(format!("bundle {bundle_index} arc is out of range: index {arc_index}, num_arcs {num_arcs}").into()); + } + if !(seen.insert(arc_index)) { + return Err(format!( + "bundle {bundle_index} contains duplicate arc index {arc_index}" + ) + .into()); + } arc_covered[arc_index] = true; arc_upper_bounds[arc_index] = arc_upper_bounds[arc_index].min(capacity); } } for (arc_index, covered) in arc_covered.iter().copied().enumerate() { - assert!( - covered, - "arc {arc_index} must belong to at least one bundle" - ); - let domain = usize::try_from(arc_upper_bounds[arc_index]) + if !(covered) { + return Err(format!("arc {arc_index} must belong to at least one bundle").into()); + } + if usize::try_from(arc_upper_bounds[arc_index]) .ok() - .and_then(|bound| bound.checked_add(1)); - assert!( - domain.is_some(), - "bundle-derived upper bound for arc {arc_index} must fit into usize for dims()" - ); + .and_then(|bound| bound.checked_add(1)) + .is_none() + { + return Err(format!("bundle-derived upper bound for arc {arc_index} must fit into usize for dimensions()").into()); + } } - Self { + Ok(Self { graph, source, sink, bundles, bundle_capacities, requirement, - } + }) } /// Get the underlying directed graph. diff --git a/src/models/graph/integral_flow_homologous_arcs.rs b/src/models/graph/integral_flow_homologous_arcs.rs index 0ea45a9e2..d5ec2ba1a 100644 --- a/src/models/graph/integral_flow_homologous_arcs.rs +++ b/src/models/graph/integral_flow_homologous_arcs.rs @@ -29,6 +29,7 @@ inventory::submit! { /// capacities, flow conservation at non-terminal vertices, every homologous-pair /// equality constraint, and the required net inflow at the sink. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "IntegralFlowHomologousArcsData")] pub struct IntegralFlowHomologousArcs { graph: DirectedGraph, capacities: Vec, @@ -38,6 +39,30 @@ pub struct IntegralFlowHomologousArcs { homologous_pairs: Vec<(usize, usize)>, } +#[derive(Deserialize)] +struct IntegralFlowHomologousArcsData { + graph: DirectedGraph, + capacities: Vec, + source: usize, + sink: usize, + requirement: i64, + homologous_pairs: Vec<(usize, usize)>, +} + +impl TryFrom for IntegralFlowHomologousArcs { + type Error = crate::registry::ConstructionError; + fn try_from(data: IntegralFlowHomologousArcsData) -> Result { + Self::try_new( + data.graph, + data.capacities, + data.source, + data.sink, + data.requirement, + data.homologous_pairs, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct IntegralFlowHomologousArcsCreateSpec { #[create(codec = "arc-list")] @@ -73,34 +98,14 @@ impl TryFrom for IntegralFlowHomologousArc return Err("num_vertices is too small".into()); } let capacities = spec.capacities.unwrap_or_else(|| vec![1; spec.arcs.len()]); - if capacities.len() != spec.arcs.len() { - return Err("capacities length must match arcs length".into()); - } - if spec.source >= count || spec.sink >= count { - return Err("source and sink must be valid vertices".into()); - } - for &(a, b) in &spec.homologous_pairs { - if a >= spec.arcs.len() || b >= spec.arcs.len() { - return Err("homologous pair arc index is out of range".into()); - } - } - for &c in &capacities { - if usize::try_from(c) - .ok() - .and_then(|v| v.checked_add(1)) - .is_none() - { - return Err("capacity is too large".into()); - } - } - Ok(Self { - graph: DirectedGraph::new(count, spec.arcs), + Self::try_new( + DirectedGraph::new(count, spec.arcs), capacities, - source: spec.source, - sink: spec.sink, - requirement: spec.requirement, - homologous_pairs: spec.homologous_pairs, - }) + spec.source, + spec.sink, + spec.requirement, + spec.homologous_pairs, + ) } } @@ -113,46 +118,72 @@ impl IntegralFlowHomologousArcs { requirement: i64, homologous_pairs: Vec<(usize, usize)>, ) -> Self { + Self::try_new( + graph, + capacities, + source, + sink, + requirement, + homologous_pairs, + ) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + graph: DirectedGraph, + capacities: Vec, + source: usize, + sink: usize, + requirement: i64, + homologous_pairs: Vec<(usize, usize)>, + ) -> Result { let num_vertices = graph.num_vertices(); let num_arcs = graph.num_arcs(); - assert_eq!( - capacities.len(), - num_arcs, - "capacities length must match graph.num_arcs()" - ); - assert!( - source < num_vertices, - "source ({source}) must be less than num_vertices ({num_vertices})" - ); - assert!( - sink < num_vertices, - "sink ({sink}) must be less than num_vertices ({num_vertices})" - ); + if capacities.len() != num_arcs { + return Err("capacities length must match graph.num_arcs()".into()); + } + if !(source < num_vertices) { + return Err(format!( + "source ({source}) must be less than num_vertices ({num_vertices})" + ) + .into()); + } + if !(sink < num_vertices) { + return Err( + format!("sink ({sink}) must be less than num_vertices ({num_vertices})").into(), + ); + } for &(a, b) in &homologous_pairs { - assert!(a < num_arcs, "homologous arc index {a} out of range"); - assert!(b < num_arcs, "homologous arc index {b} out of range"); + if !(a < num_arcs) { + return Err(format!("homologous arc index {a} out of range").into()); + } + if !(b < num_arcs) { + return Err(format!("homologous arc index {b} out of range").into()); + } } for &capacity in &capacities { - assert!( - usize::try_from(capacity) - .ok() - .and_then(|value| value.checked_add(1)) - .is_some(), - "capacities must fit into usize for dims()" - ); + if usize::try_from(capacity) + .ok() + .and_then(|value| value.checked_add(1)) + .is_none() + { + return Err( + "capacities must be nonnegative and their domains must fit into usize".into(), + ); + } } - Self { + Ok(Self { graph, capacities, source, sink, requirement, homologous_pairs, - } + }) } pub fn graph(&self) -> &DirectedGraph { diff --git a/src/models/graph/integral_flow_with_multipliers.rs b/src/models/graph/integral_flow_with_multipliers.rs index d5730c110..6fd7196fb 100644 --- a/src/models/graph/integral_flow_with_multipliers.rs +++ b/src/models/graph/integral_flow_with_multipliers.rs @@ -23,6 +23,7 @@ inventory::submit! { } #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "IntegralFlowWithMultipliersData")] pub struct IntegralFlowWithMultipliers { graph: DirectedGraph, source: usize, @@ -32,6 +33,30 @@ pub struct IntegralFlowWithMultipliers { requirement: i64, } +#[derive(Deserialize)] +struct IntegralFlowWithMultipliersData { + graph: DirectedGraph, + source: usize, + sink: usize, + multipliers: Vec, + capacities: Vec, + requirement: i64, +} + +impl TryFrom for IntegralFlowWithMultipliers { + type Error = crate::registry::ConstructionError; + fn try_from(data: IntegralFlowWithMultipliersData) -> Result { + Self::try_new( + data.graph, + data.source, + data.sink, + data.multipliers, + data.capacities, + data.requirement, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct IntegralFlowWithMultipliersCreateSpec { #[create(codec = "arc-list")] @@ -66,40 +91,14 @@ impl TryFrom for IntegralFlowWithMultipli if count < inferred { return Err("num_vertices is too small".into()); } - if spec.capacities.len() != spec.arcs.len() { - return Err("capacities length must match arcs length".into()); - } - if spec.multipliers.len() != count { - return Err("multipliers length must match num_vertices".into()); - } - if spec.source >= count || spec.sink >= count { - return Err("source and sink must be valid vertices".into()); - } - if spec.source == spec.sink { - return Err("source and sink must be distinct".into()); - } - for (v, &m) in spec.multipliers.iter().enumerate() { - if v != spec.source && v != spec.sink && m == 0 { - return Err("non-terminal multipliers must be positive".into()); - } - } - for &c in &spec.capacities { - if usize::try_from(c) - .ok() - .and_then(|v| v.checked_add(1)) - .is_none() - { - return Err("capacity is too large".into()); - } - } - Ok(Self { - graph: DirectedGraph::new(count, spec.arcs), - source: spec.source, - sink: spec.sink, - multipliers: spec.multipliers, - capacities: spec.capacities, - requirement: spec.requirement, - }) + Self::try_new( + DirectedGraph::new(count, spec.arcs), + spec.source, + spec.sink, + spec.multipliers, + spec.capacities, + spec.requirement, + ) } } @@ -112,52 +111,67 @@ impl IntegralFlowWithMultipliers { capacities: Vec, requirement: i64, ) -> Self { - assert_eq!( - capacities.len(), - graph.num_arcs(), - "capacities length must match graph num_arcs" - ); - assert_eq!( - multipliers.len(), - graph.num_vertices(), - "multipliers length must match graph num_vertices" - ); + Self::try_new(graph, source, sink, multipliers, capacities, requirement) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + graph: DirectedGraph, + source: usize, + sink: usize, + multipliers: Vec, + capacities: Vec, + requirement: i64, + ) -> Result { + if capacities.len() != graph.num_arcs() { + return Err("capacities length must match graph num_arcs".into()); + } + if multipliers.len() != graph.num_vertices() { + return Err("multipliers length must match num_vertices".into()); + } let num_vertices = graph.num_vertices(); - assert!( - source < num_vertices, - "source ({source}) must be less than num_vertices ({num_vertices})" - ); - assert!( - sink < num_vertices, - "sink ({sink}) must be less than num_vertices ({num_vertices})" - ); - assert_ne!(source, sink, "source and sink must be distinct"); + if !(source < num_vertices) { + return Err(format!( + "source ({source}) must be less than num_vertices ({num_vertices})" + ) + .into()); + } + if !(sink < num_vertices) { + return Err( + format!("sink ({sink}) must be less than num_vertices ({num_vertices})").into(), + ); + } + if source == sink { + return Err("source and sink must be distinct".into()); + } for (vertex, &multiplier) in multipliers.iter().enumerate() { - if vertex != source && vertex != sink { - assert!(multiplier > 0, "non-terminal multipliers must be positive"); + if vertex != source && vertex != sink && !(multiplier > 0) { + return Err("non-terminal multipliers must be positive".into()); } } for &capacity in &capacities { - let domain = usize::try_from(capacity) + if usize::try_from(capacity) .ok() - .and_then(|value| value.checked_add(1)); - assert!( - domain.is_some(), - "arc capacities must fit into usize for dims()" - ); + .and_then(|value| value.checked_add(1)) + .is_none() + { + return Err( + "capacities must be nonnegative and their domains must fit into usize".into(), + ); + } } - Self { + Ok(Self { graph, source, sink, multipliers, capacities, requirement, - } + }) } pub fn graph(&self) -> &DirectedGraph { diff --git a/src/unit_tests/models/graph/integral_flow_bundles.rs b/src/unit_tests/models/graph/integral_flow_bundles.rs index d8c7b3f79..2d35b7886 100644 --- a/src/unit_tests/models/graph/integral_flow_bundles.rs +++ b/src/unit_tests/models/graph/integral_flow_bundles.rs @@ -114,3 +114,51 @@ fn test_integral_flow_bundles_problem_name() { "IntegralFlowBundles" ); } + +#[test] +fn creation_and_deserialization_enforce_the_same_flow_constraints() { + let input = serde_json::json!({ + "arcs": [[0, 1], [1, 2]], "num_vertices": 3, + "source": 0, "sink": 2, "requirement": 1, + "bundles": [[0], [1]], "bundle_capacities": [1, 1], + }); + let problem = IntegralFlowBundles::try_from( + serde_json::from_value::(input.clone()).unwrap(), + ) + .unwrap(); + let persisted = serde_json::to_value(&problem).unwrap(); + let restored: IntegralFlowBundles = serde_json::from_value(persisted.clone()).unwrap(); + assert_eq!( + restored.evaluate(&vec![1, 1]).unwrap(), + crate::types::Or(true) + ); + + for (field, value, message) in [ + ("source", serde_json::json!(3), "source"), + ("sink", serde_json::json!(3), "sink"), + ("sink", serde_json::json!(0), "distinct"), + ("bundle_capacities", serde_json::json!([1]), "length"), + ("requirement", serde_json::json!(0), "positive"), + ("requirement", serde_json::json!(-1), "positive"), + ("bundle_capacities", serde_json::json!([0, 1]), "positive"), + ("bundle_capacities", serde_json::json!([-1, 1]), "positive"), + ("bundles", serde_json::json!([[2], [1]]), "out of range"), + ("bundles", serde_json::json!([[0, 0], [1]]), "duplicate"), + ( + "bundles", + serde_json::json!([[0], []]), + "at least one bundle", + ), + ] { + let mut invalid_input = input.clone(); + invalid_input[field] = value.clone(); + let spec = serde_json::from_value::(invalid_input).unwrap(); + let error = IntegralFlowBundles::try_from(spec).unwrap_err(); + assert!(error.to_string().contains(message), "{field}: {error}"); + + let mut invalid_persisted = persisted.clone(); + invalid_persisted[field] = value; + let error = serde_json::from_value::(invalid_persisted).unwrap_err(); + assert!(error.to_string().contains(message), "{field}: {error}"); + } +} diff --git a/src/unit_tests/models/graph/integral_flow_homologous_arcs.rs b/src/unit_tests/models/graph/integral_flow_homologous_arcs.rs index e15116232..55abaa6d1 100644 --- a/src/unit_tests/models/graph/integral_flow_homologous_arcs.rs +++ b/src/unit_tests/models/graph/integral_flow_homologous_arcs.rs @@ -165,3 +165,52 @@ fn test_integral_flow_homologous_arcs_paper_example() { .iter() .all(|solution| problem.evaluate(solution).unwrap().0)); } + +#[test] +fn creation_and_deserialization_enforce_the_same_flow_constraints() { + let input = serde_json::json!({ + "arcs": [[0, 1], [1, 2]], "num_vertices": 3, + "source": 0, "sink": 2, "requirement": 1, + "capacities": [1, 1], "homologous_pairs": [[0, 1]], + }); + let problem = IntegralFlowHomologousArcs::try_from( + serde_json::from_value::(input.clone()).unwrap(), + ) + .unwrap(); + let persisted = serde_json::to_value(&problem).unwrap(); + let restored: IntegralFlowHomologousArcs = serde_json::from_value(persisted.clone()).unwrap(); + assert_eq!( + restored.evaluate(&vec![1, 1]).unwrap(), + crate::types::Or(true) + ); + + for (field, value, message) in [ + ("capacities", serde_json::json!([1]), "length"), + ("source", serde_json::json!(3), "source"), + ("sink", serde_json::json!(3), "sink"), + ( + "homologous_pairs", + serde_json::json!([[2, 1]]), + "out of range", + ), + ( + "homologous_pairs", + serde_json::json!([[0, 2]]), + "out of range", + ), + ("capacities", serde_json::json!([-1, 1]), "nonnegative"), + ] { + let mut invalid_input = input.clone(); + invalid_input[field] = value.clone(); + let spec = + serde_json::from_value::(invalid_input).unwrap(); + let error = IntegralFlowHomologousArcs::try_from(spec).unwrap_err(); + assert!(error.to_string().contains(message), "{field}: {error}"); + + let mut invalid_persisted = persisted.clone(); + invalid_persisted[field] = value; + let error = + serde_json::from_value::(invalid_persisted).unwrap_err(); + assert!(error.to_string().contains(message), "{field}: {error}"); + } +} diff --git a/src/unit_tests/models/graph/integral_flow_with_multipliers.rs b/src/unit_tests/models/graph/integral_flow_with_multipliers.rs index 394d59128..f3caf8687 100644 --- a/src/unit_tests/models/graph/integral_flow_with_multipliers.rs +++ b/src/unit_tests/models/graph/integral_flow_with_multipliers.rs @@ -175,3 +175,46 @@ fn test_integral_flow_with_multipliers_paper_example() { let all_solutions = solver.find_all_witnesses(&problem).unwrap(); assert!(all_solutions.iter().any(|solution| solution == &config)); } + +#[test] +fn creation_and_deserialization_enforce_the_same_flow_constraints() { + let input = serde_json::json!({ + "arcs": [[0, 1], [1, 2]], "num_vertices": 3, + "source": 0, "sink": 2, "requirement": 1, + "multipliers": [1, 1, 1], "capacities": [1, 1], + }); + let problem = IntegralFlowWithMultipliers::try_from( + serde_json::from_value::(input.clone()).unwrap(), + ) + .unwrap(); + let persisted = serde_json::to_value(&problem).unwrap(); + let restored: IntegralFlowWithMultipliers = serde_json::from_value(persisted.clone()).unwrap(); + assert_eq!( + restored.evaluate(&vec![1, 1]).unwrap(), + crate::types::Or(true) + ); + + for (field, value, message) in [ + ("capacities", serde_json::json!([1]), "length"), + ("multipliers", serde_json::json!([1, 1]), "length"), + ("source", serde_json::json!(3), "source"), + ("sink", serde_json::json!(3), "sink"), + ("sink", serde_json::json!(0), "distinct"), + ("multipliers", serde_json::json!([1, 0, 1]), "positive"), + ("multipliers", serde_json::json!([1, -1, 1]), "positive"), + ("capacities", serde_json::json!([-1, 1]), "nonnegative"), + ] { + let mut invalid_input = input.clone(); + invalid_input[field] = value.clone(); + let spec = + serde_json::from_value::(invalid_input).unwrap(); + let error = IntegralFlowWithMultipliers::try_from(spec).unwrap_err(); + assert!(error.to_string().contains(message), "{field}: {error}"); + + let mut invalid_persisted = persisted.clone(); + invalid_persisted[field] = value; + let error = + serde_json::from_value::(invalid_persisted).unwrap_err(); + assert!(error.to_string().contains(message), "{field}: {error}"); + } +} From ccc0f03360aaea9529ec7e2e4a0eb5dd358ddcb8 Mon Sep 17 00:00:00 2001 From: Xiwei Pan Date: Fri, 18 Sep 2026 01:38:25 +0800 Subject: [PATCH 2/6] Validate deserialized cost flow models through constructor checks --- src/models/graph/minimum_cost_circulation.rs | 53 ++++++++---- src/models/graph/minimum_edge_cost_flow.rs | 81 ++++++++++++++----- .../models/graph/minimum_cost_circulation.rs | 17 ++++ .../models/graph/minimum_edge_cost_flow.rs | 20 +++++ 4 files changed, 138 insertions(+), 33 deletions(-) diff --git a/src/models/graph/minimum_cost_circulation.rs b/src/models/graph/minimum_cost_circulation.rs index a61cea112..5c732cd54 100644 --- a/src/models/graph/minimum_cost_circulation.rs +++ b/src/models/graph/minimum_cost_circulation.rs @@ -85,6 +85,7 @@ inventory::submit! { /// assert_eq!(problem.total_cost(&witness).unwrap(), -5); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "MinimumCostCirculationData")] pub struct MinimumCostCirculation { /// The directed multigraph G = (V, A). graph: DirectedGraph, @@ -94,6 +95,20 @@ pub struct MinimumCostCirculation { costs: Vec, } +#[derive(Deserialize)] +struct MinimumCostCirculationData { + graph: DirectedGraph, + capacities: Vec, + costs: Vec, +} + +impl TryFrom for MinimumCostCirculation { + type Error = crate::registry::ConstructionError; + fn try_from(data: MinimumCostCirculationData) -> Result { + Self::try_new(data.graph, data.capacities, data.costs) + } +} + impl MinimumCostCirculation { /// Create a new Minimum-Cost Circulation problem. /// @@ -106,27 +121,35 @@ impl MinimumCostCirculation { /// /// Note: costs are signed and **may be negative**. pub fn new(graph: DirectedGraph, capacities: Vec, costs: Vec) -> Self { + Self::try_new(graph, capacities, costs).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + graph: DirectedGraph, + capacities: Vec, + costs: Vec, + ) -> Result { let m = graph.num_arcs(); - assert_eq!( - capacities.len(), - m, - "capacities length ({}) must match num_arcs ({m})", - capacities.len() - ); - assert_eq!( - costs.len(), - m, - "costs length ({}) must match num_arcs ({m})", - costs.len() - ); + if capacities.len() != m { + return Err(format!( + "capacities length ({}) must match num_arcs ({m})", + capacities.len() + ) + .into()); + } + if costs.len() != m { + return Err(format!("costs length ({}) must match num_arcs ({m})", costs.len()).into()); + } for (i, &c) in capacities.iter().enumerate() { - assert!(c >= 0, "capacity[{i}] = {c} is negative"); + if c < 0 { + return Err(format!("capacity[{i}] = {c} is negative").into()); + } } - Self { + Ok(Self { graph, capacities, costs, - } + }) } /// Get a reference to the underlying directed graph. diff --git a/src/models/graph/minimum_edge_cost_flow.rs b/src/models/graph/minimum_edge_cost_flow.rs index 79e0ab8dd..a704282b2 100644 --- a/src/models/graph/minimum_edge_cost_flow.rs +++ b/src/models/graph/minimum_edge_cost_flow.rs @@ -66,6 +66,7 @@ inventory::submit! { /// assert_eq!(problem.evaluate(&witness).unwrap(), problemreductions::types::Min(Some(3))); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "MinimumEdgeCostFlowData")] pub struct MinimumEdgeCostFlow { /// The directed graph G = (V, A). graph: DirectedGraph, @@ -81,6 +82,30 @@ pub struct MinimumEdgeCostFlow { required_flow: i64, } +#[derive(Deserialize)] +struct MinimumEdgeCostFlowData { + graph: DirectedGraph, + prices: Vec, + capacities: Vec, + source: usize, + sink: usize, + required_flow: i64, +} + +impl TryFrom for MinimumEdgeCostFlow { + type Error = crate::registry::ConstructionError; + fn try_from(data: MinimumEdgeCostFlowData) -> Result { + Self::try_new( + data.graph, + data.prices, + data.capacities, + data.source, + data.sink, + data.required_flow, + ) + } +} + impl MinimumEdgeCostFlow { /// Create a new Minimum Edge-Cost Flow problem. /// @@ -110,34 +135,54 @@ impl MinimumEdgeCostFlow { sink: usize, required_flow: i64, ) -> Self { + Self::try_new(graph, prices, capacities, source, sink, required_flow) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + graph: DirectedGraph, + prices: Vec, + capacities: Vec, + source: usize, + sink: usize, + required_flow: i64, + ) -> Result { let n = graph.num_vertices(); let m = graph.num_arcs(); - assert_eq!( - prices.len(), - m, - "prices length ({}) must match num_arcs ({m})", - prices.len() - ); - assert_eq!( - capacities.len(), - m, - "capacities length ({}) must match num_arcs ({m})", - capacities.len() - ); - assert!(source < n, "source ({source}) >= num_vertices ({n})"); - assert!(sink < n, "sink ({sink}) >= num_vertices ({n})"); - assert_ne!(source, sink, "source and sink must be distinct"); + if prices.len() != m { + return Err( + format!("prices length ({}) must match num_arcs ({m})", prices.len()).into(), + ); + } + if capacities.len() != m { + return Err(format!( + "capacities length ({}) must match num_arcs ({m})", + capacities.len() + ) + .into()); + } + if source >= n { + return Err(format!("source ({source}) >= num_vertices ({n})").into()); + } + if sink >= n { + return Err(format!("sink ({sink}) >= num_vertices ({n})").into()); + } + if source == sink { + return Err("source and sink must be distinct".into()); + } for (i, &c) in capacities.iter().enumerate() { - assert!(c >= 0, "capacity[{i}] = {c} is negative"); + if c < 0 { + return Err(format!("capacity[{i}] = {c} is negative").into()); + } } - Self { + Ok(Self { graph, prices, capacities, source, sink, required_flow, - } + }) } /// Get a reference to the underlying directed graph. diff --git a/src/unit_tests/models/graph/minimum_cost_circulation.rs b/src/unit_tests/models/graph/minimum_cost_circulation.rs index c22563730..7505080c6 100644 --- a/src/unit_tests/models/graph/minimum_cost_circulation.rs +++ b/src/unit_tests/models/graph/minimum_cost_circulation.rs @@ -1,4 +1,21 @@ use super::*; + +#[test] +fn test_minimum_cost_circulation_deserialization_rejects_invalid_fields() { + let valid = serde_json::to_value(canonical_instance()).unwrap(); + for (field, value) in [ + ("capacities", serde_json::json!([])), + ("costs", serde_json::json!([])), + ("capacities", serde_json::json!([-1, 2, 1, 1])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; use crate::topology::DirectedGraph; diff --git a/src/unit_tests/models/graph/minimum_edge_cost_flow.rs b/src/unit_tests/models/graph/minimum_edge_cost_flow.rs index 6253a1c80..aafd189eb 100644 --- a/src/unit_tests/models/graph/minimum_edge_cost_flow.rs +++ b/src/unit_tests/models/graph/minimum_edge_cost_flow.rs @@ -1,4 +1,24 @@ use super::*; + +#[test] +fn test_minimum_edge_cost_flow_deserialization_rejects_invalid_fields() { + let valid = serde_json::to_value(issue_instance()).unwrap(); + for (field, value) in [ + ("prices", serde_json::json!([])), + ("capacities", serde_json::json!([])), + ("capacities", serde_json::json!([-1, 2, 2, 2, 2, 2])), + ("source", serde_json::json!(5)), + ("sink", serde_json::json!(5)), + ("sink", serde_json::json!(0)), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; use crate::topology::DirectedGraph; From a1da7e3f632f131f58124e1ba52a622d9055557f Mon Sep 17 00:00:00 2001 From: Xiwei Pan Date: Fri, 18 Sep 2026 01:43:38 +0800 Subject: [PATCH 3/6] Share lower-bounded flow validation across construction inputs --- .../graph/undirected_flow_lower_bounds.rs | 105 +++++++----------- .../graph/undirected_flow_lower_bounds.rs | 28 +++++ 2 files changed, 69 insertions(+), 64 deletions(-) diff --git a/src/models/graph/undirected_flow_lower_bounds.rs b/src/models/graph/undirected_flow_lower_bounds.rs index ff9b0657f..d9e610056 100644 --- a/src/models/graph/undirected_flow_lower_bounds.rs +++ b/src/models/graph/undirected_flow_lower_bounds.rs @@ -33,6 +33,7 @@ inventory::submit! { } #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "UndirectedFlowLowerBoundsCreateSpec")] pub struct UndirectedFlowLowerBounds { graph: SimpleGraph, capacities: Vec, @@ -60,50 +61,14 @@ struct UndirectedFlowLowerBoundsCreateSpec { impl TryFrom for UndirectedFlowLowerBounds { type Error = crate::registry::ConstructionError; fn try_from(spec: UndirectedFlowLowerBoundsCreateSpec) -> Result { - let edges = spec.graph.num_edges(); - if spec.capacities.len() != edges { - return Err(format!( - "capacities has {} entries, expected {edges}", - spec.capacities.len() - ) - .into()); - } - if spec.lower_bounds.len() != edges { - return Err(format!( - "lower_bounds has {} entries, expected {edges}", - spec.lower_bounds.len() - ) - .into()); - } - let vertices = spec.graph.num_vertices(); - if spec.source >= vertices || spec.sink >= vertices { - return Err("source and sink must be valid graph vertices" - .to_string() - .into()); - } - if spec.source == spec.sink { - return Err("source and sink must be distinct".to_string().into()); - } - if spec.requirement == 0 { - return Err("requirement must be at least 1".to_string().into()); - } - if let Some((index, _)) = spec - .lower_bounds - .iter() - .zip(&spec.capacities) - .enumerate() - .find(|(_, (&lower, &upper))| lower > upper) - { - return Err(format!("lower bound at edge {index} exceeds its capacity").into()); - } - Ok(Self::new( + Self::try_new( spec.graph, spec.capacities, spec.lower_bounds, spec.source, spec.sink, spec.requirement, - )) + ) } } @@ -116,44 +81,56 @@ impl UndirectedFlowLowerBounds { sink: usize, requirement: i64, ) -> Self { - assert_eq!( - capacities.len(), - graph.num_edges(), - "capacities length must match graph num_edges" - ); - assert_eq!( - lower_bounds.len(), - graph.num_edges(), - "lower_bounds length must match graph num_edges" - ); + Self::try_new(graph, capacities, lower_bounds, source, sink, requirement) + .unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + graph: SimpleGraph, + capacities: Vec, + lower_bounds: Vec, + source: usize, + sink: usize, + requirement: i64, + ) -> Result { + if capacities.len() != graph.num_edges() { + return Err("capacities length must match graph num_edges".into()); + } + if lower_bounds.len() != graph.num_edges() { + return Err("lower_bounds length must match graph num_edges".into()); + } let num_vertices = graph.num_vertices(); - assert!( - source < num_vertices, - "source must be less than num_vertices ({num_vertices})" - ); - assert!( - sink < num_vertices, - "sink must be less than num_vertices ({num_vertices})" - ); - assert!(source != sink, "source and sink must be distinct"); - assert!(requirement >= 1, "requirement must be at least 1"); + if source >= num_vertices { + return Err(format!("source must be less than num_vertices ({num_vertices})").into()); + } + if sink >= num_vertices { + return Err(format!("sink must be less than num_vertices ({num_vertices})").into()); + } + if source == sink { + return Err("source and sink must be distinct".into()); + } + if requirement < 1 { + return Err("requirement must be at least 1".into()); + } for (edge_index, (&lower, &upper)) in lower_bounds.iter().zip(&capacities).enumerate() { - assert!( - lower <= upper, - "lower bound at edge {edge_index} must be at most its capacity" - ); + if lower > upper { + return Err(format!( + "lower bound at edge {edge_index} must be at most its capacity" + ) + .into()); + } } - Self { + Ok(Self { graph, capacities, lower_bounds, source, sink, requirement, - } + }) } pub fn graph(&self) -> &SimpleGraph { diff --git a/src/unit_tests/models/graph/undirected_flow_lower_bounds.rs b/src/unit_tests/models/graph/undirected_flow_lower_bounds.rs index c67868b1c..143b83fd1 100644 --- a/src/unit_tests/models/graph/undirected_flow_lower_bounds.rs +++ b/src/unit_tests/models/graph/undirected_flow_lower_bounds.rs @@ -1,4 +1,32 @@ use super::*; + +#[test] +fn test_undirected_flow_lower_bounds_invalid_inputs() { + let valid = serde_json::to_value(canonical_yes_instance()).unwrap(); + for (field, value) in [ + ("capacities", serde_json::json!([])), + ("lower_bounds", serde_json::json!([])), + ("source", serde_json::json!(6)), + ("sink", serde_json::json!(6)), + ("sink", serde_json::json!(0)), + ("requirement", serde_json::json!(0)), + ("requirement", serde_json::json!(-1)), + ("lower_bounds", serde_json::json!([3, 1, 0, 0, 1, 0, 1])), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + let spec = + serde_json::from_value::(invalid.clone()).unwrap(); + assert!( + UndirectedFlowLowerBounds::try_from(spec).is_err(), + "{field}" + ); + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForceProblem as _; #[test] From e299981a8da37afaf41954efbcd93b45fa0e4b85 Mon Sep 17 00:00:00 2001 From: Xiwei Pan Date: Fri, 18 Sep 2026 01:50:06 +0800 Subject: [PATCH 4/6] Validate two-commodity flow deserialization with shared constructors --- .../directed_two_commodity_integral_flow.rs | 95 ++++++++++--- .../undirected_two_commodity_integral_flow.rs | 128 +++++++++++------- .../directed_two_commodity_integral_flow.rs | 22 +++ .../undirected_two_commodity_integral_flow.rs | 20 +++ 4 files changed, 197 insertions(+), 68 deletions(-) diff --git a/src/models/graph/directed_two_commodity_integral_flow.rs b/src/models/graph/directed_two_commodity_integral_flow.rs index 11d7e53ef..f40e40dba 100644 --- a/src/models/graph/directed_two_commodity_integral_flow.rs +++ b/src/models/graph/directed_two_commodity_integral_flow.rs @@ -68,6 +68,7 @@ inventory::submit! { /// assert!(solver.solve(&problem).unwrap().is_some()); /// ``` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "DirectedTwoCommodityIntegralFlowData")] pub struct DirectedTwoCommodityIntegralFlow { /// The directed graph G = (V, A). graph: DirectedGraph, @@ -87,6 +88,34 @@ pub struct DirectedTwoCommodityIntegralFlow { requirement_2: i64, } +#[derive(Deserialize)] +struct DirectedTwoCommodityIntegralFlowData { + graph: DirectedGraph, + capacities: Vec, + source_1: usize, + sink_1: usize, + source_2: usize, + sink_2: usize, + requirement_1: i64, + requirement_2: i64, +} + +impl TryFrom for DirectedTwoCommodityIntegralFlow { + type Error = crate::registry::ConstructionError; + fn try_from(data: DirectedTwoCommodityIntegralFlowData) -> Result { + Self::try_new( + data.graph, + data.capacities, + data.source_1, + data.sink_1, + data.source_2, + data.sink_2, + data.requirement_1, + data.requirement_2, + ) + } +} + impl DirectedTwoCommodityIntegralFlow { /// Create a new Directed Two-Commodity Integral Flow problem. /// @@ -106,25 +135,7 @@ impl DirectedTwoCommodityIntegralFlow { requirement_1: i64, requirement_2: i64, ) -> Self { - let n = graph.num_vertices(); - assert_eq!( - capacities.len(), - graph.num_arcs(), - "capacities length must match graph num_arcs" - ); - assert!( - capacities.iter().all(|&capacity| capacity >= 0), - "capacities must be nonnegative" - ); - assert!( - requirement_1 >= 0 && requirement_2 >= 0, - "flow requirements must be nonnegative" - ); - assert!(source_1 < n, "source_1 ({source_1}) >= num_vertices ({n})"); - assert!(sink_1 < n, "sink_1 ({sink_1}) >= num_vertices ({n})"); - assert!(source_2 < n, "source_2 ({source_2}) >= num_vertices ({n})"); - assert!(sink_2 < n, "sink_2 ({sink_2}) >= num_vertices ({n})"); - Self { + Self::try_new( graph, capacities, source_1, @@ -133,7 +144,53 @@ impl DirectedTwoCommodityIntegralFlow { sink_2, requirement_1, requirement_2, + ) + .unwrap_or_else(|error| panic!("{error}")) + } + + #[allow(clippy::too_many_arguments)] + fn try_new( + graph: DirectedGraph, + capacities: Vec, + source_1: usize, + sink_1: usize, + source_2: usize, + sink_2: usize, + requirement_1: i64, + requirement_2: i64, + ) -> Result { + let n = graph.num_vertices(); + if capacities.len() != graph.num_arcs() { + return Err("capacities length must match graph num_arcs".into()); + } + if capacities.iter().any(|&capacity| capacity < 0) { + return Err("capacities must be nonnegative".into()); + } + if requirement_1 < 0 || requirement_2 < 0 { + return Err("flow requirements must be nonnegative".into()); + } + if source_1 >= n { + return Err(format!("source_1 ({source_1}) >= num_vertices ({n})").into()); + } + if sink_1 >= n { + return Err(format!("sink_1 ({sink_1}) >= num_vertices ({n})").into()); } + if source_2 >= n { + return Err(format!("source_2 ({source_2}) >= num_vertices ({n})").into()); + } + if sink_2 >= n { + return Err(format!("sink_2 ({sink_2}) >= num_vertices ({n})").into()); + } + Ok(Self { + graph, + capacities, + source_1, + sink_1, + source_2, + sink_2, + requirement_1, + requirement_2, + }) } /// Get a reference to the underlying directed graph. diff --git a/src/models/graph/undirected_two_commodity_integral_flow.rs b/src/models/graph/undirected_two_commodity_integral_flow.rs index ad8a584f1..684fa4892 100644 --- a/src/models/graph/undirected_two_commodity_integral_flow.rs +++ b/src/models/graph/undirected_two_commodity_integral_flow.rs @@ -30,6 +30,7 @@ inventory::submit! { /// - `f2(u, v)` /// - `f2(v, u)` #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "UndirectedTwoCommodityIntegralFlowData")] pub struct UndirectedTwoCommodityIntegralFlow { graph: SimpleGraph, capacities: Vec, @@ -41,6 +42,34 @@ pub struct UndirectedTwoCommodityIntegralFlow { requirement_2: i64, } +#[derive(Deserialize)] +struct UndirectedTwoCommodityIntegralFlowData { + graph: SimpleGraph, + capacities: Vec, + source_1: usize, + sink_1: usize, + source_2: usize, + sink_2: usize, + requirement_1: i64, + requirement_2: i64, +} + +impl TryFrom for UndirectedTwoCommodityIntegralFlow { + type Error = crate::registry::ConstructionError; + fn try_from(data: UndirectedTwoCommodityIntegralFlowData) -> Result { + Self::try_new( + data.graph, + data.capacities, + data.source_1, + data.sink_1, + data.source_2, + data.sink_2, + data.requirement_1, + data.requirement_2, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct UndirectedTwoCommodityIntegralFlowCreateSpec { /// Undirected graph edges. @@ -82,38 +111,16 @@ impl TryFrom for UndirectedTwoComm if count < inferred { return Err("num_vertices is too small for graph endpoints".into()); } - if spec.capacities.len() != spec.graph.len() { - return Err("capacities length must match graph edge count".into()); - } - for &capacity in &spec.capacities { - if usize::try_from(capacity) - .ok() - .and_then(|v| v.checked_add(1)) - .is_none() - { - return Err("capacity is too large for this platform".into()); - } - } - for (label, vertex) in [ - ("source_1", spec.source_1), - ("sink_1", spec.sink_1), - ("source_2", spec.source_2), - ("sink_2", spec.sink_2), - ] { - if vertex >= count { - return Err(format!("{label} must be less than num_vertices").into()); - } - } - Ok(Self { - graph: SimpleGraph::new(count, spec.graph), - capacities: spec.capacities, - source_1: spec.source_1, - sink_1: spec.sink_1, - source_2: spec.source_2, - sink_2: spec.sink_2, - requirement_1: spec.requirement_1, - requirement_2: spec.requirement_2, - }) + Self::try_new( + SimpleGraph::new(count, spec.graph), + spec.capacities, + spec.source_1, + spec.sink_1, + spec.source_2, + spec.sink_2, + spec.requirement_1, + spec.requirement_2, + ) } } @@ -129,11 +136,33 @@ impl UndirectedTwoCommodityIntegralFlow { requirement_1: i64, requirement_2: i64, ) -> Self { - assert_eq!( - capacities.len(), - graph.num_edges(), - "capacities length must match graph num_edges" - ); + Self::try_new( + graph, + capacities, + source_1, + sink_1, + source_2, + sink_2, + requirement_1, + requirement_2, + ) + .unwrap_or_else(|error| panic!("{error}")) + } + + #[allow(clippy::too_many_arguments)] + fn try_new( + graph: SimpleGraph, + capacities: Vec, + source_1: usize, + sink_1: usize, + source_2: usize, + sink_2: usize, + requirement_1: i64, + requirement_2: i64, + ) -> Result { + if capacities.len() != graph.num_edges() { + return Err("capacities length must match graph edge count".into()); + } let num_vertices = graph.num_vertices(); for (label, vertex) in [ @@ -142,23 +171,24 @@ impl UndirectedTwoCommodityIntegralFlow { ("source_2", source_2), ("sink_2", sink_2), ] { - assert!( - vertex < num_vertices, - "{label} must be less than num_vertices ({num_vertices})" - ); + if vertex >= num_vertices { + return Err( + format!("{label} must be less than num_vertices ({num_vertices})").into(), + ); + } } for &capacity in &capacities { - let domain = usize::try_from(capacity) + if usize::try_from(capacity) .ok() - .and_then(|value| value.checked_add(1)); - assert!( - domain.is_some(), - "edge capacities must fit into usize for dims()" - ); + .and_then(|value| value.checked_add(1)) + .is_none() + { + return Err("edge capacities must fit into usize for dims()".into()); + } } - Self { + Ok(Self { graph, capacities, source_1, @@ -167,7 +197,7 @@ impl UndirectedTwoCommodityIntegralFlow { sink_2, requirement_1, requirement_2, - } + }) } pub fn graph(&self) -> &SimpleGraph { diff --git a/src/unit_tests/models/graph/directed_two_commodity_integral_flow.rs b/src/unit_tests/models/graph/directed_two_commodity_integral_flow.rs index 444a2e69f..eaf0437a3 100644 --- a/src/unit_tests/models/graph/directed_two_commodity_integral_flow.rs +++ b/src/unit_tests/models/graph/directed_two_commodity_integral_flow.rs @@ -1,4 +1,26 @@ use super::*; + +#[test] +fn test_directed_two_commodity_integral_flow_invalid_json() { + let valid = serde_json::to_value(yes_instance()).unwrap(); + for (field, value) in [ + ("capacities", serde_json::json!([])), + ("capacities", serde_json::json!([-1, 1, 1, 1, 1, 1, 1, 1])), + ("source_1", serde_json::json!(6)), + ("sink_1", serde_json::json!(6)), + ("source_2", serde_json::json!(6)), + ("sink_2", serde_json::json!(6)), + ("requirement_1", serde_json::json!(-1)), + ("requirement_2", serde_json::json!(-1)), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForce; use crate::solvers::BruteForceProblem as _; use crate::topology::DirectedGraph; diff --git a/src/unit_tests/models/graph/undirected_two_commodity_integral_flow.rs b/src/unit_tests/models/graph/undirected_two_commodity_integral_flow.rs index d09689d25..874845871 100644 --- a/src/unit_tests/models/graph/undirected_two_commodity_integral_flow.rs +++ b/src/unit_tests/models/graph/undirected_two_commodity_integral_flow.rs @@ -1,4 +1,24 @@ use super::*; + +#[test] +fn test_undirected_two_commodity_integral_flow_invalid_json() { + let valid = serde_json::to_value(canonical_instance()).unwrap(); + for (field, value) in [ + ("capacities", serde_json::json!([])), + ("capacities", serde_json::json!([-1, 1, 2])), + ("source_1", serde_json::json!(4)), + ("sink_1", serde_json::json!(4)), + ("source_2", serde_json::json!(4)), + ("sink_2", serde_json::json!(4)), + ] { + let mut invalid = valid.clone(); + invalid[field] = value; + assert!( + serde_json::from_value::(invalid).is_err(), + "{field}" + ); + } +} use crate::solvers::BruteForceProblem as _; #[test] From b48114cdecbae2a3c27d4a36df9d94e27348be4d Mon Sep 17 00:00:00 2001 From: Xiwei Pan Date: Fri, 18 Sep 2026 02:39:52 +0800 Subject: [PATCH 5/6] Validate postman and prescribed-path flow deserialization --- src/models/graph/mixed_chinese_postman.rs | 21 +++++++- .../graph/path_constrained_network_flow.rs | 25 +++++++++ .../models/graph/mixed_chinese_postman.rs | 38 ++++++++++++++ .../graph/path_constrained_network_flow.rs | 51 +++++++++++++++++++ 4 files changed, 134 insertions(+), 1 deletion(-) diff --git a/src/models/graph/mixed_chinese_postman.rs b/src/models/graph/mixed_chinese_postman.rs index 8bd02cb1e..9a352504f 100644 --- a/src/models/graph/mixed_chinese_postman.rs +++ b/src/models/graph/mixed_chinese_postman.rs @@ -35,13 +35,32 @@ inventory::submit! { /// edge. The minimum-cost closed walk is then computed via the directed Chinese /// Postman subproblem, using all available arcs (including both directions of /// every undirected edge) for degree-balancing detours. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize)] pub struct MixedChinesePostman> { graph: MixedGraph, arc_weights: Vec, edge_weights: Vec, } +#[derive(Deserialize)] +#[serde(bound(deserialize = "W: WeightElement + Deserialize<'de>"))] +struct MixedChinesePostmanData> { + graph: MixedGraph, + arc_weights: Vec, + edge_weights: Vec, +} + +impl<'de, W> Deserialize<'de> for MixedChinesePostman +where + W: WeightElement + Deserialize<'de>, +{ + fn deserialize>(deserializer: D) -> Result { + let data = MixedChinesePostmanData::::deserialize(deserializer)?; + Self::try_new(data.graph, data.arc_weights, data.edge_weights) + .map_err(serde::de::Error::custom) + } +} + macro_rules! mixed_chinese_postman_create_spec { ($name:ident, $weight:ty, $one:expr $(, $arc_weights:ident, $edge_weights:ident)?) => { #[derive(Debug, Deserialize, crate::CreateSpec)] diff --git a/src/models/graph/path_constrained_network_flow.rs b/src/models/graph/path_constrained_network_flow.rs index e9f324664..f1a095295 100644 --- a/src/models/graph/path_constrained_network_flow.rs +++ b/src/models/graph/path_constrained_network_flow.rs @@ -34,6 +34,7 @@ inventory::submit! { /// - the induced arc loads do not exceed the arc capacities /// - the total delivered flow reaches the requirement #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "PathConstrainedNetworkFlowData")] pub struct PathConstrainedNetworkFlow { graph: DirectedGraph, capacities: Vec, @@ -43,6 +44,30 @@ pub struct PathConstrainedNetworkFlow { requirement: i64, } +#[derive(Deserialize)] +struct PathConstrainedNetworkFlowData { + graph: DirectedGraph, + capacities: Vec, + source: usize, + sink: usize, + paths: Vec>, + requirement: i64, +} + +impl TryFrom for PathConstrainedNetworkFlow { + type Error = crate::registry::ConstructionError; + fn try_from(data: PathConstrainedNetworkFlowData) -> Result { + Self::try_new( + data.graph, + data.capacities, + data.source, + data.sink, + data.paths, + data.requirement, + ) + } +} + #[derive(Debug, Deserialize, crate::CreateSpec)] struct PathConstrainedNetworkFlowCreateSpec { /// Directed graph arcs. diff --git a/src/unit_tests/models/graph/mixed_chinese_postman.rs b/src/unit_tests/models/graph/mixed_chinese_postman.rs index 5c8b3cbc8..319188360 100644 --- a/src/unit_tests/models/graph/mixed_chinese_postman.rs +++ b/src/unit_tests/models/graph/mixed_chinese_postman.rs @@ -179,3 +179,41 @@ fn test_mixed_chinese_postman_ignores_isolated_vertices() { Min(Some(69)) ); } + +#[test] +fn test_mixed_chinese_postman_deserialization_rejects_invalid_weights() { + let valid = serde_json::to_value(sample_instance()).unwrap(); + let restored: MixedChinesePostman = serde_json::from_value(valid.clone()).unwrap(); + assert_eq!(serde_json::to_value(&restored).unwrap(), valid); + + let cases = [ + ( + "arc_weights", + serde_json::json!([2, 3, 1]), + "arc_weights length must match num_arcs", + ), + ( + "edge_weights", + serde_json::json!([2, 3, 1, 2, 7]), + "edge_weights length must match num_edges", + ), + ( + "arc_weights", + serde_json::json!([2, 3, -1, 4]), + "arc weight at index 2 must be nonnegative", + ), + ( + "edge_weights", + serde_json::json!([2, -3, 1, 2]), + "edge weight at index 1 must be nonnegative", + ), + ]; + for (field, value, expected) in cases { + let mut json = valid.clone(); + json[field] = value; + let error = serde_json::from_value::>(json) + .unwrap_err() + .to_string(); + assert_eq!(error, format!("problem construction failed: {expected}")); + } +} diff --git a/src/unit_tests/models/graph/path_constrained_network_flow.rs b/src/unit_tests/models/graph/path_constrained_network_flow.rs index af1cf3617..be52fdc44 100644 --- a/src/unit_tests/models/graph/path_constrained_network_flow.rs +++ b/src/unit_tests/models/graph/path_constrained_network_flow.rs @@ -173,3 +173,54 @@ fn test_path_constrained_network_flow_paper_example() { assert_eq!(all.len(), 2); assert!(all.contains(&config)); } + +#[test] +fn test_path_constrained_network_flow_deserialization_rejects_invalid_instances() { + let valid = serde_json::to_value(yes_instance()).unwrap(); + let restored: PathConstrainedNetworkFlow = serde_json::from_value(valid.clone()).unwrap(); + assert_eq!(serde_json::to_value(&restored).unwrap(), valid); + + let cases = [ + ( + "paths", + serde_json::json!([[99]]), + "arc index 99 out of bounds", + ), + ("paths", serde_json::json!([[]]), "must be non-empty"), + ("paths", serde_json::json!([[0, 5]]), "not contiguous"), + ( + "paths", + serde_json::json!([[0, 2, 5, 8], [0, 2]]), + "path 1: ", + ), + ( + "paths", + serde_json::json!([[0, 2]]), + "must end at sink 7, ended at 3", + ), + ( + "capacities", + serde_json::json!([1, 1]), + "capacities length must match graph num_arcs", + ), + ( + "source", + serde_json::json!(8), + "source (8) >= num_vertices (8)", + ), + ("sink", serde_json::json!(8), "sink (8) >= num_vertices (8)"), + ( + "sink", + serde_json::json!(0), + "source and sink must be distinct", + ), + ]; + for (field, value, expected) in cases { + let mut json = valid.clone(); + json[field] = value; + let error = serde_json::from_value::(json) + .unwrap_err() + .to_string(); + assert!(error.contains(expected), "{field}: {error}"); + } +} From effeae4783d02bee6dcbd4a2ef48ebf39f913f1c Mon Sep 17 00:00:00 2001 From: GiggleLiu Date: Fri, 18 Sep 2026 11:29:25 +0800 Subject: [PATCH 6/6] Simplify negated comparisons flagged by clippy nonminimal_bool Co-Authored-By: Claude Fable 5.1 --- src/models/graph/integral_flow_bundles.rs | 8 ++++---- src/models/graph/integral_flow_homologous_arcs.rs | 8 ++++---- src/models/graph/integral_flow_with_multipliers.rs | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/models/graph/integral_flow_bundles.rs b/src/models/graph/integral_flow_bundles.rs index 61336c0a6..cd5e55536 100644 --- a/src/models/graph/integral_flow_bundles.rs +++ b/src/models/graph/integral_flow_bundles.rs @@ -128,10 +128,10 @@ impl IntegralFlowBundles { let num_vertices = graph.num_vertices(); let num_arcs = graph.num_arcs(); - if !(source < num_vertices) { + if source >= num_vertices { return Err(format!("source ({source}) >= num_vertices ({num_vertices})").into()); } - if !(sink < num_vertices) { + if sink >= num_vertices { return Err(format!("sink ({sink}) >= num_vertices ({num_vertices})").into()); } if source == sink { @@ -150,7 +150,7 @@ impl IntegralFlowBundles { for (bundle_index, (bundle, &capacity)) in bundles.iter().zip(&bundle_capacities).enumerate() { - if !(capacity > 0) { + if capacity <= 0 { return Err( format!("bundle capacity at index {bundle_index} must be positive").into(), ); @@ -158,7 +158,7 @@ impl IntegralFlowBundles { let mut seen = BTreeSet::new(); for &arc_index in bundle { - if !(arc_index < num_arcs) { + if arc_index >= num_arcs { return Err(format!("bundle {bundle_index} arc is out of range: index {arc_index}, num_arcs {num_arcs}").into()); } if !(seen.insert(arc_index)) { diff --git a/src/models/graph/integral_flow_homologous_arcs.rs b/src/models/graph/integral_flow_homologous_arcs.rs index d5ec2ba1a..75a1b1599 100644 --- a/src/models/graph/integral_flow_homologous_arcs.rs +++ b/src/models/graph/integral_flow_homologous_arcs.rs @@ -143,23 +143,23 @@ impl IntegralFlowHomologousArcs { if capacities.len() != num_arcs { return Err("capacities length must match graph.num_arcs()".into()); } - if !(source < num_vertices) { + if source >= num_vertices { return Err(format!( "source ({source}) must be less than num_vertices ({num_vertices})" ) .into()); } - if !(sink < num_vertices) { + if sink >= num_vertices { return Err( format!("sink ({sink}) must be less than num_vertices ({num_vertices})").into(), ); } for &(a, b) in &homologous_pairs { - if !(a < num_arcs) { + if a >= num_arcs { return Err(format!("homologous arc index {a} out of range").into()); } - if !(b < num_arcs) { + if b >= num_arcs { return Err(format!("homologous arc index {b} out of range").into()); } } diff --git a/src/models/graph/integral_flow_with_multipliers.rs b/src/models/graph/integral_flow_with_multipliers.rs index 6fd7196fb..b0af094d5 100644 --- a/src/models/graph/integral_flow_with_multipliers.rs +++ b/src/models/graph/integral_flow_with_multipliers.rs @@ -131,13 +131,13 @@ impl IntegralFlowWithMultipliers { } let num_vertices = graph.num_vertices(); - if !(source < num_vertices) { + if source >= num_vertices { return Err(format!( "source ({source}) must be less than num_vertices ({num_vertices})" ) .into()); } - if !(sink < num_vertices) { + if sink >= num_vertices { return Err( format!("sink ({sink}) must be less than num_vertices ({num_vertices})").into(), ); @@ -147,7 +147,7 @@ impl IntegralFlowWithMultipliers { } for (vertex, &multiplier) in multipliers.iter().enumerate() { - if vertex != source && vertex != sink && !(multiplier > 0) { + if vertex != source && vertex != sink && multiplier <= 0 { return Err("non-terminal multipliers must be positive".into()); } }