Repository: PyVRP/PyVRP
Pull request #757 (2025-03-12 08:12:09)

Create Solution object in `read_solution`
Small QOL improvement to `read_solution` that came up when addressing https://github.com/PyVRP/Bench/pull/9#discussion_r1989469850.

<details>

**Notes**:

Please read our [contributing guidelines](https://pyvrp.org/dev/contributing.html) first.
In particular:

- You must add tests when making code changes.
  This keeps the code coverage level up, and helps ensure the changes work as intended.
- When fixing a bug, you must add a test that would produce the bug in the master branch, and then show that it is fixed with the new code. 
- New code additions must be well formatted. Changes should pass the pre-commit workflow, which you can set up locally using [pre-commit](https://pre-commit.com/#intro). 
- Docstring additions must render correctly, including escapes and LaTeX.
- Finally, it is essential that all contributions in this PR are license-compatible with PyVRP's MIT license.
  Please check that this PR can be included into PyVRP under the MIT license.

</details>


codspeed-hq[bot]: <!-- __CODSPEED_PERFORMANCE_REPORT_COMMENT__ -->
<!-- __CODSPEED_INSTRUMENTATION_PERFORMANCE_REPORT_COMMENT__ -->

## [CodSpeed Performance Report](https://codspeed.io/PyVRP/PyVRP/branches/read-solution)

### Merging #757 will **not alter performance**

<sub>Comparing <code>read-solution</code> (93155d3) with <code>main</code> (af0e481)</sub>



### Summary

`✅ 43` untouched benchmarks  




codecov[bot]: ## [Codecov](https://app.codecov.io/gh/PyVRP/PyVRP/pull/757?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PyVRP) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 96.53%. Comparing base [(`af0e481`)](https://app.codecov.io/gh/PyVRP/PyVRP/commit/af0e481d907d7e4c45562160a19f30b80372457c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PyVRP) to head [(`93155d3`)](https://app.codecov.io/gh/PyVRP/PyVRP/commit/93155d3a620b771139276660694c30755e5383da?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PyVRP).

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main     #757   +/-   ##
=======================================
  Coverage   96.53%   96.53%           
=======================================
  Files          76       76           
  Lines        3866     3869    +3     
  Branches     1061     1061           
=======================================
+ Hits         3732     3735    +3     
+ Misses         15       14    -1     
- Partials      119      120    +1     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/PyVRP/PyVRP/pull/757?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PyVRP).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PyVRP).

<details><summary>🚀 New features to boost your workflow: </summary>

