Validate string and scheduling model inputs

problemreductions-cli/src/dispatch.rs

@@ -596,7 +596,7 @@ mod tests {
         );
         let err = loaded.err().unwrap();
         assert!(
-            err.to_string().contains("expected positive integer, got 0"),
+            err.to_string().contains("num_processors must be positive"),
             "unexpected error: {err}"
         );
     }

problemreductions-cli/tests/cli_tests.rs

@@ -7845,7 +7845,7 @@ fn test_evaluate_multiprocessor_scheduling_rejects_zero_processors_json() {
     );
     let stderr = String::from_utf8_lossy(&output.stderr);
     assert!(
-        stderr.contains("expected positive integer, got 0"),
+        stderr.contains("num_processors must be positive"),
         "stderr: {stderr}"
     );
 

src/models/misc/closest_string.rs

@@ -46,11 +46,26 @@ inventory::submit! {
 /// syntactically feasible; the objective is its worst-case Hamming distance
 /// to the input strings.
 #[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(try_from = "ClosestStringData")]
 pub struct ClosestString {
     alphabet_size: usize,
     strings: Vec<Vec<usize>>,
 }
 
+#[derive(Deserialize)]
+struct ClosestStringData {
+    alphabet_size: usize,
+    strings: Vec<Vec<usize>>,
+}
+
+impl TryFrom<ClosestStringData> for ClosestString {
+    type Error = crate::registry::ConstructionError;
+
+    fn try_from(data: ClosestStringData) -> Result<Self, Self::Error> {
+        Self::try_new(data.alphabet_size, data.strings)
+    }
+}
+
 impl ClosestString {
     /// Create a new `ClosestString` instance.
     ///
@@ -62,30 +77,34 @@ impl ClosestString {
     /// - `alphabet_size == 0` while any input string is non-empty,
     /// - any symbol in any input string is `>= alphabet_size`.
     pub fn new(alphabet_size: usize, strings: Vec<Vec<usize>>) -> Self {
-        assert!(
-            !strings.is_empty(),
-            "ClosestString requires at least one input string"
-        );
+        Self::try_new(alphabet_size, strings).unwrap_or_else(|error| panic!("{error}"))
+    }
+
+    fn try_new(
+        alphabet_size: usize,
+        strings: Vec<Vec<usize>>,
+    ) -> Result<Self, crate::registry::ConstructionError> {
+        if strings.is_empty() {
+            return Err("ClosestString requires at least one input string".into());
+        }
         let string_length = strings[0].len();
-        assert!(
-            strings.iter().all(|s| s.len() == string_length),
-            "all input strings must have the same length"
-        );
-        assert!(
-            alphabet_size > 0 || string_length == 0,
-            "alphabet_size must be > 0 when input strings are non-empty"
-        );
-        assert!(
-            strings
-                .iter()
-                .flat_map(|s| s.iter())
-                .all(|&symbol| symbol < alphabet_size),
-            "input symbols must be less than alphabet_size"
-        );
-        Self {
+        if !(strings.iter().all(|s| s.len() == string_length)) {
+            return Err("all input strings must have the same length".into());
+        }
+        if !(alphabet_size > 0 || string_length == 0) {
+            return Err("alphabet_size must be > 0 when input strings are non-empty".into());
+        }
+        if !(strings
+            .iter()
+            .flat_map(|s| s.iter())
+            .all(|&symbol| symbol < alphabet_size))
+        {
+            return Err("input symbols must be less than alphabet_size".into());
+        }
+        Ok(Self {
             alphabet_size,
             strings,
-        }
+        })
     }
 
     /// Returns the alphabet size `q`.

src/models/misc/flow_shop_scheduling.rs

@@ -58,6 +58,7 @@ inventory::submit! {
 /// assert!(solution.is_some());
 /// ```
 #[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(try_from = "FlowShopSchedulingData")]
 pub struct FlowShopScheduling {
     /// Number of processors (machines).
     num_processors: usize,
@@ -67,6 +68,21 @@ pub struct FlowShopScheduling {
     deadline: i64,
 }
 
+#[derive(Deserialize)]
+struct FlowShopSchedulingData {
+    num_processors: usize,
+    task_lengths: Vec<Vec<i64>>,
+    deadline: i64,
+}
+
+impl TryFrom<FlowShopSchedulingData> for FlowShopScheduling {
+    type Error = crate::registry::ConstructionError;
+
+    fn try_from(data: FlowShopSchedulingData) -> Result<Self, Self::Error> {
+        Self::try_new(data.num_processors, data.task_lengths, data.deadline)
+    }
+}
+
 impl FlowShopScheduling {
     /// Create a new Flow Shop Scheduling instance.
     ///
@@ -79,26 +95,37 @@ impl FlowShopScheduling {
     /// # Panics
     /// Panics if any job does not have exactly `num_processors` tasks.
     pub fn new(num_processors: usize, task_lengths: Vec<Vec<i64>>, deadline: i64) -> Self {
+        Self::try_new(num_processors, task_lengths, deadline)
+            .unwrap_or_else(|error| panic!("{error}"))
+    }
+
+    fn try_new(
+        num_processors: usize,
+        task_lengths: Vec<Vec<i64>>,
+        deadline: i64,
+    ) -> Result<Self, crate::registry::ConstructionError> {
         for (j, tasks) in task_lengths.iter().enumerate() {
-            assert_eq!(
-                tasks.len(),
-                num_processors,
-                "Job {} has {} tasks, expected {}",
-                j,
-                tasks.len(),
-                num_processors
-            );
+            if tasks.len() != num_processors {
+                return Err(format!(
+                    "Job {} has {} tasks, expected {}",
+                    j,
+                    tasks.len(),
+                    num_processors
+                )
+                .into());
+            }
         }
-        assert!(
-            task_lengths.iter().flatten().all(|&length| length >= 0),
-            "task lengths must be nonnegative"
-        );
-        assert!(deadline >= 0, "deadline must be nonnegative");
-        Self {
+        if task_lengths.iter().flatten().any(|&length| length < 0) {
+            return Err("task lengths must be nonnegative".into());
+        }
+        if deadline < 0 {
+            return Err("deadline must be nonnegative".into());
+        }
+        Ok(Self {
             num_processors,
             task_lengths,
             deadline,
-        }
+        })
     }
 
     /// Get the number of processors.

src/models/misc/grouping_by_swapping.rs

@@ -27,12 +27,28 @@ inventory::submit! {
 /// adjacent swap position `i` (swap positions `i` and `i + 1`) or the special
 /// no-op value `string_len - 1`.
 #[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(try_from = "GroupingBySwappingData")]
 pub struct GroupingBySwapping {
     alphabet_size: usize,
     string: Vec<usize>,
     budget: usize,
 }
 
+#[derive(Deserialize)]
+struct GroupingBySwappingData {
+    alphabet_size: usize,
+    string: Vec<usize>,
+    budget: usize,
+}
+
+impl TryFrom<GroupingBySwappingData> for GroupingBySwapping {
+    type Error = crate::registry::ConstructionError;
+
+    fn try_from(data: GroupingBySwappingData) -> Result<Self, Self::Error> {
+        Self::try_new(data.alphabet_size, data.string, data.budget)
+    }
+}
+
 #[derive(Debug, Deserialize, crate::CreateSpec)]
 struct GroupingBySwappingCreateSpec {
     /// Optional alphabet size; omitted values are inferred from the string.
@@ -61,27 +77,7 @@ impl TryFrom<GroupingBySwappingCreateSpec> for GroupingBySwapping {
             .transpose()?
             .unwrap_or(0);
         let alphabet_size = spec.alphabet_size.unwrap_or(inferred_alphabet_size);
-        if alphabet_size < inferred_alphabet_size {
-            return Err(format!(
-                "alphabet size {alphabet_size} is smaller than inferred alphabet size {inferred_alphabet_size}"
-            ).into());
-        }
-        if alphabet_size == 0 && !spec.string.is_empty() {
-            return Err("alphabet size must be positive for a non-empty string"
-                .to_string()
-                .into());
-        }
-        if spec.string.is_empty() && spec.bound != 0 {
-            return Err("bound must be zero when the string is empty"
-                .to_string()
-                .into());
-        }
-
-        Ok(Self {
-            alphabet_size,
-            string: spec.string,
-            budget: spec.bound,
-        })
+        Self::try_new(alphabet_size, spec.string, spec.bound)
     }
 }
 
@@ -93,23 +89,28 @@ impl GroupingBySwapping {
     /// Panics if the string contains a symbol outside the declared alphabet,
     /// or if the string is empty while the budget is positive.
     pub fn new(alphabet_size: usize, string: Vec<usize>, budget: usize) -> Self {
-        assert!(
-            alphabet_size > 0 || string.is_empty(),
-            "alphabet_size must be > 0 when string is non-empty"
-        );
-        assert!(
-            string.iter().all(|&symbol| symbol < alphabet_size),
-            "input symbols must be less than alphabet_size"
-        );
-        assert!(
-            !string.is_empty() || budget == 0,
-            "budget must be 0 when string is empty"
-        );
-        Self {
+        Self::try_new(alphabet_size, string, budget).unwrap_or_else(|error| panic!("{error}"))
+    }
+
+    fn try_new(
+        alphabet_size: usize,
+        string: Vec<usize>,
+        budget: usize,
+    ) -> Result<Self, crate::registry::ConstructionError> {
+        if !(alphabet_size > 0 || string.is_empty()) {
+            return Err("alphabet_size must be > 0 when string is non-empty".into());
+        }
+        if !(string.iter().all(|&symbol| symbol < alphabet_size)) {
+            return Err("input symbols must be less than alphabet_size".into());
+        }
+        if string.is_empty() && budget != 0 {
+            return Err("budget must be 0 when string is empty".into());
+        }
+        Ok(Self {
             alphabet_size,
             string,
             budget,
-        }
+        })
     }
 
     /// Returns the alphabet size.