Repository: PyVRP/PyVRP (python)

Issue:
Should we collect statistics by default?
Issues based on the other PR linked from below [@leonlan]. Right now we do not collect statistics by default, citing performance reasons. Have we actually checked whether it matters? If it doesn't I think we should collect statistics by default, since they are useful to have.

### Benchmarks

Current main (698a9d70eea87ad35c1a9e0df533896169c2aa52; job ID 3067936; 10x seeds/1h with `configs/vrptw.toml`; no stats):
```
 Avg. objective: 333402.8 (min: 333220, max: 333486)
Avg. iterations: 86955.2 (min: 67928, max: 97156)
```
Current main (fe6c756f0ef981a9efe39f8cc17306b667744ebf; job ID 3069860; 10x seeds/1h with `configs/vrptw.toml`; stats):
```
 Avg. objective: 333414.6 (min: 333218, max: 333644)
Avg. iterations: 83515.8 (min: 68249, max: 94540)
```



Pull request:
Collect statistics by default
This PR:

- [x] Closes #278.
- [ ] Adds and passes relevant tests.
- [x] Has well-formatted code and documentation.

<details>

**Notes**:

- It is essential that you add a test 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.

</details>


Patch:
diff --git a/configs/cvrp.toml b/configs/cvrp.toml
index 4685819..cc59cce 100644
--- a/configs/cvrp.toml
+++ b/configs/cvrp.toml
@@ -20,7 +20,6 @@ route_ops = [
 
 
 [genetic]
-collect_statistics = false
 repair_probability = 0.50
 intensify_probability = 1.00
 intensify_on_best = false
diff --git a/configs/vrptw.toml b/configs/vrptw.toml
index d6adb96..153d9ed 100644
--- a/configs/vrptw.toml
+++ b/configs/vrptw.toml
@@ -20,7 +20,6 @@ route_ops = [
 
 
 [genetic]
-collect_statistics = false
 repair_probability = 0.80
 intensify_probability = 0.15
 intensify_on_best = true
diff --git a/docs/source/api/pyvrp.rst b/docs/source/api/pyvrp.rst
index 2255a8b..4413bbd 100644
--- a/docs/source/api/pyvrp.rst
+++ b/docs/source/api/pyvrp.rst
@@ -89,8 +89,6 @@ This object can be used to obtain the best observed solution, and detailed runti
 
 .. automodule:: pyvrp.exceptions
 
-   .. autoexception:: StatisticsNotCollectedError
-
    .. autoexception:: EmptySolutionWarning
 
    .. autoexception:: ScalingWarning
diff --git a/examples/using_pyvrp_components.ipynb b/examples/using_pyvrp_components.ipynb
index 112bb8d..6d25e07 100644
--- a/examples/using_pyvrp_components.ipynb
+++ b/examples/using_pyvrp_components.ipynb
@@ -478,52 +478,7 @@
    "cell_type": "markdown",
    "metadata": {},
    "source": [
-    "PyVRP also provides many plotting tools that can be used to investigate a data instance or solution result.\n",
-    "To use most of these, we need to collect runtime statistics.\n",
-    "This is disabled by default because it has a slight performance penalty.\n",
-    "Let's enable statistics collection, and look at some results!"
-   ]
-  },
-  {
-   "cell_type": "code",
-   "execution_count": null,
-   "metadata": {},
-   "outputs": [],
-   "source": [
-    "from pyvrp import GeneticAlgorithmParams\n",
-    "\n",
-    "algo_params = GeneticAlgorithmParams(collect_statistics=True)\n",
-    "algo = GeneticAlgorithm(\n",
-    "    INSTANCE,\n",
-    "    pen_manager,\n",
-    "    rng,\n",
-    "    pop,\n",
-    "    ls,\n",
-    "    srex,\n",
-    "    init_sols,\n",
-    "    algo_params,\n",
-    ")"
-   ]
-  },
-  {
-   "attachments": {},
-   "cell_type": "markdown",
-   "metadata": {},
-   "source": [
-    "Many objects can be customised using such a parameters object, although the defaults are typically already very good choices.\n",
-    "Note that the `Model.solve` method does collect statistics by default to provide maximum utility.\n",
-    "\n",
-    "We are now ready to solve the instance again, and then plot some of the results."
-   ]
-  },
-  {
-   "cell_type": "code",
-   "execution_count": null,
-   "metadata": {},
-   "outputs": [],
-   "source": [
-    "res = algo.run(stop=MaxIterations(500))\n",
-    "print(res)"
+    "PyVRP also provides many plotting tools that can be used to investigate a data instance or solution result."
    ]
   },
   {
diff --git a/pyvrp/GeneticAlgorithm.py b/pyvrp/GeneticAlgorithm.py
index 94526ba..0e72857 100644
--- a/pyvrp/GeneticAlgorithm.py
+++ b/pyvrp/GeneticAlgorithm.py
@@ -24,7 +24,6 @@ if TYPE_CHECKING:
 @dataclass
 class GeneticAlgorithmParams:
     repair_probability: float = 0.80
-    collect_statistics: bool = False
     intensify_probability: float = 0.15
     intensify_on_best: bool = True
     nb_iter_no_improvement: int = 20_000
@@ -148,8 +147,7 @@ class GeneticAlgorithm:
             else:
                 iters_no_improvement += 1
 
-            if self._params.collect_statistics:
-                stats.collect_from(self._pop, self._cost_evaluator)
+            stats.collect_from(self._pop, self._cost_evaluator)
 
         end = time.perf_counter() - start
         return Result(self._best, stats, iters, end)
diff --git a/pyvrp/Model.py b/pyvrp/Model.py
index 24ef948..7465bab 100644
--- a/pyvrp/Model.py
+++ b/pyvrp/Model.py
@@ -3,7 +3,7 @@ from warnings import warn
 
 import numpy as np
 
-from pyvrp.GeneticAlgorithm import GeneticAlgorithm, GeneticAlgorithmParams
+from pyvrp.GeneticAlgorithm import GeneticAlgorithm
 from pyvrp.PenaltyManager import PenaltyManager
 from pyvrp.Population import Population, PopulationParams
 from pyvrp.Result import Result
@@ -280,7 +280,6 @@ class Model:
             for _ in range(pop_params.min_pop_size)
         ]
 
-        gen_params = GeneticAlgorithmParams(collect_statistics=True)
-        gen_args = (data, pm, rng, pop, ls, srex, init, gen_params)
+        gen_args = (data, pm, rng, pop, ls, srex, init)
         algo = GeneticAlgorithm(*gen_args)  # type: ignore
         return algo.run(stop)
diff --git a/pyvrp/Result.py b/pyvrp/Result.py
index e65e0bc..efa468d 100644
--- a/pyvrp/Result.py
+++ b/pyvrp/Result.py
@@ -16,8 +16,7 @@ class Result:
     best
         The best observed solution.
     stats
-        A Statistics object containing runtime statistics. These are only
-        collected and available if statistics were collected for the given run.
+        A Statistics object containing runtime statistics.
     num_iterations
         Number of iterations performed by the genetic algorithm.
     runtime
@@ -53,6 +52,7 @@ class Result:
         """
         if not self.best.is_feasible():
             return math.inf
+
         return CostEvaluator().cost(self.best)
 
     def is_feasible(self) -> bool:
@@ -66,21 +66,6 @@ class Result:
         """
         return self.best.is_feasible()
 
-    def has_statistics(self) -> bool:
-        """
-        Returns whether detailed statistics were collected. If statistics are
-        not available, the plotting methods cannot be used.
-
-        Returns
-        -------
-        bool
-            True when detailed statistics are available, False otherwise.
-        """
-        return (
-            self.num_iterations > 0
-            and self.num_iterations == self.stats.num_iterations
-        )
-
     def __str__(self) -> str:
         obj_str = f"{self.cost():.2f}" if self.is_feasible() else "INFEASIBLE"
         summary = [
diff --git a/pyvrp/cli.py b/pyvrp/cli.py
index cacee89..e086011 100644
--- a/pyvrp/cli.py
+++ b/pyvrp/cli.py
@@ -100,8 +100,7 @@ def solve(
         Maximum number of iterations for solving. Either ``max_runtime`` or
         ``max_iterations`` must be specified.
     stats_dir
-        The directory to write runtime statistics to. Enables statistics
-        collection when passed.
+        The directory to write runtime statistics to.
     sol_dir
         The directory to write the best found solutions to.
 
@@ -117,12 +116,6 @@ def solve(
         config = {}
 
     gen_params = GeneticAlgorithmParams(**config.get("genetic", {}))
-
-    if stats_dir:
-        # The statistics directory argument trumps whatever we got earlier
-        # from the configuration file.
-        gen_params.collect_statistics = True
-
     pen_params = PenaltyParams(**config.get("penalty", {}))
     pop_params = PopulationParams(**config.get("population", {}))
     nb_params = NeighbourhoodParams(**config.get("neighbourhood", {}))
@@ -254,8 +247,6 @@ def main():
 
     msg = """
     Directory to store runtime statistics in, as CSV files (one per instance).
-    If passed, this enables collecting runtime statistics (default False), at
-    a slight performance hit.
     """
     parser.add_argument("--stats_dir", help=msg)
 
diff --git a/pyvrp/exceptions.py b/pyvrp/exceptions.py
index 1ff5b42..3661857 100644
--- a/pyvrp/exceptions.py
+++ b/pyvrp/exceptions.py
@@ -1,10 +1,3 @@
-class StatisticsNotCollectedError(Exception):
-    """
-    Raised when statistics have not been collected, but a method is accessed
-    that can only be used when statistics are available.
-    """
-
-
 class EmptySolutionWarning(UserWarning):
     """
     Raised when an empty solution is being added to the Population. This is not
diff --git a/pyvrp/plotting/plot_diversity.py b/pyvrp/plotting/plot_diversity.py
index 29cd126..31a44bc 100644
--- a/pyvrp/plotting/plot_diversity.py
+++ b/pyvrp/plotting/plot_diversity.py
@@ -4,7 +4,6 @@ import matplotlib.pyplot as plt
 import numpy as np
 
 from pyvrp.Result import Result
-from pyvrp.exceptions import StatisticsNotCollectedError
 
 
 def plot_diversity(result: Result, ax: Optional[plt.Axes] = None):
@@ -18,17 +17,7 @@ def plot_diversity(result: Result, ax: Optional[plt.Axes] = None):
     ax, optional
         Axes object to draw the plot on. One will be created if not
         provided.
-
-    Raises
-    ------
-    StatisticsNotCollectedError
-        Raised when statistics have not been collected.
     """
-    if not result.has_statistics():
-        raise StatisticsNotCollectedError(
-            "Statistics have not been collected."
-        )
-
     if not ax:
         _, ax = plt.subplots()
 
diff --git a/pyvrp/plotting/plot_objectives.py b/pyvrp/plotting/plot_objectives.py
index 326f3d2..607a625 100644
--- a/pyvrp/plotting/plot_objectives.py
+++ b/pyvrp/plotting/plot_objectives.py
@@ -4,7 +4,6 @@ import matplotlib.pyplot as plt
 import numpy as np
 
 from pyvrp.Result import Result
-from pyvrp.exceptions import StatisticsNotCollectedError
 
 
 def plot_objectives(
@@ -30,17 +29,7 @@ def plot_objectives(
         Bounds the y-axis to ``(best * ylim_adjust[0], best * ylim_adjust[1])``
         where ``best`` denotes the best found feasible objective value. Default
         (0.95, 1.15).
-
-    Raises
-    ------
-    StatisticsNotCollectedError
-        Raised when statistics have not been collected.
     """
-    if not result.has_statistics():
-        raise StatisticsNotCollectedError(
-            "Statistics have not been collected."
-        )
-
     if not ax:
         _, ax = plt.subplots()
 
diff --git a/pyvrp/plotting/plot_result.py b/pyvrp/plotting/plot_result.py
index 8e83416..7cf1f9c 100644
--- a/pyvrp/plotting/plot_result.py
+++ b/pyvrp/plotting/plot_result.py
@@ -16,7 +16,7 @@ def plot_result(
 ):
     """
     Plots the results of a run, including the best solution and detailed
-    statistics about the algorithm's performance (if available).
+    statistics about the algorithm's performance.
 
     Parameters
     ----------
@@ -38,10 +38,9 @@ def plot_result(
     # data.
     gs = fig.add_gridspec(3, 2, width_ratios=(2 / 5, 3 / 5))
 
-    if result.has_statistics():
-        ax_div = fig.add_subplot(gs[0, 0])
-        plot_diversity(result, ax=ax_div)
-        plot_objectives(result, ax=fig.add_subplot(gs[1, 0], sharex=ax_div))
-        plot_runtimes(result, ax=fig.add_subplot(gs[2, 0], sharex=ax_div))
+    ax_div = fig.add_subplot(gs[0, 0])
+    plot_diversity(result, ax=ax_div)
+    plot_objectives(result, ax=fig.add_subplot(gs[1, 0], sharex=ax_div))
+    plot_runtimes(result, ax=fig.add_subplot(gs[2, 0], sharex=ax_div))
 
     plot_solution(result.best, data, ax=fig.add_subplot(gs[:, 1]))
diff --git a/pyvrp/plotting/plot_runtimes.py b/pyvrp/plotting/plot_runtimes.py
index 662e5e8..a48ca57 100644
--- a/pyvrp/plotting/plot_runtimes.py
+++ b/pyvrp/plotting/plot_runtimes.py
@@ -4,7 +4,6 @@ import matplotlib.pyplot as plt
 import numpy as np
 
 from pyvrp.Result import Result
-from pyvrp.exceptions import StatisticsNotCollectedError
 
 
 def plot_runtimes(result: Result, ax: Optional[plt.Axes] = None):
@@ -17,17 +16,7 @@ def plot_runtimes(result: Result, ax: Optional[plt.Axes] = None):
         Result for which to plot runtimes.
     ax, optional
         Axes object to draw the plot on. One will be created if not provided.
-
-    Raises
-    ------
-    StatisticsNotCollectedError
-        Raised when statistics have not been collected.
     """
-    if not result.has_statistics():
-        raise StatisticsNotCollectedError(
-            "Statistics have not been collected."
-        )
-
     if not ax:
         _, ax = plt.subplots()
 


Test patch:
diff --git a/pyvrp/plotting/tests/test_plotting.py b/pyvrp/plotting/tests/test_plotting.py
index 659e67b..504cbb8 100644
--- a/pyvrp/plotting/tests/test_plotting.py
+++ b/pyvrp/plotting/tests/test_plotting.py
@@ -1,5 +1,4 @@
 from matplotlib.testing.decorators import image_comparison as img_comp
-from numpy.testing import assert_, assert_raises
 
 from pyvrp import (
     CostEvaluator,
@@ -12,34 +11,11 @@ from pyvrp import (
 from pyvrp.Result import Result
 from pyvrp.Statistics import Statistics
 from pyvrp.diversity import broken_pairs_distance
-from pyvrp.exceptions import StatisticsNotCollectedError
 from pyvrp.tests.helpers import make_random_solutions, read, read_solution
 
 IMG_KWARGS = dict(remove_text=True, tol=2, extensions=["png"], style="mpl20")
 
 
-def test_plotting_methods_raise_when_no_stats_available():
-    data = read("data/OkSmall.txt")
-    sol = Solution(data, [[1, 2, 3, 4]])
-    res = Result(sol, Statistics(), 0, 0.0)
-
-    assert_(not res.has_statistics())
-
-    with assert_raises(StatisticsNotCollectedError):
-        plotting.plot_diversity(res)
-
-    with assert_raises(StatisticsNotCollectedError):
-        plotting.plot_objectives(res)
-
-    with assert_raises(StatisticsNotCollectedError):
-        plotting.plot_runtimes(res)
-
-    # These should not raise, since they do not depend on statistics
-    # (plot_solution) or optionally print statistics (plot_result).
-    plotting.plot_solution(res.best, data)
-    plotting.plot_result(res, data)
-
-
 @img_comp(["plot_solution", "plot_solution_with_customers"], **IMG_KWARGS)
 def test_plot_solution():
     data = read("data/RC208.txt", "solomon", round_func="trunc")
diff --git a/pyvrp/tests/test_GeneticAlgorithm.py b/pyvrp/tests/test_GeneticAlgorithm.py
index 2b3fb73..3a4ba0b 100644
--- a/pyvrp/tests/test_GeneticAlgorithm.py
+++ b/pyvrp/tests/test_GeneticAlgorithm.py
@@ -19,21 +19,19 @@ from pyvrp.tests.helpers import make_random_solutions, read, read_solution
 
 @mark.parametrize(
     "repair_probability,"
-    "collect_statistics,"
     "intensify_probability,"
     "intensify_on_best,"
     "nb_iter_no_improvement",
     [
-        (-0.25, True, 0.5, True, 0),  # repair_probability < 0
-        (1.25, True, 0.5, True, 0),  # repair_probability > 1
-        (0.0, True, 0.5, True, -1),  # nb_iter_no_improvement < 0
-        (0.0, True, -0.25, True, 1),  # intensify_probability < 0
-        (0.0, True, 1.25, True, 1),  # intensify_probability > 1
+        (-0.25, 0.5, True, 0),  # repair_probability < 0
+        (1.25, 0.5, True, 0),  # repair_probability > 1
+        (0.0, 0.5, True, -1),  # nb_iter_no_improvement < 0
+        (0.0, -0.25, True, 1),  # intensify_probability < 0
+        (0.0, 1.25, True, 1),  # intensify_probability > 1
     ],
 )
 def test_params_constructor_throws_when_arguments_invalid(
     repair_probability: float,
-    collect_statistics: bool,
     intensify_probability: int,
     intensify_on_best: bool,
     nb_iter_no_improvement: int,
@@ -44,7 +42,6 @@ def test_params_constructor_throws_when_arguments_invalid(
     with assert_raises(ValueError):
         GeneticAlgorithmParams(
             repair_probability,
-            collect_statistics,
             intensify_probability,
             intensify_on_best,
             nb_iter_no_improvement,
@@ -53,24 +50,20 @@ def test_params_constructor_throws_when_arguments_invalid(
 
 @mark.parametrize(
     "repair_probability,"
-    "collect_statistics,"
     "intensify_probability,"
     "intensify_on_best,"
     "nb_iter_no_improvement",
     [
-        (0.0, True, 0.5, True, 0),  # nb_iter_no_improvement == 0
-        (0.0, True, 0.5, True, 1),  # repair_probability == 0
-        (1.0, True, 0.5, True, 1),  # repair_probability == 1
-        (0.5, False, 0.5, True, 1),  # collect_statistics is False
-        (0.5, True, 0, True, 1),  # intensify_probability == 0
-        (0.5, True, 1, True, 1),  # intensify_probability == 1
-        (0.5, True, 0.5, False, 1),  # intensify_on_best is False
-        (0.5, False, 0.5, False, 1),  # both False
+        (0.0, 0.5, True, 0),  # nb_iter_no_improvement == 0
+        (0.0, 0.5, True, 1),  # repair_probability == 0
+        (1.0, 0.5, True, 1),  # repair_probability == 1
+        (0.5, 0, True, 1),  # intensify_probability == 0
+        (0.5, 1, True, 1),  # intensify_probability == 1
+        (0.5, 0.5, False, 1),  # intensify_on_best is False
     ],
 )
 def test_params_constructor_does_not_raise_when_arguments_valid(
     repair_probability: float,
-    collect_statistics: bool,
     intensify_probability: float,
     intensify_on_best: bool,
     nb_iter_no_improvement: int,
@@ -80,14 +73,12 @@ def test_params_constructor_does_not_raise_when_arguments_valid(
     """
     params = GeneticAlgorithmParams(
         repair_probability,
-        collect_statistics,
         intensify_probability,
         intensify_on_best,
         nb_iter_no_improvement,
     )
 
     assert_allclose(params.repair_probability, repair_probability)
-    assert_equal(params.collect_statistics, collect_statistics)
     assert_equal(params.intensify_probability, intensify_probability)
     assert_equal(params.intensify_on_best, intensify_on_best)
     assert_equal(params.nb_iter_no_improvement, nb_iter_no_improvement)
@@ -227,5 +218,3 @@ def test_best_initial_solution():
 
 
 # TODO more functional tests
-
-# TODO test statistics collection on Result.has_statistics
diff --git a/pyvrp/tests/test_Result.py b/pyvrp/tests/test_Result.py
index a718ebc..8856a46 100644
--- a/pyvrp/tests/test_Result.py
+++ b/pyvrp/tests/test_Result.py
@@ -45,10 +45,8 @@ def test_init_raises_invalid_arguments(num_iterations, runtime):
         Result(sol, Statistics(), num_iterations, runtime)
 
 
-@mark.parametrize(
-    "num_iterations, has_statistics", [(0, False), (1, True), (10, True)]
-)
-def test_has_statistics(num_iterations: int, has_statistics: bool):
+@mark.parametrize("num_iterations", [0, 1, 10])
+def test_num_iterations(num_iterations: int):
     data = read("data/OkSmall.txt")
     cost_evaluator = CostEvaluator(20, 6)
     rng = XorShift128(seed=42)
@@ -60,7 +58,6 @@ def test_has_statistics(num_iterations: int, has_statistics: bool):
 
     best = Solution.make_random(data, rng)
     res = Result(best, stats, num_iterations, 0.0)
-    assert_equal(res.has_statistics(), has_statistics)
     assert_equal(res.num_iterations, num_iterations)