- ❄ [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
N-Wouda: LGTM!

Patch:
diff --git a/pyvrp/read.py b/pyvrp/read.py
index d27562e..2846f17 100644
--- a/pyvrp/read.py
+++ b/pyvrp/read.py
@@ -8,11 +8,18 @@ from warnings import warn
 import numpy as np
 import vrplib
 
-from pyvrp._pyvrp import Client, ClientGroup, Depot, ProblemData, VehicleType
+from pyvrp._pyvrp import (
+    Client,
+    ClientGroup,
+    Depot,
+    ProblemData,
+    Route,
+    Solution,
+    VehicleType,
+)
 from pyvrp.constants import MAX_VALUE
 from pyvrp.exceptions import ScalingWarning
 
-_Routes = list[list[int]]
 _RoundingFunc = Callable[[np.ndarray], np.ndarray]
 
 _INT_MAX = np.iinfo(np.int64).max
@@ -83,24 +90,38 @@ def read(
     return builder.data()
 
 
-def read_solution(where: str | pathlib.Path) -> _Routes:
+def read_solution(where: str | pathlib.Path, data: ProblemData) -> Solution:
     """
     Reads a solution in ``VRPLIB`` format from the give file location, and
-    returns the routes contained in it.
+    returns the corresponding Solution object.
 
     Parameters
     ----------
     where
         File location to read. Assumes the solution in the file on the given
         location is in ``VRPLIB`` solution format.
+    data
+        Problem data instance that the solution is based on.
 
     Returns
     -------
-    list
-        List of routes, where each route is a list of client numbers.
+    Solution
+        Solution object constructed from the read data.
     """
     sol = vrplib.read_solution(str(where))
-    return sol["routes"]  # type: ignore
+
+    # We assume that the routes are listed in order of vehicle types as
+    # determined by ``read()``.
+    veh2type = []
+    for idx, veh_type in enumerate(data.vehicle_types()):
+        veh2type.extend([idx] * veh_type.num_available)
+
+    routes = [
+        Route(data, visits, veh2type[idx])
+        for idx, visits in enumerate(sol["routes"])
+        if visits
+    ]
+    return Solution(data, routes)
 
 
 class _InstanceParser:


Test patch:
diff --git a/benchmarks/conftest.py b/benchmarks/conftest.py
index 7988b6e..02bcf36 100644
--- a/benchmarks/conftest.py
+++ b/benchmarks/conftest.py
@@ -9,8 +9,8 @@ def vrptw():
 
 
 @pytest.fixture(scope="session")
-def vrptw_bks():
-    return read_solution("data/RC208.sol")
+def vrptw_bks(vrptw):
+    return read_solution("data/RC208.sol", vrptw)
 
 
 @pytest.fixture(scope="session")
diff --git a/benchmarks/search/test_LocalSearch.py b/benchmarks/search/test_LocalSearch.py
index 50d0d34..c39a5f7 100644
--- a/benchmarks/search/test_LocalSearch.py
+++ b/benchmarks/search/test_LocalSearch.py
@@ -44,9 +44,8 @@ def test_all_operators_on_vrptw_from_bks(benchmark, vrptw, vrptw_bks):
     for route_op in ROUTE_OPERATORS:
         ls.add_route_operator(route_op(vrptw))
 
-    sol = Solution(vrptw, vrptw_bks)
     cost_evaluator = CostEvaluator([20], 6, 6)
-    benchmark(ls, sol, cost_evaluator)
+    benchmark(ls, vrptw_bks, cost_evaluator)
 
 
 @pytest.mark.parametrize("node_op", NODE_OPERATORS)
diff --git a/tests/data/OkSmall.sol b/tests/data/OkSmall.sol
new file mode 100644
index 0000000..a8c4b60
--- /dev/null
+++ b/tests/data/OkSmall.sol
@@ -0,0 +1,2 @@
+Route #1: 1 2
+Route #2: 3 4
diff --git a/tests/data/OkSmallMultipleDepots.sol b/tests/data/OkSmallMultipleDepots.sol
new file mode 100644
index 0000000..e475b68
--- /dev/null
+++ b/tests/data/OkSmallMultipleDepots.sol
@@ -0,0 +1,3 @@
+Route #1: 2
+Route #2:
+Route #3: 3 4
diff --git a/tests/helpers.py b/tests/helpers.py
index e77d9c8..df14964 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -19,7 +19,6 @@ def read(where: str, *args, **kwargs):
     return _read(this_dir / where, *args, **kwargs)
 
 
-@lru_cache
 def read_solution(where: str, *args, **kwargs):
     """
     Lightweight wrapper around ``pyvrp.read.read_solution()``, reading solution
diff --git a/tests/plotting/test_plotting.py b/tests/plotting/test_plotting.py
index 9b48008..dedccfd 100644
--- a/tests/plotting/test_plotting.py
+++ b/tests/plotting/test_plotting.py
@@ -28,9 +28,7 @@ def test_plot_solution():
     Compares plots generated by ``plot_solution``.
     """
     data = read("data/RC208.vrp", round_func="trunc")
-    bks = read_solution("data/RC208.sol")
-
-    sol = Solution(data, bks)
+    sol = read_solution("data/RC208.sol", data)
 
     plotting.plot_solution(sol, data)
     plotting.plot_solution(sol, data, plot_clients=True)
@@ -77,7 +75,7 @@ def test_plot_result():
     num_iterations = 100
 
     data = read("data/RC208.vrp", round_func="trunc")
-    bks = read_solution("data/RC208.sol")
+    bks = read_solution("data/RC208.sol", data)
     cost_evaluator = CostEvaluator([20], 6, 0)
     rng = RandomNumberGenerator(seed=42)
 
@@ -91,7 +89,7 @@ def test_plot_result():
     for i in range(num_iterations):
         if i == num_iterations // 2:
             # Make sure we insert a feasible solution
-            sol = Solution(data, bks)
+            sol = bks
         else:
             sol = Solution.make_random(data, rng)
 
@@ -101,7 +99,7 @@ def test_plot_result():
         # Hacky to produce deterministic result
         stats.runtimes[-1] = i % 3
 
-    res = Result(Solution(data, bks), stats, num_iterations, 0.0)
+    res = Result(bks, stats, num_iterations, 0.0)
     plotting.plot_result(res, data)
 
 
@@ -130,8 +128,7 @@ def test_plot_route_schedule():
     Compares plots generated by ``plot_route_schedule``.
     """
     data = read("data/RC208.vrp", round_func="trunc")
-    bks = read_solution("data/RC208.sol")
-    sol = Solution(data, bks)
+    sol = read_solution("data/RC208.sol", data)
     plotting.plot_route_schedule(data, sol.routes()[0])
 
 
diff --git a/tests/test_GeneticAlgorithm.py b/tests/test_GeneticAlgorithm.py
index 430cbe4..4cfb940 100644
--- a/tests/test_GeneticAlgorithm.py
+++ b/tests/test_GeneticAlgorithm.py
@@ -116,7 +116,7 @@ def test_initial_solutions_added_when_restarting(rc208):
 
     # We use the best known solution as one of the initial solutions so that
     # there are no improving iterations.
-    init = {Solution(rc208, read_solution("data/RC208.sol"))}
+    init = {read_solution("data/RC208.sol", rc208)}
     init.update(Solution.make_random(rc208, rng) for _ in range(24))
 
     params = GeneticAlgorithmParams(
@@ -177,7 +177,7 @@ def test_best_initial_solution(rc208):
     pop = Population(bpd)
 
     init = [Solution.make_random(rc208, rng) for _ in range(24)]
-    bks = Solution(rc208, read_solution("data/RC208.sol"))
+    bks = read_solution("data/RC208.sol", rc208)
     init.append(bks)
 
     ls = LocalSearch(rc208, rng, compute_neighbours(rc208))
@@ -195,7 +195,7 @@ def test_infeasible_offspring_is_repaired(rc208):
     Tests that infeasible offspring will be repaired if the repair probability
     is 1.
     """
-    bks = Solution(rc208, read_solution("data/RC208.sol"))
+    bks = read_solution("data/RC208.sol", rc208)
 
     pm = PenaltyManager(initial_penalties=([20], 6, 6))
     rng = RandomNumberGenerator(seed=42)
diff --git a/tests/test_read.py b/tests/test_read.py
index 7b71c93..c51fe09 100644
--- a/tests/test_read.py
+++ b/tests/test_read.py
@@ -12,7 +12,7 @@ from numpy.testing import (
 
 from pyvrp.constants import MAX_VALUE
 from pyvrp.exceptions import ScalingWarning
-from tests.helpers import read
+from tests.helpers import read, read_solution
 
 
 @pytest.mark.parametrize(
@@ -502,3 +502,38 @@ def test_sdvrptw_instance():
         assert_equal(distance_matrix[client, idcs], MAX_VALUE)
         assert_equal(duration_matrix[idcs, client], MAX_VALUE)
         assert_equal(duration_matrix[client, idcs], MAX_VALUE)
+
+
+def test_read_solution_single_vehicle_type(ok_small):
+    """
+    Tests that reading a solution with a single vehicle type works correctly.
+    """
+    solution = read_solution("data/OkSmall.sol", ok_small)
+    routes = solution.routes()
+
+    assert_equal(routes[0].visits(), [1, 2])
+    assert_equal(routes[1].visits(), [3, 4])
+
+    assert_equal(routes[0].vehicle_type(), 0)
+    assert_equal(routes[1].vehicle_type(), 0)
+
+
+def test_read_solution_multiple_vehicle_types(ok_small_multi_depot):
+    """
+    Tests that reading a solution with multiple vehicle types works correctly.
+    """
+    solution = read_solution(
+        "data/OkSmallMultipleDepots.sol", ok_small_multi_depot
+    )
+    routes = solution.routes()
+
+    # The solution file shows three routes, but empty routes are ignored.
+    assert_equal(solution.num_routes(), 2)
+    assert_equal(routes[0].visits(), [2])
+    assert_equal(routes[1].visits(), [3, 4])
+
+    # The instance has two vehicle types: two of the first type and one of the
+    # second type. Because the second route was empty, the second vehicle of
+    # the first type is not used.
+    assert_equal(routes[0].vehicle_type(), 0)
+    assert_equal(routes[1].vehicle_type(), 1)