Merge branch 'fix/capacity-array-check-consistency'

CHANGELOG.md

@@ -16,6 +16,7 @@
 - Refactor to use `std::format` whenever possible (#1081)
 - Reduce complexity for recreation process (#1155)
 - Refactor `SolutionIndicators` (#1169)
+- Remove amount consistency checks in `parse` in favor of upstream checks in `Input` (#1086)
 
 #### CI
 
@@ -31,6 +32,7 @@
 #### Core solving
 
 - Crash when input is valid JSON but not an object (#1172)
+- Capacity array check consistency (#1086)
 
 #### Internals
 

libvroom_examples/libvroom.cpp

@@ -105,8 +105,8 @@ void run_example_with_osrm() {
 
   // Create one-dimension capacity restrictions to model the situation
   // where one vehicle can handle 4 jobs with deliveries.
-  problem_instance.set_amount_size(amount_dimension);
   vroom::Amount vehicle_capacity(amount_dimension);
+  vehicle_capacity[0] = 4;
 
   vroom::TimeWindow vehicle_tw(28800, 43200); // Working hours.
   // Default "zero" amount data structures with relevant dimension.
@@ -120,7 +120,6 @@ void run_example_with_osrm() {
 
   vroom::UserDuration setup = 0;
   vroom::UserDuration service = 5 * 60; // 5 minutes
-  vehicle_capacity[0] = 4;
 
   // Define vehicle breaks.
   vroom::Break break_1(1, {vroom::TimeWindow(32400, 34200)}, 300);

src/structures/vroom/input/input.cpp

@@ -33,11 +33,6 @@ Input::Input(io::Servers servers, ROUTER router, bool apply_TSPFix)
   : _apply_TSPFix(apply_TSPFix), _servers(std::move(servers)), _router(router) {
 }
 
-void Input::set_amount_size(unsigned amount_size) {
-  _amount_size = amount_size;
-  _zero = Amount(amount_size);
-}
-
 void Input::set_geometry(bool geometry) {
   _geometry = geometry;
 }
@@ -103,23 +98,27 @@ void Input::add_routing_wrapper(const std::string& profile) {
 #endif
 }
 
-void Input::check_job(Job& job) {
-  // Ensure delivery size consistency.
-  if (const auto delivery_size = job.delivery.size();
-      delivery_size != _amount_size) {
-    throw InputException(
-      std::format("Inconsistent delivery length: {} instead of {}.",
-                  delivery_size,
-                  _amount_size));
-  }
+void Input::check_amount_size(const Amount& amount) {
+  const auto size = amount.size();
 
-  // Ensure pickup size consistency.
-  if (const auto pickup_size = job.pickup.size(); pickup_size != _amount_size) {
-    throw InputException(
-      std::format("Inconsistent pickup length: {} instead of {}.",
-                  pickup_size,
-                  _amount_size));
+  if (!_amount_size.has_value()) {
+    // Only setup once on first call.
+    _amount_size = size;
+    _zero = Amount(size);
+  } else {
+    if (size != _amount_size.value()) {
+      throw InputException(
+        std::format("Inconsistent delivery length: {} instead of {}.",
+                    size,
+                    _amount_size.value()));
+    }
   }
+}
+
+void Input::check_job(Job& job) {
+  // Ensure delivery and pickup size consistency.
+  check_amount_size(job.delivery);
+  check_amount_size(job.pickup);
 
   // Ensure that location index are either always or never provided.
   bool has_location_index = job.location.user_index();
@@ -245,13 +244,7 @@ void Input::add_vehicle(const Vehicle& vehicle) {
   auto& current_v = vehicles.back();
 
   // Ensure amount size consistency.
-  if (const auto vehicle_amount_size = current_v.capacity.size();
-      vehicle_amount_size != _amount_size) {
-    throw InputException(
-      std::format("Inconsistent capacity length: {} instead of {}.",
-                  vehicle_amount_size,
-                  _amount_size));
-  }
+  check_amount_size(current_v.capacity);
 
   // Check for time-windows and skills.
   _has_TW = _has_TW || !vehicle.tw.is_default() || !vehicle.breaks.empty();
@@ -612,7 +605,8 @@ void Input::set_vehicles_costs() {
 }
 
 void Input::set_vehicles_max_tasks() {
-  if (_has_jobs && !_has_shipments && _amount_size > 0) {
+  if (const auto amount_size = get_amount_size();
+      _has_jobs && !_has_shipments && amount_size > 0) {
     // For job-only instances where capacity restrictions apply:
     // compute an upper bound of the number of jobs for each vehicle
     // based on pickups load and delivery loads. This requires sorting
@@ -627,12 +621,12 @@ void Input::set_vehicles_max_tasks() {
     };
 
     std::vector<std::vector<JobAmount>>
-      job_pickups_per_component(_amount_size,
+      job_pickups_per_component(amount_size,
                                 std::vector<JobAmount>(jobs.size()));
     std::vector<std::vector<JobAmount>>
-      job_deliveries_per_component(_amount_size,
+      job_deliveries_per_component(amount_size,
                                    std::vector<JobAmount>(jobs.size()));
-    for (std::size_t i = 0; i < _amount_size; ++i) {
+    for (std::size_t i = 0; i < amount_size; ++i) {
       for (Index j = 0; j < jobs.size(); ++j) {
         job_pickups_per_component[i][j] = JobAmount({j, jobs[j].pickup[i]});
         job_deliveries_per_component[i][j] =
@@ -649,7 +643,7 @@ void Input::set_vehicles_max_tasks() {
     for (Index v = 0; v < vehicles.size(); ++v) {
       std::size_t max_tasks = jobs.size();
 
-      for (std::size_t i = 0; i < _amount_size; ++i) {
+      for (std::size_t i = 0; i < amount_size; ++i) {
         Capacity pickup_sum = 0;
         Capacity delivery_sum = 0;
         std::size_t doable_pickups = 0;

src/structures/vroom/input/input.h

@@ -12,6 +12,7 @@ All rights reserved (see LICENSE).
 
 #include <chrono>
 #include <memory>
+#include <optional>
 #include <unordered_map>
 
 #include "routing/wrapper.h"
@@ -77,14 +78,15 @@ class Input {
   bool _all_locations_have_coords{true};
   std::vector<std::vector<Eval>> _jobs_vehicles_evals;
 
-  unsigned _amount_size{0};
-  Amount _zero{0};
+  std::optional<unsigned> _amount_size;
+  Amount _zero;
 
   const io::Servers _servers;
   const ROUTER _router;
 
   std::unique_ptr<VRP> get_problem() const;
 
+  void check_amount_size(const Amount& amount);
   void check_job(Job& job);
 
   UserCost check_cost_bound(const Matrix<UserCost>& matrix) const;
@@ -114,10 +116,9 @@ class Input {
         ROUTER router = ROUTER::OSRM,
         bool apply_TSPFix = false);
 
-  void set_amount_size(unsigned amount_size);
-
   unsigned get_amount_size() const {
-    return _amount_size;
+    assert(_amount_size.has_value());
+    return _amount_size.value();
   }
 
   void set_geometry(bool geometry);

src/structures/vroom/vehicle.cpp

@@ -66,7 +66,7 @@ Vehicle::Vehicle(Id id,
     if (b.max_load.has_value() &&
         b.max_load.value().size() != capacity.size()) {
       throw InputException(
-        std::format("Inconsistent break max_load size for break: {}.", b.id));
+        std::format("Inconsistent break max_load size for break {}.", b.id));
     }
   }
 

src/utils/input_parser.cpp

@@ -51,21 +51,21 @@ inline double get_double(const rapidjson::Value& object, const char* key) {
 inline Amount get_amount(const rapidjson::Value& object,
                          const char* key,
                          unsigned amount_size) {
-  // Default to zero amount with provided size.
-  Amount amount(amount_size);
+  const bool has_amount_key = object.HasMember(key);
 
-  if (object.HasMember(key)) {
+  if (has_amount_key) {
     if (!object[key].IsArray()) {
       throw InputException("Invalid " + std::string(key) + " array.");
     }
 
-    if (object[key].Size() != amount_size) {
-      throw InputException(std::format("Inconsistent {} length: {} and {}.",
-                                       key,
-                                       object[key].Size(),
-                                       amount_size));
-    }
+    amount_size = object[key].Size();
+  }
+
+  // This will default to zero amount with provided size if expected
+  // key is omitted.
+  Amount amount(amount_size);
 
+  if (has_amount_key) {
     for (rapidjson::SizeType i = 0; i < object[key].Size(); ++i) {
       if (!object[key][i].IsUint()) {
         throw InputException("Invalid " + std::string(key) + " value.");
@@ -420,7 +420,7 @@ inline Vehicle get_vehicle(const rapidjson::Value& json_vehicle,
                  start,
                  end,
                  profile,
-                 get_amount(json_vehicle, "capacity", amount_size),
+                 get_amount(json_vehicle, "capacity", 0),
                  get_skills(json_vehicle),
                  get_vehicle_time_window(json_vehicle),
                  get_vehicle_breaks(json_vehicle, amount_size),
@@ -542,7 +542,6 @@ void parse(Input& input, const std::string& input_str, bool geometry) {
   const unsigned amount_size =
     first_vehicle_has_capacity ? first_vehicle["capacity"].Size() : 0;
 
-  input.set_amount_size(amount_size);
   input.set_geometry(geometry);
 
   // Add all vehicles.