From c561caaa342ee236ae3b790c19a3e158b1d2dfd7 Mon Sep 17 00:00:00 2001 From: Manuel Raimann Date: Thu, 12 Feb 2026 14:02:24 +0100 Subject: [PATCH] fix: validate NaN/Inf in deserialized trials during journal loading - Add `CompletedTrial::validate()` checking all f64 fields are finite - Call validate after deserializing each trial in journal storage - Make `distribution` and `param` modules public (types already in public fields) --- src/lib.rs | 4 +- src/sampler/mod.rs | 68 +++++++++++++++++++++++++++ src/storage/journal.rs | 1 + tests/journal_tests.rs | 103 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 174 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 808012d..a7c507e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -107,14 +107,14 @@ macro_rules! trace_debug { ($($arg:tt)*) => {}; } -mod distribution; +pub mod distribution; mod error; mod fanova; mod importance; mod kde; pub mod multi_objective; pub mod objective; -mod param; +pub mod param; pub mod parameter; pub mod pareto; pub mod pruner; diff --git a/src/sampler/mod.rs b/src/sampler/mod.rs index bea4200..4e20f4e 100644 --- a/src/sampler/mod.rs +++ b/src/sampler/mod.rs @@ -175,6 +175,74 @@ impl CompletedTrial { pub fn user_attrs(&self) -> &HashMap { &self.user_attrs } + + /// Validates that all floating-point fields are finite (not NaN or + /// Infinity). + /// + /// Checks distribution bounds, parameter values, constraints, and + /// intermediate values. Returns a description of the first invalid + /// field found, or `Ok(())` if everything is valid. + /// + /// # Errors + /// + /// Returns a `String` describing the first non-finite value found. + pub fn validate(&self) -> core::result::Result<(), String> { + for (id, dist) in &self.distributions { + if let Distribution::Float(fd) = dist { + if !fd.low.is_finite() { + return Err(format!( + "trial {}: float distribution for param {id} has non-finite low bound ({})", + self.id, fd.low + )); + } + if !fd.high.is_finite() { + return Err(format!( + "trial {}: float distribution for param {id} has non-finite high bound ({})", + self.id, fd.high + )); + } + if let Some(step) = fd.step + && !step.is_finite() + { + return Err(format!( + "trial {}: float distribution for param {id} has non-finite step ({step})", + self.id + )); + } + } + } + + for (id, pv) in &self.params { + if let ParamValue::Float(v) = pv + && !v.is_finite() + { + return Err(format!( + "trial {}: param {id} has non-finite float value ({v})", + self.id + )); + } + } + + for (i, &c) in self.constraints.iter().enumerate() { + if !c.is_finite() { + return Err(format!( + "trial {}: constraint[{i}] is non-finite ({c})", + self.id + )); + } + } + + for &(step, v) in &self.intermediate_values { + if !v.is_finite() { + return Err(format!( + "trial {}: intermediate value at step {step} is non-finite ({v})", + self.id + )); + } + } + + Ok(()) + } } /// A pending (running) trial with its parameters and distributions, but no objective value yet. diff --git a/src/storage/journal.rs b/src/storage/journal.rs index 1ee477b..55ad087 100644 --- a/src/storage/journal.rs +++ b/src/storage/journal.rs @@ -244,6 +244,7 @@ fn load_trials_from_file( } let trial: CompletedTrial = serde_json::from_str(line).map_err(|e| crate::Error::Storage(e.to_string()))?; + trial.validate().map_err(crate::Error::Storage)?; trials.push(trial); } diff --git a/tests/journal_tests.rs b/tests/journal_tests.rs index 4eef94e..6d5f473 100644 --- a/tests/journal_tests.rs +++ b/tests/journal_tests.rs @@ -214,3 +214,106 @@ fn pruned_trials_are_stored() { std::fs::remove_file(&path).ok(); } + +#[test] +fn rejects_non_finite_values_in_journal() { + // serde_json rejects 1e999 ("number out of range"), so non-finite + // floats cannot sneak in through standard JSON. Verify the overall + // loading path catches the error regardless of which layer rejects it. + let path = temp_path(); + std::fs::write( + &path, + r#"{"id":0,"params":{},"distributions":{"0":{"Float":{"low":0.0,"high":1e999,"log_scale":false,"step":null}}},"param_labels":{},"value":1.0,"intermediate_values":[],"state":"Complete","user_attrs":{},"constraints":[]}"#, + ) + .unwrap(); + + assert!(JournalStorage::::open(&path).is_err()); + std::fs::remove_file(&path).ok(); +} + +#[test] +fn validate_rejects_non_finite_distribution_bound() { + use optimizer::distribution::{Distribution, FloatDistribution}; + + let pid = FloatParam::new(0.0, 1.0).id(); + let mut trial = sample_trial(0, 1.0); + trial.distributions.insert( + pid, + Distribution::Float(FloatDistribution { + low: 0.0, + high: f64::INFINITY, + log_scale: false, + step: None, + }), + ); + let err = trial.validate().unwrap_err(); + assert!(err.contains("non-finite"), "unexpected: {err}"); +} + +#[test] +fn validate_rejects_nan_constraint() { + let mut trial = sample_trial(0, 1.0); + trial.constraints.push(f64::NAN); + let err = trial.validate().unwrap_err(); + assert!(err.contains("non-finite"), "unexpected: {err}"); +} + +#[test] +fn validate_rejects_non_finite_param_value() { + use optimizer::param::ParamValue; + + let pid = FloatParam::new(0.0, 1.0).id(); + let mut trial = sample_trial(0, 1.0); + trial + .params + .insert(pid, ParamValue::Float(f64::NEG_INFINITY)); + let err = trial.validate().unwrap_err(); + assert!(err.contains("non-finite"), "unexpected: {err}"); +} + +#[test] +fn validate_rejects_nan_intermediate_value() { + let mut trial = sample_trial(0, 1.0); + trial.intermediate_values.push((0, f64::NAN)); + let err = trial.validate().unwrap_err(); + assert!(err.contains("non-finite"), "unexpected: {err}"); +} + +#[test] +fn validate_accepts_valid_trial() { + use optimizer::distribution::{Distribution, FloatDistribution}; + use optimizer::param::ParamValue; + + let pid = FloatParam::new(0.0, 1.0).id(); + let mut trial = sample_trial(0, 1.0); + trial.params.insert(pid, ParamValue::Float(0.5)); + trial.distributions.insert( + pid, + Distribution::Float(FloatDistribution { + low: 0.0, + high: 1.0, + log_scale: false, + step: None, + }), + ); + trial.constraints.push(-1.0); + trial.intermediate_values.push((0, 0.5)); + assert!(trial.validate().is_ok()); +} + +#[test] +fn accepts_valid_journal_with_distributions() { + let path = temp_path(); + std::fs::write( + &path, + r#"{"id":0,"params":{"0":{"Float":0.5}},"distributions":{"0":{"Float":{"low":0.0,"high":1.0,"log_scale":false,"step":null}}},"param_labels":{},"value":0.25,"intermediate_values":[],"state":"Complete","user_attrs":{},"constraints":[-1.0]}"#, + ) + .unwrap(); + + let storage = JournalStorage::::open(&path).unwrap(); + let loaded = storage.trials_arc().read().clone(); + assert_eq!(loaded.len(), 1); + assert_eq!(loaded[0].value, 0.25); + + std::fs::remove_file(&path).ok(); +}