Validate structured graph and planning model inputs

problemreductions-cli/src/commands/create/tests.rs

@@ -1251,7 +1251,7 @@ fn test_create_production_planning_rejects_mismatched_period_lengths() {
     let err = create(&args, &out).unwrap_err();
     assert!(err
         .to_string()
-        .contains("demands has 5 entries, expected 6"));
+        .contains("all per-period vectors must have length num_periods"));
 }
 
 #[test]

src/models/graph/acyclic_partition.rs

@@ -29,7 +29,7 @@ inventory::submit! {
 }
 
 /// Acyclic Partition (Garey & Johnson ND15).
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize)]
 pub struct AcyclicPartition<W: WeightElement> {
     graph: DirectedGraph,
     vertex_weights: Vec<W>,
@@ -38,6 +38,34 @@ pub struct AcyclicPartition<W: WeightElement> {
     cost_bound: W::Sum,
 }
 
+#[derive(Deserialize)]
+#[serde(bound(deserialize = "W: WeightElement + Deserialize<'de>, W::Sum: Deserialize<'de>"))]
+struct AcyclicPartitionData<W: WeightElement> {
+    graph: DirectedGraph,
+    vertex_weights: Vec<W>,
+    arc_costs: Vec<W>,
+    weight_bound: W::Sum,
+    cost_bound: W::Sum,
+}
+
+impl<'de, W> Deserialize<'de> for AcyclicPartition<W>
+where
+    W: WeightElement + Deserialize<'de>,
+    W::Sum: Deserialize<'de>,
+{
+    fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        let data = AcyclicPartitionData::<W>::deserialize(deserializer)?;
+        Self::try_new(
+            data.graph,
+            data.vertex_weights,
+            data.arc_costs,
+            data.weight_bound,
+            data.cost_bound,
+        )
+        .map_err(serde::de::Error::custom)
+    }
+}
+
 #[derive(Debug, Deserialize, crate::CreateSpec)]
 struct AcyclicPartitionCreateSpec {
     #[create(codec = "arc-list")]
@@ -76,31 +104,16 @@ impl TryFrom<AcyclicPartitionCreateSpec> for AcyclicPartition<i64> {
         }
         let graph = DirectedGraph::new(num_vertices, spec.arcs);
         let vertex_weights = spec.weights.unwrap_or_else(|| vec![1; num_vertices]);
-        if vertex_weights.len() != num_vertices {
-            return Err(format!(
-                "weights has length {}, expected {num_vertices}",
-                vertex_weights.len()
-            )
-            .into());
-        }
         let arc_costs = spec
             .arc_weights
             .unwrap_or_else(|| vec![1; graph.num_arcs()]);
-        if arc_costs.len() != graph.num_arcs() {
-            return Err(format!(
-                "arc_weights has length {}, expected {}",
-                arc_costs.len(),
-                graph.num_arcs()
-            )
-            .into());
-        }
-        Ok(Self::new(
+        Self::try_new(
             graph,
             vertex_weights,
             arc_costs,
             spec.weight_bound,
             spec.cost_bound,
-        ))
+        )
     }
 }
 
@@ -113,23 +126,26 @@ impl<W: WeightElement> AcyclicPartition<W> {
         weight_bound: W::Sum,
         cost_bound: W::Sum,
     ) -> Self {
-        assert_eq!(
-            vertex_weights.len(),
-            graph.num_vertices(),
-            "vertex_weights length must match graph num_vertices"
-        );
-        assert_eq!(
-            arc_costs.len(),
-            graph.num_arcs(),
-            "arc_costs length must match graph num_arcs"
-        );
-        Self {
+        Self::try_new(graph, vertex_weights, arc_costs, weight_bound, cost_bound)
+            .unwrap_or_else(|error| panic!("{error}"))
+    }
+
+    fn try_new(
+        graph: DirectedGraph,
+        vertex_weights: Vec<W>,
+        arc_costs: Vec<W>,
+        weight_bound: W::Sum,
+        cost_bound: W::Sum,
+    ) -> Result<Self, crate::registry::ConstructionError> {
+        Self::check_vertex_weights(&graph, &vertex_weights)?;
+        Self::check_arc_costs(&graph, &arc_costs)?;
+        Ok(Self {
             graph,
             vertex_weights,
             arc_costs,
             weight_bound,
             cost_bound,
-        }
+        })
     }
 
     /// Get the underlying graph.
