Repository: benchopt/benchopt (python)

Issue:
FEAT Add option to disable timeout
An option to disable the timeout would be great (`--timeout -1` for instance).

Pull request:
CLI benchopt run add --no-timeout option
### Checks before merging PR
- [x] added documentation for any new feature
- [x] added unit test
- [x] edited the [what's new](../../whatsnew.rst) (if applicable)


Fixes #589


Patch:
diff --git a/benchopt/benchmark.py b/benchopt/benchmark.py
index c1a9f12..e19d758 100644
--- a/benchopt/benchmark.py
+++ b/benchopt/benchmark.py
@@ -110,6 +110,8 @@ class Benchmark:
             self.url = f"https://github.com/benchopt/{self.name}"
         else:
             self.name = Path(self.url).name
+        # replace dots to avoid issues with `with_suffix``
+        self.name = self.name.replace('.', '-')
 
     ####################################################################
     # Helpers to access and validate objective, solvers and datasets
diff --git a/benchopt/cli/main.py b/benchopt/cli/main.py
index 979d032..80c4f25 100644
--- a/benchopt/cli/main.py
+++ b/benchopt/cli/main.py
@@ -4,6 +4,7 @@ import warnings
 from pathlib import Path
 
 from benchopt.benchmark import Benchmark
+from benchopt.config import get_setting
 from benchopt.cli.completion import complete_solvers
 from benchopt.cli.completion import complete_datasets
 from benchopt.cli.completion import complete_benchmarks
@@ -51,6 +52,7 @@ def _get_run_args(cli_kwargs, config_file_kwargs):
         "max_runs",
         "n_repetitions",
         "timeout",
+        "no_timeout",
         "n_jobs",
         "slurm",
         "collect",
@@ -118,10 +120,15 @@ def _get_run_args(cli_kwargs, config_file_kwargs):
               help='Number of repetitions that are averaged to estimate the '
               'runtime.')
 @click.option('--timeout',
-              default=100, show_default=True, type=str,
-              help='Stop a solver when run for more than <timeout> seconds.'
-              ' The syntax 10h or 10m can be used to denote 10 hours or '
-              'minutes respectively.')
+              default=None, show_default=True, type=str,
+              help='Stop a solver when run for more than <timeout> seconds. '
+              'The syntax 10h or 10m can be used to denote 10 hours or '
+              'minutes respectively. Not compatible with the --no-timeout '
+              'option.')
+@click.option('--no-timeout',
+              is_flag=True,
+              help='If set, prevent solvers from stopping after running for '
+              'a long time. Not compatible with the --timeout option.')
 @click.option('--collect',
               is_flag=True,
               help='If set, this run will only collect results which are '
@@ -182,15 +189,27 @@ def run(config_file=None, **kwargs):
 
     (
         benchmark, solver_names, forced_solvers, dataset_names,
-        objective_filters, max_runs, n_repetitions, timeout, n_jobs, slurm,
-        collect, plot, display, html, pdb, do_profile, env_name, output_name
+        objective_filters, max_runs, n_repetitions, timeout, no_timeout,
+        n_jobs, slurm, collect, plot, display, html, pdb, do_profile,
+        env_name, output_name
     ) = _get_run_args(kwargs, config)
 
-    try:
-        timeout = int(float(timeout))
-    except ValueError:  # already under string format
-        import pandas as pd
-        timeout = pd.to_timedelta(timeout).total_seconds()
+    # If --no-timeout is set and --timeout is not, skip these blocks
+    # and keep timeout = None
+    if no_timeout and timeout is not None:
+        raise click.BadParameter(
+            'You cannot specify both --timeout and --no-timeout options.'
+        )
+
+    if not no_timeout:
+        if timeout is None:
+            timeout = get_setting('default_timeout')
+        else:
+            try:
+                timeout = int(float(timeout))
+            except ValueError:  # already under string format
+                import pandas as pd
+                timeout = pd.to_timedelta(timeout).total_seconds()
 
     # Create the Benchmark object
     benchmark = Benchmark(benchmark)
@@ -315,7 +334,9 @@ def run(config_file=None, **kwargs):
     cmd = (
         rf"benchopt run --local {benchmark.benchmark_dir} "
         rf"--n-repetitions {n_repetitions} "
-        rf"--max-runs {max_runs} --timeout {timeout} "
+        rf"--max-runs {max_runs} "
+        rf"{f'--timeout {timeout} ' if timeout is not None else ''}"
+        rf"{'--no-timeout ' if no_timeout else ''} "
         rf"--n-jobs {n_jobs} {'--slurm' if slurm else ''} "
         rf"{solvers_option} {forced_solvers_option} "
         rf"{datasets_option} {objective_option} "
diff --git a/benchopt/config.py b/benchopt/config.py
index 3e45092..08e549b 100644
--- a/benchopt/config.py
+++ b/benchopt/config.py
@@ -22,6 +22,7 @@ DEFAULT_GLOBAL_CONFIG = {
     'conda_cmd': 'conda',
     'shell': os.environ.get('SHELL', 'bash'),
     'cache': None,
+    'default_timeout': 100,
 }
 """
 * ``debug``: If set to true, enable debug logs.
@@ -241,7 +242,7 @@ def get_setting(name, config_file=None, benchmark_name=None,
     name : str
         Name of the config parameter to retrieve.
     config_file : str | Path
-        Path to a config file from which the setting can be retreives. When
+        Path to a config file from which the setting can be retrieved. When
         it is not provided, default to the global benchopt config file.
     benchmark_name : str
         Name of the benchmark for which the setting are retrieved.
@@ -347,6 +348,10 @@ def parse_value(value, default_value):
                       for v in value.split(',') if v != '']
             value = values
         assert isinstance(value, list), value
+    elif isinstance(default_value, int):
+        value = int(value)
+    elif isinstance(default_value, float):
+        value = float(value)
 
     return value
 
diff --git a/benchopt/runner.py b/benchopt/runner.py
index 01b5432..d031b08 100644
--- a/benchopt/runner.py
+++ b/benchopt/runner.py
@@ -156,7 +156,7 @@ def run_one_to_cvg(benchmark, objective, solver, meta, stopping_criterion,
 
 
 def run_one_solver(benchmark, dataset, objective, solver, n_repetitions,
-                   max_runs, timeout, force=False, collect=False,
+                   max_runs, timeout=None, force=False, collect=False,
                    output=None, pdb=False):
     """Run a benchmark for a given dataset, objective and solver.
 
@@ -254,9 +254,10 @@ def run_one_solver(benchmark, dataset, objective, solver, n_repetitions,
         stopping_criterion = solver._stopping_criterion.get_runner_instance(
             solver=solver,
             max_runs=max_runs,
-            timeout=timeout / n_repetitions,
+            timeout=timeout / n_repetitions if timeout is not None else None,
             output=output,
         )
+
         args_run_one_to_cvg = dict(
             benchmark=benchmark, objective=objective, solver=solver, meta=meta,
             stopping_criterion=stopping_criterion, force=force, output=output,
@@ -297,7 +298,7 @@ def run_one_solver(benchmark, dataset, objective, solver, n_repetitions,
 
 def run_benchmark(benchmark, solvers=None, forced_solvers=None,
                   datasets=None, objectives=None, max_runs=10,
-                  n_repetitions=1, timeout=100, n_jobs=1, slurm=None,
+                  n_repetitions=1, timeout=None, n_jobs=1, slurm=None,
                   plot_result=True, display=True, html=True,  collect=False,
                   show_progress=True, pdb=False, output_name="None"):
     """Run full benchmark.
@@ -345,7 +346,7 @@ def run_benchmark(benchmark, solvers=None, forced_solvers=None,
     show_progress : bool
         If show_progress is set to True, display the progress of the benchmark.
     pdb : bool
-        It pdb is set to True, open a debugger on error.
+        If pdb is set to True, open a debugger on error.
     output_name : str
         Filename for the parquet output. If given, the results will
         be stored at <BENCHMARK>/outputs/<filename>.parquet.
diff --git a/doc/names.inc b/doc/names.inc
index 09ff255..ecefb00 100644
--- a/doc/names.inc
+++ b/doc/names.inc
@@ -26,6 +26,8 @@
 
 .. _Christopher Marouani: https://github.com/chris-mrn
 
+.. _Célestin Eve: https://github.com/ceelestin
+
 .. _Pierre-Antoine Comby: https://github.com/paquiteau
 
 .. _Virginie Loison: https://www.linkedin.com/in/virginie-loison/
diff --git a/doc/whats_new.rst b/doc/whats_new.rst
index bda81ab..3f8821b 100644
--- a/doc/whats_new.rst
+++ b/doc/whats_new.rst
@@ -33,6 +33,9 @@ CLI
 - Add ``--download`` option in ``benchopt install`` to allow downloading
   the data when installing the benchmark. By `Thomas Moreau`_ (:gh:`718`)
 
+- Add ``--no-timeout`` option in ``benchopt run`` to allow solvers to bypass 
+  timeout. By `Célestin Eve`_ (:gh:`725`)
+
 FIX
 ~~~
 


Test patch:
diff --git a/benchopt/cli/tests/test_cli.py b/benchopt/cli/tests/test_cli.py
index dfd573c..0e789ff 100644
--- a/benchopt/cli/tests/test_cli.py
+++ b/benchopt/cli/tests/test_cli.py
@@ -6,6 +6,7 @@ import tempfile
 from pathlib import Path
 
 import click
+import os
 import pytest
 from joblib.memory import _FUNCTION_HASHES
 from click.shell_completion import ShellComplete
@@ -174,6 +175,46 @@ class TestRunCmd:
         # Make sure the results were saved in a result file
         assert len(out.result_files) == 1, out.output
 
+    def test_no_timeout(self):
+        # First test: --timeout==0
+        with CaptureRunOutput() as out_timeout:
+            run([str(DUMMY_BENCHMARK_PATH), '-d', SELECT_ONE_SIMULATED, '-f',
+                SELECT_ONE_PGD, '-o', SELECT_ONE_OBJECTIVE, '--no-plot',
+                '--timeout=0'], 'benchopt', standalone_mode=False)
+        out_timeout.check_output('timeout', repetition=1)
+
+        try:
+            old_value = os.environ.get('BENCHOPT_DEFAULT_TIMEOUT')
+            os.environ['BENCHOPT_DEFAULT_TIMEOUT'] = "0"
+            # Second test: no option about timeout
+            with CaptureRunOutput() as out_timeout_default:
+                run([str(DUMMY_BENCHMARK_PATH), '-d', SELECT_ONE_SIMULATED,
+                     '-f', SELECT_ONE_PGD, '-o', SELECT_ONE_OBJECTIVE,
+                     '--no-plot'], 'benchopt', standalone_mode=False)
+            out_timeout_default.check_output('timeout', repetition=1)
+
+            # Third test: --no-timeout
+            with CaptureRunOutput() as out_no_timeout:
+                run([str(DUMMY_BENCHMARK_PATH), '-d',
+                     SELECT_ONE_SIMULATED, '-f', SELECT_ONE_PGD,
+                     '-o', SELECT_ONE_OBJECTIVE, '--no-plot', '--no-timeout'],
+                    'benchopt', standalone_mode=False)
+            out_no_timeout.check_output('timeout', repetition=0)
+
+        finally:
+            if old_value is not None:
+                os.environ['BENCHOPT_DEFAULT_TIMEOUT'] = old_value
+            else:
+                del os.environ['BENCHOPT_DEFAULT_TIMEOUT']
+
+        # Fourth test: --timeout and --no-timeout both specified
+        match = 'You cannot specify both --timeout and --no-timeout options.'
+        with pytest.raises(click.BadParameter, match=match):
+            run([str(DUMMY_BENCHMARK_PATH), '-d', SELECT_ONE_SIMULATED,
+                 '-f', SELECT_ONE_PGD, '-o', SELECT_ONE_OBJECTIVE,
+                 '--no-plot', '--timeout=0', '--no-timeout'],
+                'benchopt', standalone_mode=False)
+
     def test_custom_parameters(self):
         SELECT_DATASETS = r'simulated[n_features=[100, 200]]'
         SELECT_SOLVERS = r'python-pgd-with-cb[use_acceleration=[True, False]]'
diff --git a/benchopt/tests/test_config.py b/benchopt/tests/test_config.py
index 22f49d2..761a42d 100644
--- a/benchopt/tests/test_config.py
+++ b/benchopt/tests/test_config.py
@@ -41,7 +41,6 @@ def test_parse_value():
     # Check that if the default is not a bool or a list, we return the value
     assert parse_value(1, 2) == 1
     assert parse_value(2.0, 1) == 2.0
-    assert parse_value('abc', 1) == 'abc'
     assert parse_value('abc', 'a') == 'abc'
 
 
@@ -71,7 +70,14 @@ def test_config_file_set(setting_key):
         ), default_value)
         assert get_setting(setting_key) == default_value
 
-        set_value = parse_value('True', default_value)
+        # pick arbitrary new and env values for this configuration parameter
+        if isinstance(default_value, (bool, str)) or default_value is None:
+            set_value = parse_value('True', default_value)
+            env_value = parse_value('False', default_value)
+        else:
+            set_value = parse_value(2 * default_value + 1, default_value)
+            env_value = parse_value(2 * default_value - 1, default_value)
+
         set_setting(setting_key, set_value)
 
         # Make sure the value is correctly set and retrieved
@@ -81,7 +87,6 @@ def test_config_file_set(setting_key):
             assert get_setting(setting_key) == set_value
 
             # Otherwise, we get the env value
-            env_value = parse_value('False', default_value)
             os.environ[f"BENCHOPT_{setting_key.upper()}"] = str(env_value)
             assert get_setting(setting_key) == env_value
             del os.environ[f"BENCHOPT_{setting_key.upper()}"]