Repository: benchopt/benchopt
Pull request #684 (2024-03-07 10:44:27)

ENH allow for non-numerical values in objective
cc @yanislalou, @rflamary

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/benchopt/benchopt/pull/684?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=benchopt) Report
> Merging [#684](https://app.codecov.io/gh/benchopt/benchopt/pull/684?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=benchopt) (36e0499) into [main](https://app.codecov.io/gh/benchopt/benchopt/commit/fd0a0169c6738cb497794a09be415dbd144d3c49?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=benchopt) (fd0a016) will **decrease** coverage by `0.05%`.
> The diff coverage is `25.00%`.

:exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.

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


```diff
@@            Coverage Diff             @@
##             main     #684      +/-   ##
==========================================
- Coverage   55.03%   54.98%   -0.05%     
==========================================
  Files          45       45              
  Lines        2962     2966       +4     
  Branches      548      550       +2     
==========================================
+ Hits         1630     1631       +1     
- Misses       1209     1212       +3     
  Partials      123      123              
```



</details>

Patch:
diff --git a/benchopt/plotting/generate_html.py b/benchopt/plotting/generate_html.py
index 2011620..c988eb0 100644
--- a/benchopt/plotting/generate_html.py
+++ b/benchopt/plotting/generate_html.py
@@ -195,8 +195,12 @@ def shape_objectives_columns_for_html(df, dataset, objective):
         df_filtered = df.query(
             "data_name == @dataset & objective_name == @objective"
         )
+        columns_data = shape_solvers_for_html(df_filtered, column)
+        if columns_data is None:
+            # Non-numeric column, skipping it
+            continue
         objective_columns_data[column] = {
-            'solvers': shape_solvers_for_html(df_filtered, column),
+            'solvers': columns_data,
             # Values used in javascript to do computation
             'transformers': {
                 'c_star': float(df_filtered[column].min() - 1e-10),
@@ -224,9 +228,13 @@ def shape_solvers_for_html(df, objective_column):
 
         # compute median of 'time' and objective_column
         fields = ["time", objective_column]
-        groupby_stop_val_median = df_filtered.groupby('stop_val')
-        groupby_stop_val_median = groupby_stop_val_median[fields]
-        groupby_stop_val_median = groupby_stop_val_median.median()
+        groupby_stop_val_median = (
+            df_filtered.groupby('stop_val')[fields]
+            .median(numeric_only=True)
+        )
+        if objective_column not in groupby_stop_val_median:
+            # Non-numeric values, skipping this column
+            return None
 
         q1, q9 = compute_quantiles(df_filtered)
 
diff --git a/benchopt/runner.py b/benchopt/runner.py
index 28c9763..9d99656 100644
--- a/benchopt/runner.py
+++ b/benchopt/runner.py
@@ -366,7 +366,14 @@ def run_benchmark(benchmark, solver_names=None, forced_solvers=None,
     else:
         save_file = output_dir / f"{output_name}.parquet"
         save_file = uniquify_results(save_file)
-    df.to_parquet(save_file)
+    try:
+        df.to_parquet(save_file)
+    except Exception:
+        # Failed to save the results as a parquet file, falling back
+        # to csv. This can be due to mixed types columns or missing
+        # dependencies.
+        save_file = save_file.with_suffix(".csv")
+        df.to_csv(save_file)
     output.savefile_status(save_file=save_file)
 
     if plot_result:
diff --git a/benchopt/stopping_criterion.py b/benchopt/stopping_criterion.py
index fa66df8..a240501 100644
--- a/benchopt/stopping_criterion.py
+++ b/benchopt/stopping_criterion.py
@@ -72,7 +72,10 @@ class StoppingCriterion():
 
         self.kwargs = kwargs
         self.strategy = strategy
-        self.key_to_monitor = key_to_monitor
+        self.key_to_monitor = (
+            key_to_monitor if key_to_monitor.startswith('objective_')
+            else f'objective_{key_to_monitor}'
+        )
 
     def get_runner_instance(self, max_runs=1, timeout=None, output=None,
                             solver=None):
@@ -192,6 +195,15 @@ class StoppingCriterion():
             Next value for the stopping criterion. This value depends on the
             sampling strategy for the solver.
         """
+        # Check that the objective is compatible with the stopping_criterion
+        if self.key_to_monitor not in objective_list[0]:
+            key = self.key_to_monitor.replace("objective_", "")
+            raise ValueError(
+                "Objective.evaluate_result() should contain a key named "
+                f"'{key}' to be used with this stopping_criterion. The name of"
+                " this key can be changed via the 'key_to_monitor' parameter."
+            )
+
         # Modify the criterion state:
         # - compute the number of run with the curve. We need to remove 1 as
         #   it contains the initial evaluation.


Test patch:
diff --git a/benchopt/tests/test_objective_outputs.py b/benchopt/tests/test_objective_outputs.py
new file mode 100644
index 0000000..a30022f
--- /dev/null
+++ b/benchopt/tests/test_objective_outputs.py
@@ -0,0 +1,121 @@
+import pytest
+
+from benchopt.cli.main import run
+from benchopt.utils.temp_benchmark import temp_benchmark
+from benchopt.tests.utils import CaptureRunOutput
+
+
+MINIMAL_SOLVER = """from benchopt import BaseSolver
+
+    class Solver(BaseSolver):
+        name = "test-solver"
+        sampling_strategy = 'run_once'
+        def set_objective(self, X, y): pass
+        def run(self, n_iter): pass
+        def get_result(self): return dict(beta=1)
+    """
+
+
+def test_objective_bad_name(no_debug_test):
+    # Check that if Obejctive.evaluate_result return a `name` field, a sensible
+    # error is raised.
+
+    objective = """from benchopt import BaseObjective
+
+        class Objective(BaseObjective):
+            name = "test_obj"
+            min_benchopt_version = "0.0.0"
+
+            def set_data(self, X, y): pass
+            def get_one_result(self): pass
+            def evaluate_result(self, beta): return dict(value=1, name=0)
+            def get_objective(self): return dict(X=0, y=0)
+    """
+
+    with temp_benchmark(
+            objective=objective,
+            solvers=[MINIMAL_SOLVER]
+    ) as benchmark:
+        with pytest.raises(SystemExit, match='1'):
+            with CaptureRunOutput() as out:
+                run([str(benchmark.benchmark_dir),
+                    *'-s test-solver -d test-dataset -n 1 -r 1 --no-plot'
+                    .split()], standalone_mode=False)
+
+    out.check_output("ValueError: objective output cannot be called 'name'")
+
+
+def test_objective_no_value(no_debug_test):
+    # Check that if Obejctive.evaluate_result does not return a `value` field,
+    # a sensible error is raised.
+
+    objective = """from benchopt import BaseObjective
+
+        class Objective(BaseObjective):
+            name = "test_obj"
+            min_benchopt_version = "0.0.0"
+
+            def set_data(self, X, y): pass
+            def get_one_result(self): pass
+            def evaluate_result(self, beta): return dict(test_acc=0)
+            def get_objective(self): return dict(X=0, y=0)
+    """
+
+    with temp_benchmark(
+            objective=objective,
+            solvers=[MINIMAL_SOLVER]
+    ) as benchmark:
+        with pytest.raises(SystemExit, match='1'):
+            with CaptureRunOutput() as out:
+                run([str(benchmark.benchmark_dir),
+                    *'-s test-solver -d test-dataset -n 1 -r 1 --no-plot'
+                    .split()], standalone_mode=False)
+
+    out.check_output(
+        r"Objective.evaluate_result\(\) should contain a key named 'value'"
+    )
+
+
+def test_objective_nonnumeric_values(no_debug_test):
+    # Check that non-numerical values in objective do not raise error
+    # in saving and generating the plots.
+
+    objective = """from benchopt import BaseObjective
+
+        class Objective(BaseObjective):
+            name = "test_obj"
+            min_benchopt_version = "0.0.0"
+
+            def set_data(self, X, y): self.X, self.y = X, y
+            def get_one_result(self): pass
+            def evaluate_result(self, beta):
+                return dict(value=1, test_obj={})
+
+            def get_objective(self):
+                return dict(X=0, y=0)
+    """
+
+    with temp_benchmark(
+            objective=objective,
+            solvers=[MINIMAL_SOLVER]
+    ) as benchmark:
+        with CaptureRunOutput() as out:
+            run([str(benchmark.benchmark_dir),
+                 *'-s test-solver -d test-dataset -n 1 -r 1 --no-display'
+                 .split()], standalone_mode=False)
+
+        assert out.result_files[0].endswith('.csv')
+
+    objective = objective.replace(
+        "test_obj={}", "test_obj={'a':0, 'b': 1.0, 'c': '', 'd': {}}"
+    )
+    with temp_benchmark(
+            objective=objective,
+            solvers=[MINIMAL_SOLVER]
+    ) as benchmark:
+        with CaptureRunOutput() as out:
+            run([str(benchmark.benchmark_dir),
+                 *'-s test-solver -d test-dataset -n 1 -r 1 --no-display'
+                 .split()], standalone_mode=False)
+
+        assert out.result_files[0].endswith('.csv')
diff --git a/benchopt/tests/test_stopping_criterion.py b/benchopt/tests/test_stopping_criterion.py
index b0c7749..b29ec01 100644
--- a/benchopt/tests/test_stopping_criterion.py
+++ b/benchopt/tests/test_stopping_criterion.py
@@ -98,19 +98,62 @@ def test_key_to_monitor(criterion_class, strategy):
     criterion = criterion_class(strategy=strategy, key_to_monitor=key)
 
     criterion = criterion.get_runner_instance(max_runs=10)
-    assert criterion.key_to_monitor == key
+    assert criterion.key_to_monitor == f"objective_{key}"
     stop_val = criterion.init_stop_val()
-    objective_list = [{'objective_value': np.nan, key: 1}]
+    objective_list = [{'objective_value': np.nan, f"objective_{key}": 1}]
     stop, status, stop_val = criterion.should_stop(stop_val, objective_list)
     assert not stop, "Should not have stopped"
     assert status == 'running', "Should stop on diverged"
 
-    objective_list.append({key: 1e5+2})
+    objective_list.append({f"objective_{key}": 1e5+2})
     stop, status, stop_val = criterion.should_stop(stop_val, objective_list)
     assert stop, "Should have stopped"
     assert status == 'diverged', "Should stop on diverged"
 
 
+@pytest.mark.parametrize('strategy', SAMPLING_STRATEGIES)
+@pytest.mark.parametrize('criterion_class', [
+    SufficientDescentCriterion, SufficientProgressCriterion
+])
+def test_key_to_monitor_objective(no_debug_test, criterion_class, strategy):
+    "Check that the criterion tracks the right objective key."
+    key = 'test_key'
+
+    objective = f"""from benchopt import BaseObjective
+
+        class Objective(BaseObjective):
+            name = "test_obj"
+            min_benchopt_version = "0.0.0"
+
+            def set_data(self, X, y): pass
+            def get_one_result(self): pass
+            def evaluate_result(self, beta): return dict({key}=1)
+            def get_objective(self): return dict(X=0, y=0)
+    """
+
+    solver = f"""from benchopt import BaseSolver
+    from benchopt.stopping_criterion import *
+
+    class Solver(BaseSolver):
+        name = "test-solver"
+        stopping_criterion = {criterion_class.__name__}(
+            key_to_monitor='{key}'
+        )
+        def set_objective(self, X, y): pass
+        def run(self, n_iter): pass
+        def get_result(self): return dict(beta=1)
+    """
+
+    with temp_benchmark(objective=objective, solvers=[solver]) as benchmark:
+        with CaptureRunOutput() as out:
+            run([str(benchmark.benchmark_dir),
+                *'-s test-solver -d test-dataset -n 10 -r 1 --no-display'
+                .split()], standalone_mode=False)
+
+    out.check_output('test-solver', 5)
+    out.check_output('done', 1)
+
+
 @pytest.mark.parametrize('strategy', SAMPLING_STRATEGIES)
 def test_solver_strategy(no_debug_test, strategy):
 
diff --git a/benchopt/tests/utils/capture_run_output.py b/benchopt/tests/utils/capture_run_output.py
index b22f568..22e570b 100644
--- a/benchopt/tests/utils/capture_run_output.py
+++ b/benchopt/tests/utils/capture_run_output.py
@@ -36,7 +36,7 @@ class CaptureRunOutput(object):
 
         # Make sure to delete all the result that created by the run command.
         self.result_files = re.findall(
-            r'Saving result in: (.*\.parquet)', self.output
+            r'Saving result in: (.*\.parquet|.*\.csv)', self.output
         )
         if len(self.result_files) >= 1 and self.delete_result_files:
             for result_file in self.result_files: