diff --git a/vrp-cli/src/extensions/analyze/clusters.rs b/vrp-cli/src/extensions/analyze/clusters.rs index 652896b6c..89c6164f8 100644 --- a/vrp-cli/src/extensions/analyze/clusters.rs +++ b/vrp-cli/src/extensions/analyze/clusters.rs @@ -7,8 +7,7 @@ use std::sync::Arc; use vrp_core::construction::clustering::dbscan::create_job_clusters; use vrp_core::models::problem::{get_job_locations, JobIdDimension}; use vrp_core::models::Problem; -use vrp_core::prelude::{Float, GenericError}; -use vrp_core::utils::Environment; +use vrp_core::prelude::{Float, GenericResult}; use vrp_pragmatic::format::problem::{deserialize_matrix, deserialize_problem, PragmaticProblem}; use vrp_pragmatic::format::solution::serialize_named_locations_as_geojson; use vrp_pragmatic::format::{CoordIndexExtraProperty, MultiFormatError}; @@ -19,14 +18,13 @@ pub fn get_clusters( matrices_readers: Option>>, min_points: Option, epsilon: Option, -) -> Result { +) -> GenericResult { let problem = Arc::new(get_core_problem(problem_reader, matrices_readers).map_err(|errs| errs.to_string())?); let coord_index = problem.extras.get_coord_index().expect("cannot find coord index"); let coord_index = coord_index.as_ref(); - let environment = Arc::new(Environment::default()); - let clusters = create_job_clusters(problem.as_ref(), environment.random.as_ref(), min_points, epsilon); + let clusters = create_job_clusters(problem.as_ref(), min_points, epsilon)?; let locations = clusters .iter() diff --git a/vrp-cli/src/extensions/solve/config.rs b/vrp-cli/src/extensions/solve/config.rs index ed20fa801..7295f9e4d 100644 --- a/vrp-cli/src/extensions/solve/config.rs +++ b/vrp-cli/src/extensions/solve/config.rs @@ -686,7 +686,8 @@ fn create_ruin_method( (Arc::new(WorstJobRemoval::new(*worst_skip, get_limits(*min, *max))), *probability) } RuinMethod::Cluster { probability, min, max, min_items } => ( - Arc::new(ClusterRemoval::new(problem.clone(), environment, *min_items, get_limits(*min, *max))), + // TODO: remove unwrap + Arc::new(ClusterRemoval::new(problem.clone(), environment, *min_items, get_limits(*min, *max)).unwrap()), *probability, ), RuinMethod::CloseRoute { probability } => (Arc::new(CloseRouteRemoval::new(limits)), *probability), diff --git a/vrp-core/src/construction/clustering/dbscan/mod.rs b/vrp-core/src/construction/clustering/dbscan/mod.rs index f107c689a..8b95141bf 100644 --- a/vrp-core/src/construction/clustering/dbscan/mod.rs +++ b/vrp-core/src/construction/clustering/dbscan/mod.rs @@ -17,15 +17,14 @@ use std::sync::Arc; /// Creates clusters of jobs using DBSCAN algorithm. pub fn create_job_clusters( problem: &Problem, - random: &(dyn Random), min_points: Option, epsilon: Option, -) -> Vec> { +) -> GenericResult>> { let min_points = min_points.unwrap_or(3).max(2); let epsilon = epsilon.unwrap_or_else(|| estimate_epsilon(problem, min_points)); // get main parameters with some randomization - let profile = &problem.fleet.profiles[random.uniform_int(0, problem.fleet.profiles.len() as i32 - 1) as usize]; + let profile = problem.fleet.profiles.first().ok_or_else(|| GenericError::from("cannot find any profile"))?; // exclude jobs without locations from clustering let jobs = problem.jobs.all().iter().filter(|j| job_has_locations(j)).cloned().collect::>(); @@ -38,10 +37,10 @@ pub fn create_job_clusters( .map(|(job, _)| job) }; - create_clusters(jobs.as_slice(), min_points, neighbor_fn) + Ok(create_clusters(jobs.as_slice(), min_points, neighbor_fn) .into_iter() .map(|cluster| cluster.into_iter().cloned().collect::>()) - .collect() + .collect()) } /// Estimates DBSCAN epsilon parameter. diff --git a/vrp-core/src/solver/heuristic.rs b/vrp-core/src/solver/heuristic.rs index ba111bbe1..1a1819ff1 100644 --- a/vrp-core/src/solver/heuristic.rs +++ b/vrp-core/src/solver/heuristic.rs @@ -428,7 +428,8 @@ mod statik { (vec![(Arc::new(WorstJobRemoval::new(4, normal_limits)), 1.), (extra_random_job.clone(), 0.1)], 10), ( vec![ - (Arc::new(ClusterRemoval::new_with_defaults(problem.clone(), environment.clone())), 1.), + // TODO avoid unwrap + (Arc::new(ClusterRemoval::new_with_defaults(problem.clone(), environment.clone()).unwrap()), 1.), (extra_random_job.clone(), 0.1), ], 5, @@ -512,7 +513,8 @@ mod dynamic { fn get_ruins(problem: Arc, environment: Arc) -> Vec<(Arc, String, Float)> { // NOTE: creating cluster removal is not fast on large problems vec![( - Arc::new(ClusterRemoval::new_with_defaults(problem.clone(), environment)), + // TODO avoid unwrap + Arc::new(ClusterRemoval::new_with_defaults(problem.clone(), environment).unwrap()), "cluster_removal".to_string(), 4., )] diff --git a/vrp-core/src/solver/search/ruin/cluster_removal.rs b/vrp-core/src/solver/search/ruin/cluster_removal.rs index a8aed3f88..c2a30c2c1 100644 --- a/vrp-core/src/solver/search/ruin/cluster_removal.rs +++ b/vrp-core/src/solver/search/ruin/cluster_removal.rs @@ -20,18 +20,23 @@ pub struct ClusterRemoval { impl ClusterRemoval { /// Creates a new instance of `ClusterRemoval`. - pub fn new(problem: Arc, environment: Arc, min_items: usize, limits: RemovalLimits) -> Self { - let clusters = create_job_clusters(problem.as_ref(), environment.random.as_ref(), Some(min_items), None); + pub fn new( + problem: Arc, + environment: Arc, + min_items: usize, + limits: RemovalLimits, + ) -> GenericResult { + let clusters = create_job_clusters(problem.as_ref(), Some(min_items), None)?; let mut clusters = clusters.into_iter().map(|cluster| cluster.into_iter().collect::>()).collect::>(); clusters.shuffle(&mut environment.random.get_rng()); - Self { clusters, limits } + Ok(Self { clusters, limits }) } /// Creates a new instance of `ClusterRemoval` with default parameters. - pub fn new_with_defaults(problem: Arc, environment: Arc) -> Self { + pub fn new_with_defaults(problem: Arc, environment: Arc) -> GenericResult { let limits = RemovalLimits::new(problem.as_ref()); Self::new(problem, environment, 3, limits) } diff --git a/vrp-core/tests/unit/construction/clustering/dbscan_test.rs b/vrp-core/tests/unit/construction/clustering/dbscan_test.rs index fa524706c..24690d0d7 100644 --- a/vrp-core/tests/unit/construction/clustering/dbscan_test.rs +++ b/vrp-core/tests/unit/construction/clustering/dbscan_test.rs @@ -4,7 +4,6 @@ use crate::helpers::construction::clustering::p; use crate::helpers::models::domain::TestGoalContextBuilder; use crate::helpers::models::problem::TestSingleBuilder; use crate::helpers::solver::{generate_matrix_distances_from_points, generate_matrix_routes}; -use crate::helpers::utils::random::FakeRandom; use crate::models::common::Location; use crate::models::{Extras, GoalContext}; use crate::prelude::{ActivityCost, TransportCost}; @@ -115,9 +114,9 @@ fn can_create_job_clusters_impl(param: (usize, Float), expected: &[Vec |v| v, |_| (vec![0.; 64], create_test_distances()), ); - let random: Arc = Arc::new(FakeRandom::new(vec![0, 0], vec![epsilon])); - let clusters = create_job_clusters(&problem, random.as_ref(), Some(min_points), Some(epsilon)) + let clusters = create_job_clusters(&problem, Some(min_points), Some(epsilon)) + .expect("cannot create job clusters") .iter() .map(|cluster| { let mut cluster = diff --git a/vrp-core/tests/unit/solver/search/ruin/cluster_removal_test.rs b/vrp-core/tests/unit/solver/search/ruin/cluster_removal_test.rs index 217bfe050..4d006ae0f 100644 --- a/vrp-core/tests/unit/solver/search/ruin/cluster_removal_test.rs +++ b/vrp-core/tests/unit/solver/search/ruin/cluster_removal_test.rs @@ -19,7 +19,7 @@ fn can_create_ruin_cluster_with_default_params() { |_| (vec![0.; 64], create_test_distances()), ); - let removal = ClusterRemoval::new_with_defaults(Arc::new(problem), environment); + let removal = ClusterRemoval::new_with_defaults(Arc::new(problem), environment).unwrap(); assert!(!removal.clusters.is_empty()); } @@ -29,7 +29,7 @@ fn can_handle_empty_problem() { let problem = Arc::new(ProblemBuilder::default().build()); let limits = RemovalLimits::new(&problem); - let removal = ClusterRemoval::new(problem, Arc::new(Environment::default()), 3, limits); + let removal = ClusterRemoval::new(problem, Arc::new(Environment::default()), 3, limits).unwrap(); assert!(removal.clusters.is_empty()); } @@ -60,6 +60,7 @@ fn can_ruin_jobs_impl(limit: usize, min_items: usize, expected: usize) { let insertion_ctx = InsertionContext::new_from_solution(problem.clone(), (solution, None), environment.clone()); let insertion_ctx = ClusterRemoval::new(problem, environment, min_items, limits) + .expect("cannot create clusters") .run(&create_default_refinement_ctx(insertion_ctx.problem.clone()), insertion_ctx); assert_eq!(insertion_ctx.solution.unassigned.len(), 0);