Repository: benchopt/benchopt
Pull request #313 (2022-03-25 11:25:45)

TST make test more permissive for complex benchmarks
For some benchmark, the return of the solver is not a simple `np.array` but a complex object.
For those, the test are too restrictive. I propose to skip part of the test when the dataset return `dimension = 'object'`.

Also, some algorithms do not converge quickly. thus `test_solver` can be very long/slow.
I will try to mitigate this in the PR.

- [x] skip dimension tests
- [x] mitigate slow `test_solver`

codecov-commenter: # [Codecov](https://codecov.io/gh/benchopt/benchOpt/pull/313?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=benchopt) Report
> Merging [#313](https://codecov.io/gh/benchopt/benchOpt/pull/313?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=benchopt) (8177d9a) into [main](https://codecov.io/gh/benchopt/benchOpt/commit/24c0ad0d660707cab74fc088ec83c2ffe6f2e04d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=benchopt) (24c0ad0) will **not change** coverage.
> The diff coverage is `n/a`.


```diff
@@           Coverage Diff           @@
##             main     #313   +/-   ##
=======================================
  Coverage   61.46%   61.46%           
=======================================
  Files          41       41           
  Lines        2273     2273           
  Branches      417      417           
=======================================
  Hits         1397     1397           
  Misses        778      778           
  Partials       98       98           
```




Patch:
diff --git a/benchopt/base.py b/benchopt/base.py
index ed4f216..d174de4 100644
--- a/benchopt/base.py
+++ b/benchopt/base.py
@@ -351,9 +351,30 @@ class BaseObjective(ParametrizedNameMixin, DependenciesMixin):
     # hashing the data directly.
     def set_dataset(self, dataset):
         self._dataset = dataset
-        _, data = dataset._get_data()
+        self._dimension, data = dataset._get_data()
         return self.set_data(**data)
 
+    def get_one_beta(self):
+        """Return one point in which the objective function can be evaluated.
+
+        This method is mainly for testing purposes, to check that the return
+        computation and the return type of `Objective.compute`. It should
+        return an object that can be passed to compute.
+
+        By default, if `Dataset.get_data` returns a shape, this function will
+        return an array full of 0. It can be overriden to provide a different
+        point. When `Dataset.get_data` returns "object", this method must be
+        overwritten.
+        """
+        if self._dimension == 'object':
+            raise NotImplementedError(
+                "If solvers return objects, `Objective.get_one_beta` should "
+                "be overriden to return an object compatible with `compute`."
+            )
+
+        import numpy as np
+        return np.zeros(self._dimension)
+
     # Reduce the pickling and hashing burden by only pickling class parameters.
     @staticmethod
     def _reconstruct(module_filename, pickled_module_hash, parameters,


Test patch:
diff --git a/benchopt/tests/test_benchmarks.py b/benchopt/tests/test_benchmarks.py
index d6d7c11..ce0ec03 100644
--- a/benchopt/tests/test_benchmarks.py
+++ b/benchopt/tests/test_benchmarks.py
@@ -14,23 +14,22 @@ def test_benchmark_objective(benchmark, dataset_simu):
     objective = objective_class.get_instance()
 
     dataset = dataset_simu.get_instance()
-    dimension, data = dataset._get_data()
-    objective.set_data(**data)
+    objective.set_dataset(dataset)
 
     # check that the reported dimension is correct and that the result of
     # the objective function is a dictionary containing a scalar value for
     # `objective_value`.
-    beta_hat = np.zeros(dimension)
+    beta_hat = objective.get_one_beta()
     objective_dict = objective(beta_hat)
 
     assert 'objective_value' in objective_dict, (
-        'When the output of objective is a dict, it should at least contain '
-        'a value associated to `objective_value` which will be used to detect '
-        'the convergence of the algorithm.'
+        "When the output of objective is a dict, it should at least "
+        "contain a value associated to `objective_value` which will be "
+        "used to detect the convergence of the algorithm."
     )
     assert np.isscalar(objective_dict['objective_value']), (
-        "The output of the objective function should be a scalar, or a dict "
-        "containing a scalar associated to `objective_value`."
+        "The output of the objective function should be a scalar, or a "
+        "dict containing a scalar associated to `objective_value`."
     )
 
 
@@ -76,14 +75,15 @@ def test_dataset_get_data(benchmark, dataset_class):
 
     dimension, data = res
 
-    assert isinstance(dimension, tuple), (
-        "First output of get_data should be an integer or a tuple of integers."
-        f" Got {dimension}."
-    )
-    assert all(isinstance(d, numbers.Integral) for d in dimension), (
+    assert isinstance(dimension, tuple) or dimension == 'object', (
         "First output of get_data should be an integer or a tuple of integers."
         f" Got {dimension}."
     )
+    if dimension != 'object':
+        assert all(isinstance(d, numbers.Integral) for d in dimension), (
+            "First output of get_data should be an integer or a tuple of "
+            f"integers. Got {dimension}."
+        )
     assert isinstance(data, dict), (
         f"Second output of get_data should be a dict. Got {data}."
     )
@@ -162,34 +162,40 @@ def test_solver(benchmark, solver_class):
     dataset_class = simulated_dataset[0]
     dataset = dataset_class.get_instance()
 
-    dimension, data = dataset._get_data()
-    objective.set_data(**data)
+    objective.set_dataset(dataset)
 
     solver = solver_class.get_instance()
     solver.set_objective(**objective.to_dict())
 
-    # Either call run_with_cb or run
+    is_convex = getattr(objective, "is_convex", True)
 
+    # Either call run_with_cb or run
     if solver._solver_strategy == 'callback':
         sc = solver.stopping_criterion.get_runner_instance(
             max_runs=25, timeout=None, solver=solver
         )
+        if not is_convex:
+            # Set large tolerance for the stopping criterion to stop fast
+            sc.eps = 5e-1
         cb = _Callback(
             objective, meta={}, stopping_criterion=sc
         )
         solver.run(cb)
     else:
-        stop_val = 5000 if solver._solver_strategy == 'iteration' else 1e-15
+        if solver._solver_strategy == 'iteration':
+            stop_val = 5000 if is_convex else 10
+        else:
+            stop_val = 1e-15 if is_convex else 1e-2
         solver.run(stop_val)
 
-    beta_hat_i = solver.get_result()
-
-    assert beta_hat_i.shape == dimension
+    # Check that beta_hat is compatible to compute the objective function
+    beta_hat = solver.get_result()
+    objective(beta_hat)
 
-    if getattr(objective, "is_convex", True):
-        val_star = objective(beta_hat_i)['objective_value']
+    if is_convex:
+        val_star = objective(beta_hat)['objective_value']
         for _ in range(100):
-            eps = 1e-5 * np.random.randn(*dimension)
-            val_eps = objective(beta_hat_i + eps)['objective_value']
+            eps = 1e-5 * np.random.randn(*beta_hat.shape)
+            val_eps = objective(beta_hat + eps)['objective_value']
             diff = val_eps - val_star
             assert diff >= 0