@@ -149,24 +165,37 @@ impl<W: WeightElement> AcyclicPartition<W> {
 
     /// Replace the vertex weights.
     pub fn set_vertex_weights(&mut self, vertex_weights: Vec<W>) {
-        assert_eq!(
-            vertex_weights.len(),
-            self.graph.num_vertices(),
-            "vertex_weights length must match graph num_vertices"
-        );
+        Self::check_vertex_weights(&self.graph, &vertex_weights)
+            .unwrap_or_else(|error| panic!("{error}"));
         self.vertex_weights = vertex_weights;
     }
 
+    fn check_vertex_weights(
+        graph: &DirectedGraph,
+        vertex_weights: &[W],
+    ) -> Result<(), crate::registry::ConstructionError> {
+        if vertex_weights.len() != graph.num_vertices() {
+            return Err("vertex_weights length must match graph num_vertices".into());
+        }
+        Ok(())
+    }
+
     /// Replace the arc costs.
     pub fn set_arc_costs(&mut self, arc_costs: Vec<W>) {
-        assert_eq!(
-            arc_costs.len(),
-            self.graph.num_arcs(),
-            "arc_costs length must match graph num_arcs"
-        );
+        Self::check_arc_costs(&self.graph, &arc_costs).unwrap_or_else(|error| panic!("{error}"));
         self.arc_costs = arc_costs;
     }
 
+    fn check_arc_costs(
+        graph: &DirectedGraph,
+        arc_costs: &[W],
+    ) -> Result<(), crate::registry::ConstructionError> {
+        if arc_costs.len() != graph.num_arcs() {
+            return Err("arc_costs length must match graph num_arcs".into());
+        }
+        Ok(())
+    }
+
     /// Get the per-part weight bound.
     pub fn weight_bound(&self) -> &W::Sum {
         &self.weight_bound

src/models/graph/bottleneck_traveling_salesman.rs

@@ -24,11 +24,25 @@ inventory::submit! {
 
 /// The Bottleneck Traveling Salesman problem on a simple weighted graph.
 #[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(try_from = "BottleneckTravelingSalesmanData")]
 pub struct BottleneckTravelingSalesman {
     graph: SimpleGraph,
     edge_weights: Vec<i64>,
 }
 
+#[derive(Deserialize)]
+struct BottleneckTravelingSalesmanData {
+    graph: SimpleGraph,
+    edge_weights: Vec<i64>,
+}
+
+impl TryFrom<BottleneckTravelingSalesmanData> for BottleneckTravelingSalesman {
+    type Error = crate::registry::ConstructionError;
+    fn try_from(data: BottleneckTravelingSalesmanData) -> Result<Self, Self::Error> {
+        Self::try_new(data.graph, data.edge_weights)
+    }
+}
+
 #[derive(Debug, Deserialize, crate::CreateSpec)]
 struct BottleneckTravelingSalesmanCreateSpec {
     #[create(codec = "edge-list")]
@@ -46,15 +60,7 @@ impl TryFrom<BottleneckTravelingSalesmanCreateSpec> for BottleneckTravelingSales
         let edge_weights = spec
             .edge_weights
             .unwrap_or_else(|| vec![1; graph.num_edges()]);
-        if edge_weights.len() != graph.num_edges() {
-            return Err(format!(
-                "edge_weights has length {}, expected {}",
-                edge_weights.len(),
-                graph.num_edges()
-            )
-            .into());
-        }
-        Ok(Self::new(graph, edge_weights))
+        Self::try_new(graph, edge_weights)
     }
 }
 
@@ -92,15 +98,18 @@ fn simple_graph_from_create(
 impl BottleneckTravelingSalesman {
     /// Create a BottleneckTravelingSalesman problem from a graph with edge weights.
     pub fn new(graph: SimpleGraph, edge_weights: Vec<i64>) -> Self {
-        assert_eq!(
-            edge_weights.len(),
-            graph.num_edges(),
-            "edge_weights length must match num_edges"
-        );
-        Self {
+        Self::try_new(graph, edge_weights).unwrap_or_else(|error| panic!("{error}"))
+    }
+
+    fn try_new(
+        graph: SimpleGraph,
+        edge_weights: Vec<i64>,
+    ) -> Result<Self, crate::registry::ConstructionError> {
+        Self::check_weights(&graph, &edge_weights)?;
+        Ok(Self {
             graph,
             edge_weights,
-        }
+        })
     }
 
     /// Get a reference to the underlying graph.
@@ -115,9 +124,18 @@ impl BottleneckTravelingSalesman {
 
     /// Set new weights for the problem.
     pub fn set_weights(&mut self, weights: Vec<i64>) {
-        assert_eq!(weights.len(), self.graph.num_edges());
+        Self::check_weights(&self.graph, &weights).unwrap_or_else(|error| panic!("{error}"));
         self.edge_weights = weights;
     }
+    fn check_weights(
+        graph: &SimpleGraph,
+        weights: &[i64],
+    ) -> Result<(), crate::registry::ConstructionError> {
+        if weights.len() != graph.num_edges() {
+            return Err("edge_weights length must match num_edges".into());
+        }
+        Ok(())
+    }
 
     /// Get all edges with their weights.
     pub fn edges(&self) -> Vec<(usize, usize, i64)> {

src/models/graph/kth_best_spanning_tree.rs

@@ -34,14 +34,35 @@ inventory::submit! {
 ///
 /// A configuration is `k` consecutive binary blocks of length `|E|`.
 /// Each block selects the edges of one candidate spanning tree.
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize)]
 pub struct KthBestSpanningTree<W: WeightElement> {
     graph: SimpleGraph,
     weights: Vec<W>,
     k: usize,
     bound: W::Sum,
 }
 
+#[derive(Deserialize)]
+#[serde(bound(deserialize = "W: WeightElement + Deserialize<'de>, W::Sum: Deserialize<'de>"))]
+struct KthBestSpanningTreeData<W: WeightElement> {
+    graph: SimpleGraph,
+    weights: Vec<W>,
+    k: usize,
+    bound: W::Sum,
+}
+
+impl<'de, W> Deserialize<'de> for KthBestSpanningTree<W>
+where
+    W: WeightElement + Deserialize<'de>,
+    W::Sum: Deserialize<'de>,
+{
+    fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        let data = KthBestSpanningTreeData::<W>::deserialize(deserializer)?;
+        Self::try_new(data.graph, data.weights, data.k, data.bound)
+            .map_err(serde::de::Error::custom)
+    }
+}
+
 #[derive(Debug, Deserialize, crate::CreateSpec)]
 struct KthBestSpanningTreeCreateSpec {
     #[create(codec = "edge-list")]
@@ -61,18 +82,7 @@ impl TryFrom<KthBestSpanningTreeCreateSpec> for KthBestSpanningTree<i64> {
         let weights = spec
             .edge_weights
             .unwrap_or_else(|| vec![1; graph.num_edges()]);
-        if weights.len() != graph.num_edges() {
-            return Err(format!(
-                "edge_weights has length {}, expected {}",
-                weights.len(),
-                graph.num_edges()
-            )
-            .into());
-        }
-        if spec.k == 0 {
-            return Err("k must be positive".to_string().into());
-        }
-        Ok(Self::new(graph, weights, spec.k, spec.bound))
+        Self::try_new(graph, weights, spec.k, spec.bound)
     }
 }
 
@@ -112,19 +122,28 @@ impl<W: WeightElement> KthBestSpanningTree<W> {
     /// Panics if the number of weights does not match the number of edges, or
     /// if `k` is zero.
     pub fn new(graph: SimpleGraph, weights: Vec<W>, k: usize, bound: W::Sum) -> Self {
-        assert_eq!(
-            weights.len(),
-            graph.num_edges(),
-            "weights length must match graph num_edges"
-        );
-        assert!(k > 0, "k must be positive");
-
-        Self {
+        Self::try_new(graph, weights, k, bound).unwrap_or_else(|error| panic!("{error}"))
+    }
+
+    fn try_new(
+        graph: SimpleGraph,
+        weights: Vec<W>,
+        k: usize,
+        bound: W::Sum,
+    ) -> Result<Self, crate::registry::ConstructionError> {
+        if weights.len() != graph.num_edges() {
+            return Err("weights length must match graph num_edges".into());
+        }
+        if k == 0 {
+            return Err("k must be positive".into());
+        }
+
+        Ok(Self {
             graph,
             weights,
             k,
             bound,
-        }
+        })
     }
 
     /// Get the underlying graph.