From 2b6eeac1d526c9a40866891acc574c9299c6a221 Mon Sep 17 00:00:00 2001 From: bartzbeielstein <32470350+bartzbeielstein@users.noreply.github.com> Date: Sun, 7 Jun 2026 17:47:17 +0200 Subject: [PATCH] fix(utils): clean the configured tensorboard_path, not only the runs/ dir tensorboard_clean=True previously removed subdirectories of a hardcoded relative "runs" directory in the CWD and ignored tensorboard_path entirely - with an explicit path the flag deleted unrelated logs and left the actual log directory untouched. Now, when tensorboard_path is set, the configured directory itself is removed (and re-created empty by the writer), so every run starts with a fresh dashboard; without a path the legacy runs/ cleanup is kept. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/spotoptim/SpotOptim.py | 12 ++++-- src/spotoptim/utils/tensorboard.py | 21 +++++++++- tests/test_tensorboard_clean.py | 63 ++++++++++++++++++++++++------ 3 files changed, 77 insertions(+), 19 deletions(-) diff --git a/src/spotoptim/SpotOptim.py b/src/spotoptim/SpotOptim.py index 1b510a60..29d17e4c 100644 --- a/src/spotoptim/SpotOptim.py +++ b/src/spotoptim/SpotOptim.py @@ -63,7 +63,8 @@ class SpotOptimConfig: ocba_delta (int): Delta for OCBA. tensorboard_log (bool): Whether to log to TensorBoard. tensorboard_path (Optional[str]): Path to TensorBoard logs. - tensorboard_clean (bool): Whether to clean TensorBoard logs. + tensorboard_clean (bool): Whether to clean old TensorBoard logs (the + configured tensorboard_path, or the 'runs' folder if no path is set). fun_mo2so (Optional[Callable]): Function to convert multi-objective to single-objective. seed (Optional[int]): Seed for random number generator. verbose (bool): Whether to print verbose output. @@ -360,9 +361,12 @@ class SpotOptim(BaseEstimator): Path for TensorBoard log files. If None and tensorboard_log is True, creates a default path: runs/spotoptim_YYYYMMDD_HHMMSS. Defaults to None. tensorboard_clean (bool, optional): - If True, removes all old TensorBoard log directories from - the 'runs' folder before starting optimization. Use with caution as this permanently - deletes all subdirectories in 'runs'. Defaults to False. + If True, removes old TensorBoard logs before starting optimization + so every run begins with a fresh dashboard. With tensorboard_path + set, the configured directory itself is removed (and re-created + empty by the writer); without a path, all subdirectories of the + default 'runs' folder are removed. Use with caution as this + permanently deletes the affected log directories. Defaults to False. fun_mo2so (callable, optional): Function to convert multi-objective values to single-objective. Takes an array of shape (n_samples, n_objectives) and returns array of shape (n_samples,). diff --git a/src/spotoptim/utils/tensorboard.py b/src/spotoptim/utils/tensorboard.py index 947adbf0..e4eddaf5 100644 --- a/src/spotoptim/utils/tensorboard.py +++ b/src/spotoptim/utils/tensorboard.py @@ -21,14 +21,31 @@ def clean_tensorboard_logs(optimizer: SpotOptimProtocol) -> None: - """Clean old TensorBoard log directories from the runs folder. + """Clean old TensorBoard logs if tensorboard_clean is True. - Removes all subdirectories in the 'runs' directory if tensorboard_clean is True. + When ``tensorboard_path`` is set, removes that directory (the writer + re-creates it empty afterwards), so every run starts with a fresh + dashboard. When ``tensorboard_path`` is None, falls back to the legacy + behavior of removing all subdirectories of the default ``runs`` + directory. Args: optimizer: SpotOptim instance. """ if optimizer.tensorboard_clean: + if optimizer.tensorboard_path is not None: + path = optimizer.tensorboard_path + if os.path.isdir(path): + try: + shutil.rmtree(path) + if optimizer.verbose: + print(f"Removed old TensorBoard logs: {path}") + except Exception as e: + if optimizer.verbose: + print(f"Warning: Could not remove {path}: {e}") + elif optimizer.verbose: + print(f"No old TensorBoard logs to clean at '{path}'") + return runs_dir = "runs" if os.path.exists(runs_dir) and os.path.isdir(runs_dir): subdirs = [ diff --git a/tests/test_tensorboard_clean.py b/tests/test_tensorboard_clean.py index d2c176e0..bdb14be4 100644 --- a/tests/test_tensorboard_clean.py +++ b/tests/test_tensorboard_clean.py @@ -230,12 +230,15 @@ def teardown_method(self): shutil.rmtree("runs") def test_clean_with_custom_tensorboard_path(self): - """Test that clean doesn't interfere with custom paths.""" - # Create custom directory + """Test that clean targets the custom path and leaves 'runs' alone.""" + # Create custom directory with a stale event file from a prior run custom_path = "my_logs/experiment_1" os.makedirs(custom_path, exist_ok=True) + stale_file = os.path.join(custom_path, "events.out.tfevents.stale") + with open(stale_file, "w") as f: + f.write("stale") - # Create some old logs in runs + # Create some old logs in runs (unrelated to the configured path) os.makedirs("runs/old_log", exist_ok=True) try: @@ -251,22 +254,56 @@ def test_clean_with_custom_tensorboard_path(self): verbose=False, ) - # Runs directory should be cleaned - if os.path.exists("runs"): - subdirs = [ - d - for d in os.listdir("runs") - if os.path.isdir(os.path.join("runs", d)) - ] - assert len(subdirs) == 0 + # Custom directory exists (re-created by the writer) but the + # stale event file from the previous run is gone + assert os.path.isdir(custom_path) + assert not os.path.exists(stale_file) - # Custom directory should still exist - assert os.path.exists(custom_path) + # The unrelated 'runs' directory is untouched + assert os.path.isdir("runs/old_log") finally: if os.path.exists("my_logs"): shutil.rmtree("my_logs") + def test_clean_with_custom_path_without_logging(self): + """Clean honors a custom path even when logging is disabled.""" + custom_path = "my_logs/experiment_2" + os.makedirs(custom_path, exist_ok=True) + with open(os.path.join(custom_path, "events.out.tfevents.stale"), "w") as f: + f.write("stale") + + try: + _ = SpotOptim( + fun=lambda X: np.sum(X**2, axis=1), + bounds=[(-5, 5), (-5, 5)], + max_iter=10, + n_initial=5, + tensorboard_path=custom_path, + tensorboard_clean=True, + verbose=False, + ) + + # Directory removed and not re-created (no writer without logging) + assert not os.path.exists(custom_path) + + finally: + if os.path.exists("my_logs"): + shutil.rmtree("my_logs") + + def test_clean_with_missing_custom_path(self): + """Clean handles a non-existent custom path gracefully.""" + optimizer = SpotOptim( + fun=lambda X: np.sum(X**2, axis=1), + bounds=[(-5, 5), (-5, 5)], + max_iter=10, + n_initial=5, + tensorboard_path="my_logs/does_not_exist", + tensorboard_clean=True, + verbose=False, + ) + assert optimizer.tensorboard_clean is True + def test_clean_with_nested_directories(self): """Test that clean handles nested structures correctly.""" # Create nested structure