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)
This commit is contained in:
Manuel Raimann
2026-02-12 14:02:24 +01:00
parent 04d822479e
commit c561caaa34
4 changed files with 174 additions and 2 deletions
+2 -2
View File
@@ -107,14 +107,14 @@ macro_rules! trace_debug {
($($arg:tt)*) => {}; ($($arg:tt)*) => {};
} }
mod distribution; pub mod distribution;
mod error; mod error;
mod fanova; mod fanova;
mod importance; mod importance;
mod kde; mod kde;
pub mod multi_objective; pub mod multi_objective;
pub mod objective; pub mod objective;
mod param; pub mod param;
pub mod parameter; pub mod parameter;
pub mod pareto; pub mod pareto;
pub mod pruner; pub mod pruner;
+68
View File
@@ -175,6 +175,74 @@ impl<V> CompletedTrial<V> {
pub fn user_attrs(&self) -> &HashMap<String, AttrValue> { pub fn user_attrs(&self) -> &HashMap<String, AttrValue> {
&self.user_attrs &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. /// A pending (running) trial with its parameters and distributions, but no objective value yet.
+1
View File
@@ -244,6 +244,7 @@ fn load_trials_from_file<V: DeserializeOwned>(
} }
let trial: CompletedTrial<V> = let trial: CompletedTrial<V> =
serde_json::from_str(line).map_err(|e| crate::Error::Storage(e.to_string()))?; serde_json::from_str(line).map_err(|e| crate::Error::Storage(e.to_string()))?;
trial.validate().map_err(crate::Error::Storage)?;
trials.push(trial); trials.push(trial);
} }
+103
View File
@@ -214,3 +214,106 @@ fn pruned_trials_are_stored() {
std::fs::remove_file(&path).ok(); 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::<f64>::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::<f64>::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();
}