Add place index in activity place

CHANGELOG.md

@@ -5,13 +5,19 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Added
+
+* original job place index in activity place to simplify activity-job place matching
+
 ### Changed
 
 * apply code style refactoring
 
 ### Fixed
 
 * double reload assignment when initial solution is used (#126)
+* unexpected total_order behavior in dynamic heuristic (#128)
+* improve validation rule for break with time offset  (#129)
 
 
 ## [v1.22.1]- 2023-08-26

docs/src/internals/development/code-style.md

@@ -51,18 +51,22 @@ println!("{}", (1..11).fold(0, |a, b| a + b));
 Advantages (personal taste):
 - code is easier to follow (fewer jumps here and there over code base)
 - call stack is less nested, so debug is easier
+- benefit of not making it possible to call the function from other places
 
 However, this is not hard rule. In some cases, you might prefer to split:
 - multiple usages
 - separate function provides a good abstraction over complex logic
-- you want to test it separately
+- make sense to test it separately
 - ..
 
 In general, don’t be over-eager to abstract, or offended by a few lines of duplication. Premature abstraction often
 ends up coupling code that should not have to evolve together.
 
 Please note, that this is not about proposing a single 1000-lines god function.
 
+Additional resources:
+- http://number-none.com/blow/blog/programming/2014/09/26/carmack-on-inlined-code.html
+
 
 ## Code organization level
 

vrp-core/src/construction/heuristics/evaluators.rs

@@ -280,10 +280,11 @@ fn analyze_insertion_in_route_leg(
     };
     let start_time = route_ctx.route().tour.start().unwrap().schedule.departure;
     // analyze service details
-    single.places.iter().try_fold(init, |acc, detail| {
+    single.places.iter().enumerate().try_fold(init, |acc, (idx, detail)| {
         // analyze detail time windows
         detail.times.iter().try_fold(acc, |acc, time| {
             target.place = Place {
+                idx,
                 location: detail.location.unwrap_or(prev.place.location),
                 duration: detail.duration,
                 time: time.to_time_window(start_time),

vrp-core/src/construction/heuristics/factories.rs

@@ -32,16 +32,19 @@ pub fn create_insertion_context(problem: Arc<Problem>, environment: Arc<Environm
 
                 let create_activity = |single: Arc<Single>, previous_location: usize| {
                     assert_eq!(single.places.len(), 1);
-                    assert_eq!(single.places.first().unwrap().times.len(), 1);
 
-                    let place = single.places.first().unwrap();
+                    let place_idx = 0;
+                    let place = &single.places[place_idx];
+                    assert_eq!(place.times.len(), 1);
+
                     let time = single.places.first().unwrap().times.first().unwrap();
                     let time = time
                         .as_time_window()
                         .unwrap_or_else(|| panic!("Job with no time window is not supported in locks"));
 
                     Activity {
                         place: ActivityPlace {
+                            idx: place_idx,
                             location: place.location.unwrap_or(previous_location),
                             duration: place.duration,
                             time,

vrp-core/src/models/solution/route.rs

@@ -33,6 +33,9 @@ pub struct CommuteInfo {
 /// Specifies activity place.
 #[derive(Clone, Debug)]
 pub struct Place {
+    /// An index of the place in original job definition.
+    pub idx: usize,
+
     /// Location where activity is performed.
     pub location: Location,
 
@@ -86,7 +89,7 @@ impl Activity {
     /// Creates an activity with a job.
     pub fn new_with_job(job: Arc<Single>) -> Self {
         Activity {
-            place: Place { location: 0, duration: 0.0, time: TimeWindow { start: 0.0, end: f64::MAX } },
+            place: Place { idx: 0, location: 0, duration: 0.0, time: TimeWindow { start: 0.0, end: f64::MAX } },
             schedule: Schedule { arrival: 0.0, departure: 0.0 },
             job: Some(job),
             commute: None,
@@ -97,6 +100,7 @@ impl Activity {
     pub fn deep_copy(&self) -> Self {
         Self {
             place: Place {
+                idx: self.place.idx,
                 location: self.place.location,
                 duration: self.place.duration,
                 time: self.place.time.clone(),

vrp-core/src/models/solution/tour.rs

@@ -236,7 +236,7 @@ fn create_start_activity(actor: &Actor) -> Activity {
 
     Activity {
         schedule: Schedule { arrival: time.start, departure: time.start },
-        place: Place { location: start.location, duration: 0.0, time },
+        place: Place { idx: 0, location: start.location, duration: 0., time },
         job: None,
         commute: None,
     }
@@ -248,7 +248,7 @@ fn create_end_activity(actor: &Actor) -> Option<Activity> {
         let time = place.time.to_time_window();
         Activity {
             schedule: Schedule { arrival: time.start, departure: time.start },
-            place: Place { location: place.location, duration: 0.0, time },
+            place: Place { idx: 0, location: place.location, duration: 0.0, time },
             job: None,
             commute: None,
         }

vrp-core/src/solver/processing/vicinity_clustering.rs

@@ -125,7 +125,8 @@ impl HeuristicSolutionProcessing for VicinityClustering {
                     cluster.into_iter().fold((cluster_arrival, Vec::new()), |(arrival, mut activities), info| {
                         // NOTE assumption: no waiting time possible in between of clustered jobs
                         let job = info.job.to_single().clone();
-                        let place = job.places.first().unwrap();
+                        let place_idx = 0;
+                        let place = &job.places[place_idx];
 
                         let backward = match config.visiting {
                             VisitPolicy::Return => info.commute.backward.duration,
@@ -139,6 +140,7 @@ impl HeuristicSolutionProcessing for VicinityClustering {
 
                         activities.push(Activity {
                             place: Place {
+                                idx: place_idx,
                                 location: place.location.unwrap(),
                                 duration: info.service_time,
                                 time: cluster_time.clone(),

vrp-core/tests/helpers/models/solution/route.rs

@@ -14,7 +14,7 @@ pub fn test_activity() -> Activity {
 
 pub fn test_activity_with_location(location: Location) -> Activity {
     Activity {
-        place: Place { location, duration: DEFAULT_JOB_DURATION, time: DEFAULT_ACTIVITY_TIME_WINDOW },
+        place: Place { idx: 0, location, duration: DEFAULT_JOB_DURATION, time: DEFAULT_ACTIVITY_TIME_WINDOW },
         schedule: Schedule::new(location as f64, location as f64 + DEFAULT_JOB_DURATION),
         job: Some(test_single_with_location(Some(location))),
         commute: None,
@@ -23,7 +23,7 @@ pub fn test_activity_with_location(location: Location) -> Activity {
 
 pub fn test_activity_with_location_and_duration(location: Location, duration: Duration) -> Activity {
     Activity {
-        place: Place { location, duration, time: DEFAULT_ACTIVITY_TIME_WINDOW },
+        place: Place { idx: 0, location, duration, time: DEFAULT_ACTIVITY_TIME_WINDOW },
         schedule: Schedule::new(location as f64, location as f64 + DEFAULT_JOB_DURATION),
         job: Some(test_single_with_location(Some(location))),
         commute: None,
@@ -32,7 +32,7 @@ pub fn test_activity_with_location_and_duration(location: Location, duration: Du
 
 pub fn test_activity_with_location_and_tw(location: Location, tw: TimeWindow) -> Activity {
     Activity {
-        place: Place { location, duration: DEFAULT_JOB_DURATION, time: tw },
+        place: Place { idx: 0, location, duration: DEFAULT_JOB_DURATION, time: tw },
         schedule: Schedule::new(location as f64, location as f64 + DEFAULT_JOB_DURATION),
         job: Some(test_single_with_location(Some(location))),
         commute: None,
@@ -41,7 +41,7 @@ pub fn test_activity_with_location_and_tw(location: Location, tw: TimeWindow) ->
 
 pub fn test_activity_with_location_tw_and_duration(location: Location, tw: TimeWindow, duration: Duration) -> Activity {
     Activity {
-        place: Place { location, duration, time: tw },
+        place: Place { idx: 0, location, duration, time: tw },
         schedule: Schedule::new(location as f64, location as f64 + duration),
         job: Some(test_single_with_location(Some(location))),
         commute: None,
@@ -51,6 +51,7 @@ pub fn test_activity_with_location_tw_and_duration(location: Location, tw: TimeW
 pub fn test_activity_with_schedule(schedule: Schedule) -> Activity {
     Activity {
         place: Place {
+            idx: 0,
             location: DEFAULT_JOB_LOCATION,
             duration: DEFAULT_JOB_DURATION,
             time: DEFAULT_ACTIVITY_TIME_WINDOW,
@@ -64,6 +65,7 @@ pub fn test_activity_with_schedule(schedule: Schedule) -> Activity {
 pub fn test_activity_with_job(job: Arc<Single>) -> Activity {
     Activity {
         place: Place {
+            idx: 0,
             location: DEFAULT_JOB_LOCATION,
             duration: DEFAULT_JOB_DURATION,
             time: DEFAULT_ACTIVITY_TIME_WINDOW,
@@ -77,6 +79,7 @@ pub fn test_activity_with_job(job: Arc<Single>) -> Activity {
 pub fn test_activity_without_job() -> Activity {
     Activity {
         place: Place {
+            idx: 0,
             location: DEFAULT_JOB_LOCATION,
             duration: DEFAULT_JOB_DURATION,
             time: DEFAULT_ACTIVITY_TIME_WINDOW,

vrp-core/tests/helpers/solver/mutation.rs

@@ -87,9 +87,11 @@ pub fn rearrange_jobs_in_routes(insertion_ctx: &mut InsertionContext, job_order:
 
         order.iter().for_each(|job_id| {
             let job = jobs_map.get(&job_id.to_string()).unwrap().to_single().clone();
-            let place = job.places.first().unwrap();
+            let place_idx = 0;
+            let place = &job.places[place_idx];
             route_ctx.route_mut().tour.insert_last(Activity {
                 place: Place {
+                    idx: place_idx,
                     location: place.location.unwrap(),
                     duration: place.duration,
                     time: place.times.first().unwrap().to_time_window(0.),

vrp-core/tests/unit/construction/features/transport_test.rs

@@ -138,11 +138,16 @@ mod timing {
                 "v1",
                 vec![
                     ActivityBuilder::default()
-                        .place(Place { location: 10, duration: 5., time: TimeWindow { start: 20., end: 30. } })
+                        .place(Place { idx: 0, location: 10, duration: 5., time: TimeWindow { start: 20., end: 30. } })
                         .schedule(Schedule::new(10., 25.))
                         .build(),
                     ActivityBuilder::default()
-                        .place(Place { location: 20, duration: 10., time: TimeWindow { start: 50., end: 100. } })
+                        .place(Place {
+                            idx: 0,
+                            location: 20,
+                            duration: 10.,
+                            time: TimeWindow { start: 50., end: 100. },
+                        })
                         .schedule(Schedule::new(35., 60.))
                         .build(),
                 ],
@@ -166,7 +171,7 @@ mod timing {
             .build();
         let route_ctx = create_route_context_with_activities(&fleet, "v1", vec![]);
         let target = Box::new(Activity {
-            place: Place { location: 5, duration: 1.0, time: DEFAULT_ACTIVITY_TIME_WINDOW },
+            place: Place { idx: 0, location: 5, duration: 1.0, time: DEFAULT_ACTIVITY_TIME_WINDOW },
             schedule: DEFAULT_ACTIVITY_SCHEDULE,
             job: None,
             commute: None,
@@ -194,16 +199,16 @@ mod timing {
             "v1",
             vec![
                 ActivityBuilder::default()
-                    .place(Place { location: 10, duration: 0.0, time: DEFAULT_ACTIVITY_TIME_WINDOW.clone() })
+                    .place(Place { idx: 0, location: 10, duration: 0.0, time: DEFAULT_ACTIVITY_TIME_WINDOW.clone() })
                     .schedule(Schedule { arrival: 0.0, departure: 10.0 })
                     .build(),
                 ActivityBuilder::default()
-                    .place(Place { location: 20, duration: 0.0, time: TimeWindow { start: 40.0, end: 70.0 } })
+                    .place(Place { idx: 0, location: 20, duration: 0.0, time: TimeWindow { start: 40.0, end: 70.0 } })
                     .build(),
             ],
         );
         let target = Box::new(Activity {
-            place: Place { location: 30, duration: 10.0, time: DEFAULT_ACTIVITY_TIME_WINDOW },
+            place: Place { idx: 0, location: 30, duration: 10.0, time: DEFAULT_ACTIVITY_TIME_WINDOW },
             schedule: DEFAULT_ACTIVITY_SCHEDULE,
             job: None,
             commute: None,

vrp-core/tests/unit/construction/heuristics/evaluators_test.rs

@@ -13,7 +13,7 @@ type JobPlace = crate::models::problem::Place;
 
 fn create_activity_at(loc_and_time: usize) -> Activity {
     ActivityBuilder::default()
-        .place(Place { location: loc_and_time, duration: 0.0, time: DEFAULT_JOB_TIME_SPAN.to_time_window(0.) })
+        .place(Place { idx: 0, location: loc_and_time, duration: 0.0, time: DEFAULT_JOB_TIME_SPAN.to_time_window(0.) })
         .schedule(Schedule { arrival: loc_and_time as Timestamp, departure: loc_and_time as Timestamp })
         .build()
 }

vrp-core/tests/unit/construction/probing/repair_solution_test.rs

@@ -115,7 +115,7 @@ fn add_new_route(insertion_ctx: &mut InsertionContext, vehicle_id: &str, activit
 
     activities.into_iter().for_each(|(location, duration, (tw_start, tw_end), single)| {
         tour.insert_last(Activity {
-            place: Place { location, duration, time: TimeWindow::new(tw_start, tw_end) },
+            place: Place { idx: 0, location, duration, time: TimeWindow::new(tw_start, tw_end) },
             schedule: Schedule::new(0., 0.),
             job: Some(single),
             commute: None,

vrp-core/tests/unit/solver/processing/vicinity_clustering_test.rs

@@ -88,7 +88,7 @@ fn can_unwrap_clusters_in_route_on_post_process_impl(
                     test_activity_with_schedule(Schedule::new(0., 0.)),
                     test_activity_with_schedule(Schedule::new(0., 0.)),
                     vec![Activity {
-                        place: Place { location: 3, duration: DEFAULT_JOB_DURATION * 3., time: clustered_time },
+                        place: Place { idx: 0, location: 3, duration: DEFAULT_JOB_DURATION * 3., time: clustered_time },
                         schedule: Schedule::new(3., 3. + duration),
                         job: Some(clustered_single),
                         commute: Some(Commute {

vrp-core/tests/unit/solver/search/local/exchange_swap_star_test.rs

@@ -14,7 +14,7 @@ fn create_insertion_success(insertion_ctx: &InsertionContext, insertion_data: (u
     let actor = insertion_ctx.solution.routes.get(route_idx).unwrap().route().actor.clone();
     let job = get_jobs_by_ids(insertion_ctx, &[job_id]).first().cloned().unwrap();
     let activity = Activity {
-        place: Place { location: 0, duration: 0.0, time: TimeWindow::new(0., 1.) },
+        place: Place { idx: 0, location: 0, duration: 0.0, time: TimeWindow::new(0., 1.) },
         schedule: Schedule { arrival: 0., departure: 0. },
         job: Some(job.to_single().clone()),
         commute: None,

vrp-pragmatic/src/format/solution/activity_matcher.rs

@@ -151,14 +151,15 @@ fn match_place(single: &Arc<Single>, is_job_activity: bool, activity_ctx: &Activ
         (true, true, _) | (true, false, false) => single
             .places
             .iter()
-            .find(|place| {
+            .enumerate()
+            .find(|(_, place)| {
                 let is_same_location = place.location.map_or(true, |l| l == activity_ctx.location);
                 let is_proper_time =
                     place.times.iter().any(|time| time.intersects(activity_ctx.route_start_time, &activity_ctx.time));
 
                 is_same_location && is_proper_time
             })
-            .map(|place| {
+            .map(|(idx, place)| {
                 // NOTE search for the latest occurrence assuming that times are sorted
                 let time = place
                     .times
@@ -173,7 +174,7 @@ fn match_place(single: &Arc<Single>, is_job_activity: bool, activity_ctx: &Activ
                     }
                 };
 
-                Place { location: activity_ctx.location, duration: place.duration, time }
+                Place { idx, location: activity_ctx.location, duration: place.duration, time }
             }),
         _ => None,
     }
@@ -224,7 +225,7 @@ pub(super) fn get_job_tag(single: &Single, place: (Location, (TimeWindow, Timest
 
 fn get_job_id(single: &Arc<Single>) -> String {
     Activity {
-        place: Place { location: 0, duration: 0.0, time: TimeWindow::new(0., 0.) },
+        place: Place { idx: 0, location: 0, duration: 0.0, time: TimeWindow::new(0., 0.) },
         schedule: Schedule { arrival: 0.0, departure: 0.0 },
         job: Some(single.clone()),
         commute: None,

vrp-pragmatic/tests/helpers/core.rs

@@ -69,6 +69,7 @@ pub fn create_route_with_activities(fleet: &Fleet, vehicle: &str, activities: Ve
 pub fn create_activity_at_location(location: Location) -> Activity {
     Activity {
         place: vrp_core::models::solution::Place {
+            idx: 0,
             location,
             duration: DEFAULT_JOB_DURATION,
             time: DEFAULT_ACTIVITY_TIME_WINDOW,

vrp-scientific/src/common/initial_reader.rs

@@ -47,9 +47,11 @@ pub fn read_init_solution<R: Read>(
 
                 route.last().unwrap().split_whitespace().for_each(|id| {
                     let single = id_map.get(id).unwrap();
-                    let place = single.places.first().unwrap();
+                    let place_idx = 0;
+                    let place = &single.places[place_idx];
                     tour.insert_last(Activity {
                         place: vrp_core::models::solution::Place {
+                            idx: place_idx,
                             location: place.location.unwrap(),
                             duration: place.duration,
                             time: place.times.first().and_then(|span| span.as_time_window()).unwrap(),