From 4225d61027d9453e6ab9213508d34609f702b8e6 Mon Sep 17 00:00:00 2001 From: Manuel Raimann Date: Thu, 30 Apr 2026 09:27:06 +0200 Subject: [PATCH] fix(sampler): deterministic parameter matching in TPE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When two parameters share the same Distribution (e.g. two FloatParam::new(-5.0, 5.0) in one study), TpeSampler iterated trial.distributions (a HashMap) with find_map to extract a value matching the target distribution. HashMap iteration order varies across runs, so x's history could be conflated with y's history non-deterministically — making seeded TPE runs flaky in parallel test execution. Pick the smallest-ParamId match instead, so the choice is stable across runs. Also: - src/sampler/genetic.rs: collapse if into match guard (clippy) - src/lib.rs: drop std_instead_of_core lint (the only remaining hits are std::io::Error/ErrorKind, which have no stable core equivalent yet) - Cargo.toml: relax minimum versions for fastrand/tokio/tracing/ optimizer-derive; bump dev-dep tokio to 1.50 to match in-tree usage --- Cargo.toml | 10 ++-- src/lib.rs | 1 - src/sampler/genetic.rs | 8 +-- src/sampler/tpe/sampler.rs | 100 +++++++++++++------------------------ 4 files changed, 43 insertions(+), 76 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8ca9a69..b5b005d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,14 +16,14 @@ categories = ["algorithms", "science", "mathematics"] readme = "README.md" [dependencies] -fastrand = "2.3" +fastrand = "2" thiserror = "2" parking_lot = "0.12" -tokio = { version = "1.30", features = ["sync", "rt-multi-thread"], optional = true } -optimizer-derive = { version = "0.1.0", path = "optimizer-derive", optional = true } +tokio = { version = "1", features = ["sync", "rt-multi-thread"], optional = true } +optimizer-derive = { version = "0.1", path = "optimizer-derive", optional = true } serde = { version = "1", features = ["derive"], optional = true } serde_json = { version = "1", optional = true } -tracing = { version = "0.1.29", optional = true } +tracing = { version = "0.1", optional = true } sobol_burley = { version = "0.5", optional = true } nalgebra = { version = "0.34", optional = true } fs2 = { version = "0.4", optional = true } @@ -40,7 +40,7 @@ cma-es = ["dep:nalgebra"] gp = ["dep:nalgebra"] [dev-dependencies] -tokio = { version = "1.30", features = ["rt-multi-thread", "macros", "time"] } +tokio = { version = "1.50", features = ["rt-multi-thread", "macros", "time"] } optimizer-derive = { version = "0.1.0", path = "optimizer-derive" } serde_json = "1" criterion = { version = "0.8", features = ["html_reports"] } diff --git a/src/lib.rs b/src/lib.rs index 9ce87ad..aeb01d6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,7 +7,6 @@ #![deny(clippy::complexity)] #![deny(clippy::perf)] #![deny(clippy::pedantic)] -#![deny(clippy::std_instead_of_core)] //! Bayesian and population-based optimization library with an Optuna-like API //! for hyperparameter tuning and black-box optimization. It ships 12 samplers diff --git a/src/sampler/genetic.rs b/src/sampler/genetic.rs index d6d6fa2..6fbb9e0 100644 --- a/src/sampler/genetic.rs +++ b/src/sampler/genetic.rs @@ -295,10 +295,10 @@ pub(crate) fn crossover( child2[i] = ParamValue::Int((c2.round() as i64).clamp(d.low, d.high)); } } - (ParamValue::Categorical(_), ParamValue::Categorical(_), _) => { - if rng_util::f64_range(rng, 0.0, 1.0) < 0.5 { - core::mem::swap(&mut child1[i], &mut child2[i]); - } + (ParamValue::Categorical(_), ParamValue::Categorical(_), _) + if rng_util::f64_range(rng, 0.0, 1.0) < 0.5 => + { + core::mem::swap(&mut child1[i], &mut child2[i]); } _ => {} } diff --git a/src/sampler/tpe/sampler.rs b/src/sampler/tpe/sampler.rs index b4c1c69..94dd313 100644 --- a/src/sampler/tpe/sampler.rs +++ b/src/sampler/tpe/sampler.rs @@ -642,6 +642,22 @@ impl Default for TpeSamplerBuilder { } } +/// Deterministically pick a parameter value matching `target_dist` from a +/// trial. When multiple parameters share the same distribution (e.g. two +/// `FloatParam::new(-5.0, 5.0)` in the same study), the one with the +/// smallest [`ParamId`] is chosen so behavior does not depend on +/// `HashMap` iteration order. +fn find_matching_value<'t>( + t: &'t CompletedTrial, + target_dist: &Distribution, +) -> Option<&'t ParamValue> { + t.distributions + .iter() + .filter(|(_, dist)| *dist == target_dist) + .min_by_key(|(id, _)| *id) + .and_then(|(id, _)| t.params.get(id)) +} + impl TpeSampler { fn sample_float( &self, @@ -653,33 +669,17 @@ impl TpeSampler { let target_dist = Distribution::Float(d.clone()); let good_values: Vec = good_trials .iter() - .filter_map(|t| { - t.distributions.iter().find_map(|(id, dist)| { - if *dist == target_dist { - t.params.get(id).and_then(|v| match v { - ParamValue::Float(f) => Some(*f), - _ => None, - }) - } else { - None - } - }) + .filter_map(|t| match find_matching_value(t, &target_dist)? { + ParamValue::Float(f) => Some(*f), + _ => None, }) .collect(); let bad_values: Vec = bad_trials .iter() - .filter_map(|t| { - t.distributions.iter().find_map(|(id, dist)| { - if *dist == target_dist { - t.params.get(id).and_then(|v| match v { - ParamValue::Float(f) => Some(*f), - _ => None, - }) - } else { - None - } - }) + .filter_map(|t| match find_matching_value(t, &target_dist)? { + ParamValue::Float(f) => Some(*f), + _ => None, }) .collect(); @@ -708,33 +708,17 @@ impl TpeSampler { let target_dist = Distribution::Int(d.clone()); let good_values: Vec = good_trials .iter() - .filter_map(|t| { - t.distributions.iter().find_map(|(id, dist)| { - if *dist == target_dist { - t.params.get(id).and_then(|v| match v { - ParamValue::Int(i) => Some(*i), - _ => None, - }) - } else { - None - } - }) + .filter_map(|t| match find_matching_value(t, &target_dist)? { + ParamValue::Int(i) => Some(*i), + _ => None, }) .collect(); let bad_values: Vec = bad_trials .iter() - .filter_map(|t| { - t.distributions.iter().find_map(|(id, dist)| { - if *dist == target_dist { - t.params.get(id).and_then(|v| match v { - ParamValue::Int(i) => Some(*i), - _ => None, - }) - } else { - None - } - }) + .filter_map(|t| match find_matching_value(t, &target_dist)? { + ParamValue::Int(i) => Some(*i), + _ => None, }) .collect(); @@ -764,33 +748,17 @@ impl TpeSampler { let target_dist = Distribution::Categorical(d.clone()); let good_indices: Vec = good_trials .iter() - .filter_map(|t| { - t.distributions.iter().find_map(|(id, dist)| { - if *dist == target_dist { - t.params.get(id).and_then(|v| match v { - ParamValue::Categorical(i) => Some(*i), - _ => None, - }) - } else { - None - } - }) + .filter_map(|t| match find_matching_value(t, &target_dist)? { + ParamValue::Categorical(i) => Some(*i), + _ => None, }) .collect(); let bad_indices: Vec = bad_trials .iter() - .filter_map(|t| { - t.distributions.iter().find_map(|(id, dist)| { - if *dist == target_dist { - t.params.get(id).and_then(|v| match v { - ParamValue::Categorical(i) => Some(*i), - _ => None, - }) - } else { - None - } - }) + .filter_map(|t| match find_matching_value(t, &target_dist)? { + ParamValue::Categorical(i) => Some(*i), + _ => None, }) .collect();