Fix ctypes memory safety and enrich tests

.github/workflows/ci.yml

@@ -17,7 +17,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ "ubuntu-latest", "windows-2019", "macos-latest" ]
-        python-version: [ "3.11", "3.10" ]
+        python-version: [ "3.10", "3.13" ]
 
     steps:
       - uses: actions/checkout@v2

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "hygese"
-version = "0.0.0.11"
+version = "0.1.0"
 authors = [{name = "Changhyun Kwon"}]
 description = "Wrapper for the Hybrid Genetic Search algorithm for Capacitated Vehicle Routing Problems (HGS-CVRP)"
 readme = {file = "README.md", content-type = "text/markdown"}

src/hygese/hygese.py

@@ -8,7 +8,6 @@
     c_double,
     c_char,
     sizeof,
-    cast,
     byref,
 )
 from dataclasses import dataclass
@@ -287,10 +286,13 @@ def _solve_cvrp(
         verbose: bool,
     ):
         n_nodes = x_coords.size
-        x_ct = x_coords.astype(c_double).ctypes
-        y_ct = y_coords.astype(c_double).ctypes
-        s_ct = service_times.astype(c_double).ctypes
-        d_ct = demand.astype(c_double).ctypes
+        # Bind each contiguous float64 view to a local so the buffer is pinned
+        # for the full duration of the C call. ascontiguousarray copies only
+        # when dtype/layout already mismatches the C expectation.
+        x = np.ascontiguousarray(x_coords, dtype=np.float64)
+        y = np.ascontiguousarray(y_coords, dtype=np.float64)
+        s = np.ascontiguousarray(service_times, dtype=np.float64)
+        d = np.ascontiguousarray(demand, dtype=np.float64)
         ap_ct = algorithm_parameters.ctypes
 
         # struct Solution * solve_cvrp(
