From de69f9e1b7501b54c66fb527ddbaf88be2939929 Mon Sep 17 00:00:00 2001 From: Nikolai Poperechnyi Date: Mon, 8 Jun 2026 22:49:24 +0300 Subject: [PATCH 1/2] bugfix (routing): fix incorrect min_vehicle_count --- cpp/src/routing/crossovers/ox_recombiner.cuh | 17 +++-- .../local_search/cycle_finder/cycle.hpp | 11 ++- .../move_candidates/move_candidates.cuh | 4 +- cpp/src/routing/solver.cu | 4 +- .../cuopt/cuopt/tests/routing/test_solver.py | 69 +++++++++++++++++++ 5 files changed, 96 insertions(+), 9 deletions(-) diff --git a/cpp/src/routing/crossovers/ox_recombiner.cuh b/cpp/src/routing/crossovers/ox_recombiner.cuh index 404d450585..ee8ac03aa2 100644 --- a/cpp/src/routing/crossovers/ox_recombiner.cuh +++ b/cpp/src/routing/crossovers/ox_recombiner.cuh @@ -204,7 +204,13 @@ struct OX { routes_number = std::min(routesA.size(), routesB.size()); const auto& dimensions_info = A.problem->dimensions_info; - if (dimensions_info.has_dimension(dim_t::VEHICLE_FIXED_COST)) { + // Optimal-routes search lets Bellman-Ford pick a variable number of routes to minimize + // vehicle fixed cost. This is incompatible with fixed_route mode (fleet_size == + // min_vehicles): the vehicle count cannot change, so searching over route counts both + // breaks the recreate_solution invariant (routes removed == routes added) and can drift + // the solution below min_vehicles. When the route count is fixed there is nothing to + // optimize over, so keep the strict fixed-route path. + if (!fixed_route && dimensions_info.has_dimension(dim_t::VEHICLE_FIXED_COST)) { routes_number = max_vehicle_increase + std::max(routesA.size(), routesB.size()); optimal_routes_search = true; } @@ -390,9 +396,12 @@ struct OX { --i; } - if (fixed_route) { - cuopt_assert(routes_to_remove.size() == tmp_routes.size(), - "number of routes removed and routes added should be same"); + if (fixed_route && routes_to_remove.size() != tmp_routes.size()) { + // In fixed_route mode the vehicle count must be preserved: we add one route per changed + // offspring segment (tmp_routes) and remove the distinct original routes those segments + // touch (routes_to_remove). A mismatch would change the vehicle count and drop it below + // min_vehicles, so reject this offspring instead of applying it. + return false; } if (routes_to_remove.size() == 0 || tmp_routes.size() == 0) { return false; } diff --git a/cpp/src/routing/local_search/cycle_finder/cycle.hpp b/cpp/src/routing/local_search/cycle_finder/cycle.hpp index 7e3e275e13..eac771e7a2 100644 --- a/cpp/src/routing/local_search/cycle_finder/cycle.hpp +++ b/cpp/src/routing/local_search/cycle_finder/cycle.hpp @@ -8,6 +8,7 @@ #pragma once #include +#include #include #include "../../solution/solution_handle.cuh" @@ -55,11 +56,19 @@ struct ret_cycles_t { } struct view_t { - DI void push_back(i_t val) { paths[offsets[*n_cycles_] + curr_cycle_size++] = val; } + DI void push_back(i_t val) + { + const auto write_pos = offsets[*n_cycles_] + curr_cycle_size++; + cuopt_assert(write_pos < (i_t)paths.size(), + "ret_cycles paths overflow: increase cycle buffer size"); + paths[write_pos] = val; + } DI void append_cycle(i_t cycle_size) { *n_cycles_ += 1; + cuopt_assert(*n_cycles_ < (i_t)offsets.size(), + "ret_cycles offsets overflow: increase cycle buffer size"); offsets[*n_cycles_] = offsets[*n_cycles_ - 1] + cycle_size; } diff --git a/cpp/src/routing/local_search/move_candidates/move_candidates.cuh b/cpp/src/routing/local_search/move_candidates/move_candidates.cuh index 1fb6ab1bba..a0f746a28c 100644 --- a/cpp/src/routing/local_search/move_candidates/move_candidates.cuh +++ b/cpp/src/routing/local_search/move_candidates/move_candidates.cuh @@ -123,8 +123,8 @@ class move_path_t { void reset(solution_handle_t const* sol_handle) { - constexpr i_t zero_val = 0; - n_insertions.set_value_async(zero_val, sol_handle->get_stream()); + // set_value_to_zero_async() is capture-safe (no host source). + n_insertions.set_value_to_zero_async(sol_handle->get_stream()); async_fill(loop_closed, 1, sol_handle->get_stream()); async_fill(changed_routes, 0, sol_handle->get_stream()); } diff --git a/cpp/src/routing/solver.cu b/cpp/src/routing/solver.cu index b160a2d140..e8ed6551be 100644 --- a/cpp/src/routing/solver.cu +++ b/cpp/src/routing/solver.cu @@ -1,6 +1,6 @@ /* clang-format off */ /* - * SPDX-FileCopyrightText: Copyright (c) 2021-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-FileCopyrightText: Copyright (c) 2021-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. * SPDX-License-Identifier: Apache-2.0 */ /* clang-format on */ @@ -66,7 +66,7 @@ assignment_t solver_t::solve() // We only set target vehicles and use fixed route loop in the below case. The other paths will // run regular fixed route loop. auto target_vehicles = -1; - if (data_view_ptr_->get_fleet_size() == data_view_ptr_->get_min_vehicles()) { + if (data_view_ptr_->get_fleet_size() >= data_view_ptr_->get_min_vehicles()) { target_vehicles = data_view_ptr_->get_min_vehicles(); } diff --git a/python/cuopt/cuopt/tests/routing/test_solver.py b/python/cuopt/cuopt/tests/routing/test_solver.py index 92622b1f16..6895fe6976 100644 --- a/python/cuopt/cuopt/tests/routing/test_solver.py +++ b/python/cuopt/cuopt/tests/routing/test_solver.py @@ -262,3 +262,72 @@ def test_prize_collection(): assert objectives[routing.Objective.COST] == 13.0 assert sol.get_status() == 0 assert sol.get_vehicle_count() >= 2 + + +# Cost matrix from issue #904 (7 locations: depot 0 + orders 1-6) +_ISSUE_904_COST_MATRIX = [ + [0, 17, 12, 11, 10, 18, 10], + [16, 0, 15, 11, 19, 15, 16], + [19, 19, 0, 11, 16, 11, 17], + [17, 19, 17, 0, 11, 18, 19], + [10, 19, 19, 19, 0, 17, 15], + [12, 18, 15, 18, 18, 0, 14], + [12, 12, 11, 19, 10, 17, 0], +] + + +def _build_issue_904_data_model(vehicle_fixed_costs, min_vehicles=3): + n_locations = 7 + n_vehicles = 3 + n_orders = 6 + dm = routing.DataModel(n_locations, n_vehicles, n_orders) + dm.add_cost_matrix( + cudf.DataFrame(_ISSUE_904_COST_MATRIX).astype(np.float32) + ) + # Capacity 10 lets all 6 orders fit on one vehicle; min_vehicles must still + # force 3 routes. + dm.add_capacity_dimension( + "demand", + cudf.Series([1] * n_orders, dtype=np.int32), + cudf.Series([10] * n_vehicles, dtype=np.int32), + ) + dm.set_order_locations(cudf.Series([1, 2, 3, 4, 5, 6], dtype=np.int32)) + dm.set_vehicle_fixed_costs( + cudf.Series(vehicle_fixed_costs, dtype=np.float32) + ) + dm.set_min_vehicles(min_vehicles) + dm.set_objective_function( + cudf.Series( + [routing.Objective.COST, routing.Objective.VEHICLE_FIXED_COST] + ), + cudf.Series([1.0, 1.0], dtype=np.float32), + ) + return dm + + +def test_issue_904_min_vehicles_with_fixed_costs(): + # Regression for https://github.com/NVIDIA/cuopt/issues/904: with non-zero + # vehicle_fixed_costs and min_vehicles == fleet_size, the solver entered the + # fixed-route LS loop where a CUDA-graph-captured reset left n_insertions + # corrupted, crashing with a device-side assert. It must now honor + # min_vehicles and complete cleanly. + dm = _build_issue_904_data_model(vehicle_fixed_costs=[10.0, 20.0, 30.0]) + ss = routing.SolverSettings() + ss.set_time_limit(3) + + sol = routing.Solve(dm, ss) + + assert sol.get_status() == 0, sol.get_error_message() + assert sol.get_vehicle_count() >= 3 + + +def test_issue_904_min_vehicles_with_zero_fixed_costs(): + # Control case: min_vehicles enforced with zero fixed costs. + dm = _build_issue_904_data_model(vehicle_fixed_costs=[0.0, 0.0, 0.0]) + ss = routing.SolverSettings() + ss.set_time_limit(3) + + sol = routing.Solve(dm, ss) + + assert sol.get_status() == 0, sol.get_error_message() + assert sol.get_vehicle_count() >= 3 From 5c495463db090a332470f002d768d719fcd94738 Mon Sep 17 00:00:00 2001 From: Nikolai Poperechnyi Date: Tue, 9 Jun 2026 11:20:58 +0300 Subject: [PATCH 2/2] #904: add 0 min_vehicles guard, comments, cleaner test --- cpp/src/routing/crossovers/ox_recombiner.cuh | 9 ++++ .../local_search/cycle_finder/cycle.hpp | 6 +++ .../move_candidates/move_candidates.cuh | 1 + cpp/src/routing/solver.cu | 7 +-- .../cuopt/cuopt/tests/routing/test_solver.py | 43 ++++++++++--------- 5 files changed, 43 insertions(+), 23 deletions(-) diff --git a/cpp/src/routing/crossovers/ox_recombiner.cuh b/cpp/src/routing/crossovers/ox_recombiner.cuh index ee8ac03aa2..cefbd8df15 100644 --- a/cpp/src/routing/crossovers/ox_recombiner.cuh +++ b/cpp/src/routing/crossovers/ox_recombiner.cuh @@ -169,6 +169,14 @@ struct OX { return graphs_size + sol_arrays_size + helper_arrays_size; } + /// @brief OX recombination of two parents into A (offspring built in place). + /// In fixed_route mode (fleet_size == min_vehicles) the vehicle count + /// is preserved; otherwise the offspring route count may vary. + /// @param A first parent; on success, replaced by the recombined offspring. + /// @param B second parent (read-only donor genome). + /// @return true if a valid offspring was produced and applied to A; false if + /// recombination was rejected (e.g. mismatched parents, fixed-route + /// count violation, size/memory guards) — A is then left unchanged. bool recombine(Solution& A, Solution& B) { raft::common::nvtx::range fun_scope("ox"); @@ -401,6 +409,7 @@ struct OX { // offspring segment (tmp_routes) and remove the distinct original routes those segments // touch (routes_to_remove). A mismatch would change the vehicle count and drop it below // min_vehicles, so reject this offspring instead of applying it. + cuopt_assert(false, "number of routes removed and routes added should be same"); return false; } if (routes_to_remove.size() == 0 || tmp_routes.size() == 0) { return false; } diff --git a/cpp/src/routing/local_search/cycle_finder/cycle.hpp b/cpp/src/routing/local_search/cycle_finder/cycle.hpp index eac771e7a2..9b922ccda2 100644 --- a/cpp/src/routing/local_search/cycle_finder/cycle.hpp +++ b/cpp/src/routing/local_search/cycle_finder/cycle.hpp @@ -56,6 +56,10 @@ struct ret_cycles_t { } struct view_t { + /// Append a vertex to the in-progress cycle (`*n_cycles_`): writes at + /// `offsets[*n_cycles_] + curr_cycle_size` into the flat `paths` buffer and + /// bumps the per-cycle counter. Caller resets `curr_cycle_size` per cycle; + /// append_cycle() finalizes it. DI void push_back(i_t val) { const auto write_pos = offsets[*n_cycles_] + curr_cycle_size++; @@ -64,6 +68,8 @@ struct ret_cycles_t { paths[write_pos] = val; } + /// Close the current cycle: advance the cycle count and store its end as the + /// next offsets[] prefix-sum boundary (offsets[c] = offsets[c-1] + cycle_size). DI void append_cycle(i_t cycle_size) { *n_cycles_ += 1; diff --git a/cpp/src/routing/local_search/move_candidates/move_candidates.cuh b/cpp/src/routing/local_search/move_candidates/move_candidates.cuh index a0f746a28c..e91139c962 100644 --- a/cpp/src/routing/local_search/move_candidates/move_candidates.cuh +++ b/cpp/src/routing/local_search/move_candidates/move_candidates.cuh @@ -121,6 +121,7 @@ class move_path_t { } } + /// Reset per-iteration move state void reset(solution_handle_t const* sol_handle) { // set_value_to_zero_async() is capture-safe (no host source). diff --git a/cpp/src/routing/solver.cu b/cpp/src/routing/solver.cu index e8ed6551be..6dca14ce90 100644 --- a/cpp/src/routing/solver.cu +++ b/cpp/src/routing/solver.cu @@ -65,9 +65,10 @@ assignment_t solver_t::solve() // TODO accept a settings object once we have full feature in ges solver // We only set target vehicles and use fixed route loop in the below case. The other paths will // run regular fixed route loop. - auto target_vehicles = -1; - if (data_view_ptr_->get_fleet_size() >= data_view_ptr_->get_min_vehicles()) { - target_vehicles = data_view_ptr_->get_min_vehicles(); + auto target_vehicles = -1; + const auto min_vehicles = data_view_ptr_->get_min_vehicles(); + if (min_vehicles > 0 && data_view_ptr_->get_fleet_size() >= min_vehicles) { + target_vehicles = min_vehicles; } const bool is_pdp = data_view_ptr_->get_pickup_delivery_pair().first != nullptr; diff --git a/python/cuopt/cuopt/tests/routing/test_solver.py b/python/cuopt/cuopt/tests/routing/test_solver.py index 6895fe6976..9534e5d24a 100644 --- a/python/cuopt/cuopt/tests/routing/test_solver.py +++ b/python/cuopt/cuopt/tests/routing/test_solver.py @@ -3,6 +3,7 @@ import os import numpy as np +import pytest import cudf @@ -276,7 +277,8 @@ def test_prize_collection(): ] -def _build_issue_904_data_model(vehicle_fixed_costs, min_vehicles=3): +def _build_min_vehicles_data_model(vehicle_fixed_costs, min_vehicles=3): + """Builds min vehicles regression data model""" n_locations = 7 n_vehicles = 3 n_orders = 6 @@ -305,25 +307,26 @@ def _build_issue_904_data_model(vehicle_fixed_costs, min_vehicles=3): return dm -def test_issue_904_min_vehicles_with_fixed_costs(): - # Regression for https://github.com/NVIDIA/cuopt/issues/904: with non-zero - # vehicle_fixed_costs and min_vehicles == fleet_size, the solver entered the - # fixed-route LS loop where a CUDA-graph-captured reset left n_insertions - # corrupted, crashing with a device-side assert. It must now honor - # min_vehicles and complete cleanly. - dm = _build_issue_904_data_model(vehicle_fixed_costs=[10.0, 20.0, 30.0]) - ss = routing.SolverSettings() - ss.set_time_limit(3) - - sol = routing.Solve(dm, ss) - - assert sol.get_status() == 0, sol.get_error_message() - assert sol.get_vehicle_count() >= 3 - - -def test_issue_904_min_vehicles_with_zero_fixed_costs(): - # Control case: min_vehicles enforced with zero fixed costs. - dm = _build_issue_904_data_model(vehicle_fixed_costs=[0.0, 0.0, 0.0]) +@pytest.mark.parametrize( + "vehicle_fixed_costs", + [ + [10.0, 20.0, 30.0], # non-zero fixed costs (the bug case in #904) + [ + 0.0, + 0.0, + 0.0, + ], # zero fixed costs (H100 12.2/13.1 compat crashed in debug) + ], +) +def test_min_vehicles_respected(vehicle_fixed_costs): + """ + Regression for https://github.com/NVIDIA/cuopt/issues/904. + Verifies that min_vehicles is respected and no crash occurs, regardless of + vehicle fixed costs. + """ + dm = _build_min_vehicles_data_model( + vehicle_fixed_costs=vehicle_fixed_costs + ) ss = routing.SolverSettings() ss.set_time_limit(3)