Refactor dbscan api

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<F: Read>(
     matrices_readers: Option<Vec<BufReader<F>>>,
     min_points: Option<usize>,
     epsilon: Option<Float>,
-) -> Result<String, GenericError> {
+) -> GenericResult<String> {
     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()

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),

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<usize>,
     epsilon: Option<Float>,
-) -> Vec<HashSet<Job>> {
+) -> GenericResult<Vec<HashSet<Job>>> {
     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::<Vec<_>>();
 
@@ -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::<HashSet<_>>())
-        .collect()
+        .collect())
 }
 
 /// Estimates DBSCAN epsilon parameter.

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<Problem>, environment: Arc<Environment>) -> Vec<(Arc<dyn Ruin>, 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.,
         )]

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<Problem>, environment: Arc<Environment>, 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<Problem>,
+        environment: Arc<Environment>,
+        min_items: usize,
+        limits: RemovalLimits,
+    ) -> GenericResult<Self> {
+        let clusters = create_job_clusters(problem.as_ref(), Some(min_items), None)?;
         let mut clusters =
             clusters.into_iter().map(|cluster| cluster.into_iter().collect::<Vec<_>>()).collect::<Vec<_>>();
 
         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<Problem>, environment: Arc<Environment>) -> Self {
+    pub fn new_with_defaults(problem: Arc<Problem>, environment: Arc<Environment>) -> GenericResult<Self> {
         let limits = RemovalLimits::new(problem.as_ref());
         Self::new(problem, environment, 3, limits)
     }

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<Location>
         |v| v,
         |_| (vec![0.; 64], create_test_distances()),
     );
-    let random: Arc<dyn Random> = 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 =

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);