@@ -299,10 +301,10 @@ def _solve_cvrp(
         # 	int max_nbVeh, const struct AlgorithmParameters* ap, char verbose);
         sol_p = self._c_api_solve_cvrp(
             n_nodes,
-            cast(x_ct, c_double_p),
-            cast(y_ct, c_double_p),
-            cast(s_ct, c_double_p),
-            cast(d_ct, c_double_p),
+            x.ctypes.data_as(c_double_p),
+            y.ctypes.data_as(c_double_p),
+            s.ctypes.data_as(c_double_p),
+            d.ctypes.data_as(c_double_p),
             vehicle_capacity,
             duration_limit,
             is_rounding_integer,
@@ -312,8 +314,11 @@ def _solve_cvrp(
             verbose,
         )
 
-        result = RoutingSolution(sol_p)
-        self._c_api_delete_sol(sol_p)
+        try:
+            result = RoutingSolution(sol_p)
+        finally:
+            if sol_p:
+                self._c_api_delete_sol(sol_p)
         return result
 
     def _solve_cvrp_dist_mtx(
@@ -332,12 +337,13 @@ def _solve_cvrp_dist_mtx(
     ):
         n_nodes = x_coords.size
 
-        x_ct = x_coords.astype(c_double).ctypes
-        y_ct = y_coords.astype(c_double).ctypes
-        s_ct = service_times.astype(c_double).ctypes
-        d_ct = demand.astype(c_double).ctypes
-
-        m_ct = dist_mtx.reshape(n_nodes * n_nodes).astype(c_double).ctypes
+        x = np.ascontiguousarray(x_coords, dtype=np.float64)
+        y = np.ascontiguousarray(y_coords, dtype=np.float64)
+        s = np.ascontiguousarray(service_times, dtype=np.float64)
+        d = np.ascontiguousarray(demand, dtype=np.float64)
+        # A 2-D contiguous float64 array's data buffer is already a flat n*n
+        # block of doubles, so no extra reshape/ravel is needed.
+        m = np.ascontiguousarray(dist_mtx, dtype=np.float64)
         ap_ct = algorithm_parameters.ctypes
 
         # struct Solution *solve_cvrp_dist_mtx(
@@ -346,11 +352,11 @@ def _solve_cvrp_dist_mtx(
         # 	int max_nbVeh, const struct AlgorithmParameters *ap, char verbose);
         sol_p = self._c_api_solve_cvrp_dist_mtx(
             n_nodes,
-            cast(x_ct, c_double_p),
-            cast(y_ct, c_double_p),
-            cast(m_ct, c_double_p),
-            cast(s_ct, c_double_p),
-            cast(d_ct, c_double_p),
+            x.ctypes.data_as(c_double_p),
+            y.ctypes.data_as(c_double_p),
+            m.ctypes.data_as(c_double_p),
+            s.ctypes.data_as(c_double_p),
+            d.ctypes.data_as(c_double_p),
             vehicle_capacity,
             duration_limit,
             is_duration_constraint,
@@ -359,6 +365,9 @@ def _solve_cvrp_dist_mtx(
             verbose,
         )
 
-        result = RoutingSolution(sol_p)
-        self._c_api_delete_sol(sol_p)
+        try:
+            result = RoutingSolution(sol_p)
+        finally:
+            if sol_p:
+                self._c_api_delete_sol(sol_p)
         return result

src/hygese/tests/conftest.py

@@ -0,0 +1,47 @@
+import pytest
+
+from hygese import AlgorithmParameters
+
+
+_OR_TOOLS_DISTANCE_MATRIX = [
+    [0, 548, 776, 696, 582, 274, 502, 194, 308, 194, 536, 502, 388, 354, 468, 776, 662],
+    [548, 0, 684, 308, 194, 502, 730, 354, 696, 742, 1084, 594, 480, 674, 1016, 868, 1210],
+    [776, 684, 0, 992, 878, 502, 274, 810, 468, 742, 400, 1278, 1164, 1130, 788, 1552, 754],
+    [696, 308, 992, 0, 114, 650, 878, 502, 844, 890, 1232, 514, 628, 822, 1164, 560, 1358],
+    [582, 194, 878, 114, 0, 536, 764, 388, 730, 776, 1118, 400, 514, 708, 1050, 674, 1244],
+    [274, 502, 502, 650, 536, 0, 228, 308, 194, 240, 582, 776, 662, 628, 514, 1050, 708],
+    [502, 730, 274, 878, 764, 228, 0, 536, 194, 468, 354, 1004, 890, 856, 514, 1278, 480],
+    [194, 354, 810, 502, 388, 308, 536, 0, 342, 388, 730, 468, 354, 320, 662, 742, 856],
+    [308, 696, 468, 844, 730, 194, 194, 342, 0, 274, 388, 810, 696, 662, 320, 1084, 514],
+    [194, 742, 742, 890, 776, 240, 468, 388, 274, 0, 342, 536, 422, 388, 274, 810, 468],
+    [536, 1084, 400, 1232, 1118, 582, 354, 730, 388, 342, 0, 878, 764, 730, 388, 1152, 354],
+    [502, 594, 1278, 514, 400, 776, 1004, 468, 810, 536, 878, 0, 114, 308, 650, 274, 844],
+    [388, 480, 1164, 628, 514, 662, 890, 354, 696, 422, 764, 114, 0, 194, 536, 388, 730],
+    [354, 674, 1130, 822, 708, 628, 856, 320, 662, 388, 730, 308, 194, 0, 342, 422, 536],
+    [468, 1016, 788, 1164, 1050, 514, 514, 662, 320, 274, 388, 650, 536, 342, 0, 764, 194],
+    [776, 868, 1552, 560, 674, 1050, 1278, 742, 1084, 810, 1152, 274, 388, 422, 764, 0, 798],
+    [662, 1210, 754, 1358, 1244, 708, 480, 856, 514, 468, 354, 844, 730, 536, 194, 798, 0],
+]
+_OR_TOOLS_DEMANDS = [0, 1, 1, 2, 4, 2, 4, 8, 8, 1, 2, 1, 2, 4, 4, 8, 8]
+
+
+@pytest.fixture
+def or_tools_data():
+    """The 17-node CVRP instance from the OR-Tools VRP tutorial.
+
+    Returns a fresh dict each call so tests can mutate it without bleeding into
+    each other (note: Solver.solve_tsp also mutates the dict it receives).
+    """
+    return {
+        "distance_matrix": [row[:] for row in _OR_TOOLS_DISTANCE_MATRIX],
+        "num_vehicles": 4,
+        "depot": 0,
+        "demands": list(_OR_TOOLS_DEMANDS),
+        "vehicle_capacity": 15,
+    }
+
+
+@pytest.fixture
+def quick_ap():
+    """AlgorithmParameters tuned for fast, deterministic runs (no optimum-cost asserts)."""
+    return AlgorithmParameters(timeLimit=0.5, seed=0)

src/hygese/tests/test_cvrp.py

@@ -107,3 +107,34 @@ def test_cvrp_duration():
 
     result = hgs_solver.solve_cvrp(data, rounding=True)
     assert result.cost == 42
+
+
+def test_routing_solution_invariants():
+    """A feasible CVRP solution must visit every customer exactly once,
+    keep the depot out of route bodies, respect vehicle count, and not
+    exceed per-vehicle capacity.
+    """
+    data = get_data()
+    n_nodes = len(data['demands'])
+
+    ap = AlgorithmParameters(timeLimit=1.1)
+    result = Solver(ap, verbose=False).solve_cvrp(data)
+
+    assert result.cost > 0
+    assert result.n_routes >= 1
+    assert result.n_routes <= data['num_vehicles']
+    assert len(result.routes) == result.n_routes
+
+    visited = [node for route in result.routes for node in route]
+    assert sorted(visited) == list(range(1, n_nodes)), (
+        f"Each customer 1..{n_nodes - 1} must appear exactly once across "
+        f"all routes (and depot 0 must not appear); got {sorted(visited)}"
+    )
+
+    capacity = data['vehicle_capacity']
+    demand = data['demands']
+    for route in result.routes:
+        load = sum(demand[i] for i in route)
+        assert load <= capacity, (
+            f"Route {route} has total demand {load} > capacity {capacity}"
+        )

src/hygese/tests/test_inputs.py

@@ -0,0 +1,118 @@
+"""Input-variant and validation tests for hygese.Solver.solve_cvrp.
+
+Variant tests pin down the dtype / contiguity branches of np.ascontiguousarray
+in _solve_cvrp and _solve_cvrp_dist_mtx. Validation tests pin down the
+explicit ValueError / assertion guards in solve_cvrp.
+"""
+
+import numpy as np
+import pytest
+
+from hygese import AlgorithmParameters, Solver
+
+
+# --- helpers ---------------------------------------------------------------
+
+
+def _tiny_coord_data():
+    """A tiny coord-based CVRP instance used by variant tests that need coords."""
+    return {
+        "x_coordinates": [10.0, 20.0, 30.0, 40.0, 50.0],
+        "y_coordinates": [50.0, 40.0, 30.0, 20.0, 10.0],
+        "demands": [0, 2, 3, 1, 2],
+        "vehicle_capacity": 5,
+        "depot": 0,
+    }
+
+
+# --- dtype / contiguity correctness ----------------------------------------
+
+
+def test_float32_inputs_match_float64(quick_ap):
+    solver = Solver(quick_ap, verbose=False)
+
+    base = _tiny_coord_data()
+    f64 = {
+        **base,
+        "x_coordinates": np.asarray(base["x_coordinates"], dtype=np.float64),
+        "y_coordinates": np.asarray(base["y_coordinates"], dtype=np.float64),
+        "demands": np.asarray(base["demands"], dtype=np.float64),
+    }
+    f32 = {
+        **base,
+        "x_coordinates": np.asarray(base["x_coordinates"], dtype=np.float32),
+        "y_coordinates": np.asarray(base["y_coordinates"], dtype=np.float32),
+        "demands": np.asarray(base["demands"], dtype=np.float32),
+    }
+
+    cost_f64 = solver.solve_cvrp(f64).cost
+    cost_f32 = solver.solve_cvrp(f32).cost
+    assert cost_f64 == cost_f32
+
+
+def test_non_contiguous_distance_matrix(or_tools_data, quick_ap):
+    """A non-C-contiguous (F-contiguous transpose of a symmetric matrix) view
+    must produce the same cost as the contiguous original — ascontiguousarray
+    is responsible for the layout copy.
+    """
+    solver = Solver(quick_ap, verbose=False)
+
+    base = or_tools_data
+    cost_contig = solver.solve_cvrp(base).cost
+
+    m = np.asarray(base["distance_matrix"], dtype=np.float64)
+    assert np.array_equal(m, m.T), "OR-Tools matrix should be symmetric"
+    non_contig = m.T  # F-contiguous view, same numerical content
+    assert not non_contig.flags["C_CONTIGUOUS"]
+
+    variant = {**base, "distance_matrix": non_contig}
+    cost_view = solver.solve_cvrp(variant).cost
+
+    assert cost_contig == cost_view
+
+
+def test_int_demands_accepted(quick_ap):
+    """np.int64 demands must round-trip through ascontiguousarray(float64)."""
+    solver = Solver(quick_ap, verbose=False)
+
+    data = _tiny_coord_data()
+    data["demands"] = np.array(data["demands"], dtype=np.int64)
+
+    result = solver.solve_cvrp(data)
+    assert result.cost > 0
+    assert result.n_routes >= 1
+
+
+def test_list_coordinates_accepted(quick_ap):
+    """Plain Python list coordinates must work through asarray -> ascontiguousarray."""
+    solver = Solver(quick_ap, verbose=False)
+
+    result = solver.solve_cvrp(_tiny_coord_data())
+    assert result.cost > 0
+    assert result.n_routes >= 1
+
+
+# --- input validation ------------------------------------------------------
+
+
+def test_invalid_depot_raises_value_error(or_tools_data, quick_ap):
+    solver = Solver(quick_ap, verbose=False)
+    or_tools_data["depot"] = 1
+    with pytest.raises(ValueError, match="depot location must be 0"):
+        solver.solve_cvrp(or_tools_data)
+
+
+def test_mismatched_lengths_raises_assertion(or_tools_data, quick_ap):
+    """Coords explicitly shorter than demands must trip the length assertion."""
+    solver = Solver(quick_ap, verbose=False)
+    or_tools_data["x_coordinates"] = [0.0, 1.0]
+    or_tools_data["y_coordinates"] = [0.0, 1.0]
+    with pytest.raises(AssertionError):
+        solver.solve_cvrp(or_tools_data)
+
+
+def test_negative_demand_raises_assertion(or_tools_data, quick_ap):
+    solver = Solver(quick_ap, verbose=False)
+    or_tools_data["demands"][1] = -1
+    with pytest.raises(AssertionError):
+        solver.solve_cvrp(or_tools_data)

src/hygese/tests/test_memory_safety.py

@@ -0,0 +1,51 @@
+"""Regression tests for the ctypes memory-safety fixes in hygese.Solver.
+
+- delete_solution must run even when RoutingSolution extraction raises
+  (the `try/finally` block in _solve_cvrp / _solve_cvrp_dist_mtx).
+- A single Solver must be reusable: each call gets its own ctypes buffers
+  and produces a clean result.
+"""
+
+import pytest
+
+import hygese.hygese as _hg
+from hygese import AlgorithmParameters, Solver
+
+
+def test_delete_called_on_exception(or_tools_data, quick_ap, monkeypatch):
+    solver = Solver(quick_ap, verbose=False)
+
+    delete_calls = []
+    original_delete = solver._c_api_delete_sol
+
+    def counting_delete(ptr):
+        delete_calls.append(bool(ptr))
+        return original_delete(ptr)
+
+    solver._c_api_delete_sol = counting_delete
+
+    def boom(self, sol_ptr):
+        raise RuntimeError("simulated extraction failure")
+
+    monkeypatch.setattr(_hg.RoutingSolution, "__init__", boom)
+
+    with pytest.raises(RuntimeError, match="simulated extraction failure"):
+        solver.solve_cvrp(or_tools_data)
+
+    assert delete_calls == [True], (
+        "delete_solution must be called exactly once with a non-null pointer "
+        "when RoutingSolution.__init__ raises"
+    )
+
+
+def test_solver_reuse(or_tools_data):
+    ap = AlgorithmParameters(timeLimit=0.5, seed=42)
+    solver = Solver(ap, verbose=False)
+
+    costs = [solver.solve_cvrp(or_tools_data).cost for _ in range(3)]
+
+    assert all(c == costs[0] for c in costs), (
+        f"Reusing a Solver with seed=42 should give identical costs across "
+        f"calls; got {costs}"
+    )
+    assert costs[0] > 0