From 5b747da6325f31c3029a766e17879f6f3c0b2875 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Fri, 10 Jul 2026 13:02:42 -0500 Subject: [PATCH 1/3] Routing DataModel: store-then-build (defer device construction to solve) The public DataModel no longer inherits the Cython wrapper and no longer pushes each input to the GPU as it is set. Instead it records the setter calls and materializes the device (Cython) data model lazily -- when a solve runs or a getter is queried -- by replaying the recorded calls onto the wrapper. Size scalars are answered directly from the constructor args so setter-time validation does not trigger a build. This keeps the user's inputs host-resident while a problem is being assembled and exposes the recorded inputs for host-only problem construction (remote / gRPC serialization). Behavior is preserved except that wrapper/C++ validation and dtype-cast warnings now surface at build/solve time rather than at the setter call; two tests are updated to reflect this deferral. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Ramakrishna Prabhu --- python/cuopt/cuopt/routing/_recording.py | 164 ++++++++++++++++++ python/cuopt/cuopt/routing/vehicle_routing.py | 10 +- .../tests/routing/test_warnings_exceptions.py | 32 ++-- 3 files changed, 191 insertions(+), 15 deletions(-) create mode 100644 python/cuopt/cuopt/routing/_recording.py diff --git a/python/cuopt/cuopt/routing/_recording.py b/python/cuopt/cuopt/routing/_recording.py new file mode 100644 index 0000000000..48bdbf5359 --- /dev/null +++ b/python/cuopt/cuopt/routing/_recording.py @@ -0,0 +1,164 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Store-then-build recording layer for the routing DataModel. + +The public DataModel records each mutating setter call instead of pushing to the +GPU immediately, and materializes the Cython/device data model only when a solve +runs (or a getter is queried). This keeps the user's inputs available on the host +until they are actually needed on the device, which: + + * avoids eager device allocation while a problem is being assembled, and + * exposes the recorded, host-resident inputs (``_calls``) for host-only + problem construction (remote / gRPC serialization). + +The device model is built by replaying the recorded calls onto the existing +Cython wrapper, so all conversion/validation behavior is unchanged. +""" + +# Mutating setters -> recorded and replayed onto the device model at build time. +_SETTERS = ( + "add_break_dimension", + "add_capacity_dimension", + "add_initial_solutions", + "add_order_precedence", + "add_order_vehicle_match", + "add_vehicle_break", + "add_vehicle_order_match", + "set_break_locations", + "set_drop_return_trips", + "set_min_vehicles", + "set_objective_function", + "set_order_locations", + "set_order_prizes", + "set_order_service_times", + "set_order_time_windows", + "set_pickup_delivery_pairs", + "set_skip_first_trips", + "set_vehicle_fixed_costs", + "set_vehicle_locations", + "set_vehicle_max_costs", + "set_vehicle_max_times", + "set_vehicle_time_windows", + "set_vehicle_types", +) + +# Non-scalar getters -> answered from the built device model. The three size +# scalars are answered directly (below) because setters query them during +# validation, before any build should happen. +_GETTERS = ( + "get_break_dimensions", + "get_break_locations", + "get_capacity_dimensions", + "get_cost_matrix", + "get_drop_return_trips", + "get_initial_solutions", + "get_min_vehicles", + "get_non_uniform_breaks", + "get_objective_function", + "get_order_locations", + "get_order_prizes", + "get_order_service_times", + "get_order_time_windows", + "get_order_vehicle_match", + "get_pickup_delivery_pairs", + "get_skip_first_trips", + "get_transit_time_matrices", + "get_transit_time_matrix", + "get_vehicle_fixed_costs", + "get_vehicle_locations", + "get_vehicle_max_costs", + "get_vehicle_max_times", + "get_vehicle_order_match", + "get_vehicle_time_windows", + "get_vehicle_types", +) + + +class _RecordingDataModel: + """Records DataModel setter calls and builds the device model lazily.""" + + def __init__(self, num_locations, fleet_size, n_orders=-1): + self._init_args = (num_locations, fleet_size, n_orders) + self._calls = [] + self._built = None + # Track added matrix vehicle-types so the public layer's duplicate + # check (``if vehicle_type in self.costs``) works before build. + self.costs = {} + self.transit_times = {} + + def add_cost_matrix(self, costs, vehicle_type=0): + self.costs[vehicle_type] = None + self._record("add_cost_matrix", (costs, vehicle_type), {}) + + def add_transit_time_matrix(self, times, vehicle_type=0): + self.transit_times[vehicle_type] = None + self._record("add_transit_time_matrix", (times, vehicle_type), {}) + + # -- size scalars: answered without building (queried during validation) -- + def get_num_locations(self): + return self._init_args[0] + + def get_fleet_size(self): + return self._init_args[1] + + def get_num_orders(self): + n_orders = self._init_args[2] + return self._init_args[0] if n_orders == -1 else n_orders + + # -- record / build -- + def _record(self, name, args, kwargs): + self._calls.append((name, args, kwargs)) + self._built = None + + def _build(self): + """Materialize the device (Cython) data model by replaying calls.""" + if self._built is None: + model = _built_cls()(*self._init_args) + for name, args, kwargs in self._calls: + getattr(model, name)(*args, **kwargs) + self._built = model + return self._built + + +_BUILT_CLS = None + + +def _built_cls(): + """Return a Python subclass of the Cython wrapper DataModel. + + The wrapper is a ``cdef class`` with no ``__dict__``; its ``__init__`` + stores Python attributes (``self.costs`` etc.), so it must be subclassed by + a Python class to be instantiable. Imported lazily. + """ + global _BUILT_CLS + if _BUILT_CLS is None: + from . import vehicle_routing_wrapper as _wrapper + + class _BuiltDataModel(_wrapper.DataModel): + pass + + _BUILT_CLS = _BuiltDataModel + return _BUILT_CLS + + +def _make_setter(name): + def _setter(self, *args, **kwargs): + self._record(name, args, kwargs) + + _setter.__name__ = name + return _setter + + +def _make_getter(name): + def _getter(self, *args, **kwargs): + return getattr(self._build(), name)(*args, **kwargs) + + _getter.__name__ = name + return _getter + + +for _name in _SETTERS: + setattr(_RecordingDataModel, _name, _make_setter(_name)) +for _name in _GETTERS: + setattr(_RecordingDataModel, _name, _make_getter(_name)) diff --git a/python/cuopt/cuopt/routing/vehicle_routing.py b/python/cuopt/cuopt/routing/vehicle_routing.py index d441a18283..3bebf40b26 100644 --- a/python/cuopt/cuopt/routing/vehicle_routing.py +++ b/python/cuopt/cuopt/routing/vehicle_routing.py @@ -7,6 +7,7 @@ from cuopt import routing from cuopt.routing import vehicle_routing_wrapper +from cuopt.routing._recording import _RecordingDataModel from cuopt.utilities import catch_cuopt_exception from .validation import ( @@ -19,7 +20,7 @@ ) -class DataModel(vehicle_routing_wrapper.DataModel): +class DataModel(_RecordingDataModel): """ DataModel(n_locations, n_fleet, n_orders: int = -1) @@ -1546,7 +1547,9 @@ def Solve(data_model, solver_settings=None): if solver_settings is None: solver_settings = SolverSettings() - solution = vehicle_routing_wrapper.Solve(data_model, solver_settings) + solution = vehicle_routing_wrapper.Solve( + data_model._build(), solver_settings + ) if solver_settings.get_config_file_name() is not None: routing.utils.save_data_model_to_yaml( data_model, @@ -1603,4 +1606,5 @@ def BatchSolve(data_model_list, solver_settings=None): if solver_settings is None: solver_settings = SolverSettings() - return vehicle_routing_wrapper.BatchSolve(data_model_list, solver_settings) + built_list = [dm._build() for dm in data_model_list] + return vehicle_routing_wrapper.BatchSolve(built_list, solver_settings) diff --git a/python/cuopt/cuopt/tests/routing/test_warnings_exceptions.py b/python/cuopt/cuopt/tests/routing/test_warnings_exceptions.py index aafcb45a6a..d8c8f8f74c 100644 --- a/python/cuopt/cuopt/tests/routing/test_warnings_exceptions.py +++ b/python/cuopt/cuopt/tests/routing/test_warnings_exceptions.py @@ -22,18 +22,23 @@ def test_type_casting_warnings(): constraints["service"] = [2.5, 2.5, 2.5] dm = routing.DataModel(3, 2) - with warnings.catch_warnings(record=True) as w: - dm.add_cost_matrix(cost_matrix) - assert "Casting cost_matrix from int64 to float32" in str(w[0].message) - - dm.set_order_time_windows( - constraints["earliest"], constraints["latest"] - ) + dm.add_cost_matrix(cost_matrix) + dm.set_order_time_windows(constraints["earliest"], constraints["latest"]) + dm.set_order_service_times(constraints["service"]) - dm.set_order_service_times(constraints["service"]) - assert "Casting service_times from float64 to int32" in str( - w[1].message - ) + # DataModel records inputs and converts them to device only when the model + # is built (store-then-build), so the dtype-cast warnings surface at build + # time. get_cost_matrix triggers the build that replays the recorded setters. + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + dm.get_cost_matrix(0) + messages = [str(rec.message) for rec in w] + assert any( + "Casting cost_matrix from int64 to float32" in m for m in messages + ) + assert any( + "Casting service_times from float64 to int32" in m for m in messages + ) # ----- Validation (matrix, time windows, range) ----- @@ -124,6 +129,9 @@ def test_range(): def test_invalid_datamodel(): + # Validation is deferred to build/solve time (store-then-build): the empty + # model is recorded at construction and rejected when it is solved. + dm = routing.DataModel(0, 0, 0) with pytest.raises(InputValidationError) as err: - routing.DataModel(0, 0, 0) + routing.Solve(dm) assert str(err.value) == "The data model needs at least one location" From 1205542c69e19b2641df5d57e3bed25f264f3155 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Fri, 10 Jul 2026 15:25:34 -0500 Subject: [PATCH 2/3] Derive recording methods from the wrapper instead of hardcoded lists The recorder/delegator methods were enumerated in _SETTERS/_GETTERS name lists, a second copy of the DataModel method surface that would silently drift when a setter/getter is added to the wrapper. Install them by introspecting the wrapper's method surface instead, so the wrapper stays the single source of truth and adding a method needs no change here. Explicit overrides (matrix setters, size scalars) are skipped by the auto-install. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Ramakrishna Prabhu --- python/cuopt/cuopt/routing/_recording.py | 142 +++++++++-------------- 1 file changed, 54 insertions(+), 88 deletions(-) diff --git a/python/cuopt/cuopt/routing/_recording.py b/python/cuopt/cuopt/routing/_recording.py index 48bdbf5359..d774f4f404 100644 --- a/python/cuopt/cuopt/routing/_recording.py +++ b/python/cuopt/cuopt/routing/_recording.py @@ -5,80 +5,28 @@ The public DataModel records each mutating setter call instead of pushing to the GPU immediately, and materializes the Cython/device data model only when a solve -runs (or a getter is queried). This keeps the user's inputs available on the host -until they are actually needed on the device, which: - - * avoids eager device allocation while a problem is being assembled, and - * exposes the recorded, host-resident inputs (``_calls``) for host-only - problem construction (remote / gRPC serialization). - -The device model is built by replaying the recorded calls onto the existing -Cython wrapper, so all conversion/validation behavior is unchanged. +runs (or a getter is queried) by replaying the recorded calls onto the wrapper. + +The recorded setters/getters mirror the Cython wrapper's method surface and are +installed automatically *from the wrapper* (see ``_install_methods``): every +``set_*``/``add_*`` becomes a recorder and every ``get_*`` a delegator. So there +is a single source of truth -- adding a method to the wrapper/DataModel needs no +change here. Only methods that need special behavior are written out explicitly +below (the matrix setters track vehicle-type, the size scalars answer without a +build); those explicit definitions are left untouched by the auto-install. """ -# Mutating setters -> recorded and replayed onto the device model at build time. -_SETTERS = ( - "add_break_dimension", - "add_capacity_dimension", - "add_initial_solutions", - "add_order_precedence", - "add_order_vehicle_match", - "add_vehicle_break", - "add_vehicle_order_match", - "set_break_locations", - "set_drop_return_trips", - "set_min_vehicles", - "set_objective_function", - "set_order_locations", - "set_order_prizes", - "set_order_service_times", - "set_order_time_windows", - "set_pickup_delivery_pairs", - "set_skip_first_trips", - "set_vehicle_fixed_costs", - "set_vehicle_locations", - "set_vehicle_max_costs", - "set_vehicle_max_times", - "set_vehicle_time_windows", - "set_vehicle_types", -) - -# Non-scalar getters -> answered from the built device model. The three size -# scalars are answered directly (below) because setters query them during -# validation, before any build should happen. -_GETTERS = ( - "get_break_dimensions", - "get_break_locations", - "get_capacity_dimensions", - "get_cost_matrix", - "get_drop_return_trips", - "get_initial_solutions", - "get_min_vehicles", - "get_non_uniform_breaks", - "get_objective_function", - "get_order_locations", - "get_order_prizes", - "get_order_service_times", - "get_order_time_windows", - "get_order_vehicle_match", - "get_pickup_delivery_pairs", - "get_skip_first_trips", - "get_transit_time_matrices", - "get_transit_time_matrix", - "get_vehicle_fixed_costs", - "get_vehicle_locations", - "get_vehicle_max_costs", - "get_vehicle_max_times", - "get_vehicle_order_match", - "get_vehicle_time_windows", - "get_vehicle_types", -) +# ``get_*`` methods on the wrapper that are helpers, not problem-data getters. +_SKIP_GETTERS = frozenset({"get_type_from_str", "get_type_from_int"}) + +_methods_installed = False class _RecordingDataModel: """Records DataModel setter calls and builds the device model lazily.""" def __init__(self, num_locations, fleet_size, n_orders=-1): + _install_methods() self._init_args = (num_locations, fleet_size, n_orders) self._calls = [] self._built = None @@ -87,6 +35,7 @@ def __init__(self, num_locations, fleet_size, n_orders=-1): self.costs = {} self.transit_times = {} + # -- explicit matrix setters: also track the vehicle-type key -- def add_cost_matrix(self, costs, vehicle_type=0): self.costs[vehicle_type] = None self._record("add_cost_matrix", (costs, vehicle_type), {}) @@ -121,6 +70,45 @@ def _build(self): return self._built +def _make_setter(name): + def _setter(self, *args, **kwargs): + self._record(name, args, kwargs) + + _setter.__name__ = name + return _setter + + +def _make_getter(name): + def _getter(self, *args, **kwargs): + return getattr(self._build(), name)(*args, **kwargs) + + _getter.__name__ = name + return _getter + + +def _install_methods(): + """Mirror the wrapper's setter/getter surface onto _RecordingDataModel. + + Derived from the wrapper so there is nothing to keep in sync. Methods + already defined on _RecordingDataModel (the explicit overrides above) are + skipped. Done once, lazily, on first construction so importing this module + does not require importing the wrapper. + """ + global _methods_installed + if _methods_installed: + return + from . import vehicle_routing_wrapper as _wrapper + + for name in dir(_wrapper.DataModel): + if name.startswith("_") or name in _RecordingDataModel.__dict__: + continue + if name.startswith(("set_", "add_")): + setattr(_RecordingDataModel, name, _make_setter(name)) + elif name.startswith("get_") and name not in _SKIP_GETTERS: + setattr(_RecordingDataModel, name, _make_getter(name)) + _methods_installed = True + + _BUILT_CLS = None @@ -140,25 +128,3 @@ class _BuiltDataModel(_wrapper.DataModel): _BUILT_CLS = _BuiltDataModel return _BUILT_CLS - - -def _make_setter(name): - def _setter(self, *args, **kwargs): - self._record(name, args, kwargs) - - _setter.__name__ = name - return _setter - - -def _make_getter(name): - def _getter(self, *args, **kwargs): - return getattr(self._build(), name)(*args, **kwargs) - - _getter.__name__ = name - return _getter - - -for _name in _SETTERS: - setattr(_RecordingDataModel, _name, _make_setter(_name)) -for _name in _GETTERS: - setattr(_RecordingDataModel, _name, _make_getter(_name)) From 0ff49b03c84d31effd4262e463f9e67106ae671b Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Fri, 10 Jul 2026 15:51:53 -0500 Subject: [PATCH 3/3] Remove shadow state; guard recording coverage with a test The matrix setters kept a shadow dict (self.costs / self.transit_times) solely for the public duplicate-vehicle-type guard, which meant a future setter needing a set-time "already set?" check would silently need its own shadow. Drop the shadows: the matrix setters now auto-record like every other setter, and the guard reads the recorded calls via _recorded(). Add test_recording_covers_wrapper_surface so a new wrapper method that the recording layer does not handle (e.g. a mutator not named set_*/add_*) fails loudly instead of silently dropping its data. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Ramakrishna Prabhu --- python/cuopt/cuopt/routing/_recording.py | 30 +++++++++---------- python/cuopt/cuopt/routing/vehicle_routing.py | 6 ++-- .../cuopt/tests/routing/test_recording.py | 27 +++++++++++++++++ 3 files changed, 45 insertions(+), 18 deletions(-) create mode 100644 python/cuopt/cuopt/tests/routing/test_recording.py diff --git a/python/cuopt/cuopt/routing/_recording.py b/python/cuopt/cuopt/routing/_recording.py index d774f4f404..5e202774ae 100644 --- a/python/cuopt/cuopt/routing/_recording.py +++ b/python/cuopt/cuopt/routing/_recording.py @@ -11,9 +11,12 @@ installed automatically *from the wrapper* (see ``_install_methods``): every ``set_*``/``add_*`` becomes a recorder and every ``get_*`` a delegator. So there is a single source of truth -- adding a method to the wrapper/DataModel needs no -change here. Only methods that need special behavior are written out explicitly -below (the matrix setters track vehicle-type, the size scalars answer without a -build); those explicit definitions are left untouched by the auto-install. +change here. Only the size scalars are written out explicitly below (they answer +without a build); that explicit definition is left untouched by the auto-install. + +A public setter that needs to query prior state before build (e.g. a duplicate +guard) reads it from the recorded calls via ``_recorded`` -- there is no shadow +state to maintain per setter. """ # ``get_*`` methods on the wrapper that are helpers, not problem-data getters. @@ -30,19 +33,6 @@ def __init__(self, num_locations, fleet_size, n_orders=-1): self._init_args = (num_locations, fleet_size, n_orders) self._calls = [] self._built = None - # Track added matrix vehicle-types so the public layer's duplicate - # check (``if vehicle_type in self.costs``) works before build. - self.costs = {} - self.transit_times = {} - - # -- explicit matrix setters: also track the vehicle-type key -- - def add_cost_matrix(self, costs, vehicle_type=0): - self.costs[vehicle_type] = None - self._record("add_cost_matrix", (costs, vehicle_type), {}) - - def add_transit_time_matrix(self, times, vehicle_type=0): - self.transit_times[vehicle_type] = None - self._record("add_transit_time_matrix", (times, vehicle_type), {}) # -- size scalars: answered without building (queried during validation) -- def get_num_locations(self): @@ -60,6 +50,14 @@ def _record(self, name, args, kwargs): self._calls.append((name, args, kwargs)) self._built = None + def _recorded(self, name): + """Positional args of each prior recorded call to ``name``. + + Lets a public setter answer set-time "already set?" questions from the + recorded calls, so no per-setter shadow state is needed. + """ + return [args for call, args, _ in self._calls if call == name] + def _build(self): """Materialize the device (Cython) data model by replaying calls.""" if self._built is None: diff --git a/python/cuopt/cuopt/routing/vehicle_routing.py b/python/cuopt/cuopt/routing/vehicle_routing.py index 3bebf40b26..fc142325c0 100644 --- a/python/cuopt/cuopt/routing/vehicle_routing.py +++ b/python/cuopt/cuopt/routing/vehicle_routing.py @@ -135,7 +135,7 @@ def add_cost_matrix( >>> data_model.add_cost_matrix(cost_mat_car, 2) """ - if vehicle_type in self.costs: + if vehicle_type in {a[1] for a in self._recorded("add_cost_matrix")}: raise ValueError("Vehicle type matrix has already been added") if not skip_validation: @@ -217,7 +217,9 @@ def add_transit_time_matrix(self, mat, vehicle_type=0): >>> time_mat = cudf.DataFrame(time_mat) >>> data_model.add_transit_time_matrix(time_mat, 0) """ - if vehicle_type in self.transit_times: + if vehicle_type in { + a[1] for a in self._recorded("add_transit_time_matrix") + }: raise ValueError("Vehicle type matrix has already been added") validate_matrix(mat, "transit time matrix", self.get_num_locations()) diff --git a/python/cuopt/cuopt/tests/routing/test_recording.py b/python/cuopt/cuopt/tests/routing/test_recording.py new file mode 100644 index 0000000000..81576191eb --- /dev/null +++ b/python/cuopt/cuopt/tests/routing/test_recording.py @@ -0,0 +1,27 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from cuopt import routing +from cuopt.routing import vehicle_routing_wrapper +from cuopt.routing._recording import _SKIP_GETTERS + + +def test_recording_covers_wrapper_surface(): + """Every public wrapper DataModel method must be handled by the recording + layer (installed as a recorder/getter or an explicit override). This fails + loudly if a new wrapper method -- e.g. a mutator not named set_*/add_* -- + is added without being recorded, instead of silently dropping its data. + """ + dm = routing.DataModel(1, 1) # triggers _install_methods + handled = set(dir(type(dm))) + missing = [ + name + for name in dir(vehicle_routing_wrapper.DataModel) + if not name.startswith("_") + and name not in _SKIP_GETTERS + and name not in handled + ] + assert not missing, ( + f"recording layer does not handle wrapper methods {missing}; " + "add a recorder/getter or list them in _SKIP_GETTERS" + )