diff --git a/docs/source/whats_new.rst b/docs/source/whats_new.rst index f7f391a1bd..9d99cdcb8d 100644 --- a/docs/source/whats_new.rst +++ b/docs/source/whats_new.rst @@ -32,6 +32,13 @@ API changes folds that hold out multiple sessions now emit one result row per held-out session instead of one aggregate row per fold. The default leave-one-session-out behavior is unchanged (:gh:`1210` by `lindicaphxag-tech`_). +- Saved evaluation model paths now include the paradigm and optional suffix, so + separate benchmark runs no longer overwrite artifacts that otherwise share the + same evaluation/dataset/subject/session/pipeline keys (:gh:`1182` by + `lindicaphxag-tech`_). Existing legacy ``Models_*``/``GridSearch_*`` trees + remain untouched at their historical paths and are not auto-migrated because + those paths do not encode the missing paradigm/suffix provenance; legacy + artifacts therefore remain manually readable in place. Requirements ~~~~~~~~~~~~ diff --git a/examples/data_management_and_configuration/noplot_load_model.py b/examples/data_management_and_configuration/noplot_load_model.py index c0801c221c..5377487c50 100644 --- a/examples/data_management_and_configuration/noplot_load_model.py +++ b/examples/data_management_and_configuration/noplot_load_model.py @@ -30,9 +30,13 @@ ############################################################################### # Loading the Scikit-learn pipelines +# +# New model saves are namespaced by paradigm and benchmark suffix. Legacy model +# trees created by older MOABB versions are not moved and remain readable at +# their original paths. with open( - "../how_to_benchmark/results/Models_WithinSession/Zhou2016/1/0/csp+svm/fitted_model_best.pkl", + "../how_to_benchmark/results/Models_WithinSession/LeftRightImagery/benchmark/Zhou2016/1/0/csp+svm/fitted_model_best.pkl", "rb", ) as pickle_file: CSP_SVM_Trained = load(pickle_file) diff --git a/moabb/evaluations/base.py b/moabb/evaluations/base.py index b8fa875aee..c0c86a8026 100644 --- a/moabb/evaluations/base.py +++ b/moabb/evaluations/base.py @@ -269,6 +269,8 @@ def _evaluate_fold( name=pipeline_name, grid=is_search, eval_type=eval_type, + paradigm=config["paradigm"], + suffix=config["suffix"], ) _save_model_cv(model=cvclf, save_path=model_save_path, cv_index=str(cv_ind)) @@ -447,6 +449,7 @@ def __init__( self.n_jobs = n_jobs self.error_score = error_score self.hdf5_path = hdf5_path + self.suffix = suffix self.return_epochs = return_epochs self.return_raws = return_raws self.mne_labels = mne_labels @@ -680,6 +683,8 @@ def _maybe_save_model_cv( name=name, grid=self.search, eval_type=eval_type, + paradigm=type(self.paradigm).__name__, + suffix=self.suffix, ) _save_model_cv(model=model, save_path=model_save_path, cv_index=str(cv_ind)) @@ -747,6 +752,8 @@ def _build_eval_config(self, param_grid): "save_model": self.save_model, "hdf5_path": self.hdf5_path, "eval_type": self._eval_type or self.__class__.__name__, + "paradigm": type(self.paradigm).__name__, + "suffix": self.suffix, "mne_labels": self.mne_labels, "codecarbon_config": ( self.emissions.codecarbon_config if _carbonfootprint else None diff --git a/moabb/evaluations/utils.py b/moabb/evaluations/utils.py index 4a061cc83c..601fb62744 100644 --- a/moabb/evaluations/utils.py +++ b/moabb/evaluations/utils.py @@ -209,6 +209,8 @@ def _create_save_path( name: str, grid=False, eval_type="WithinSession", + paradigm=None, + suffix="", ): """Create a save path based on evaluation parameters. @@ -229,6 +231,10 @@ def _create_save_path( eval_type : str, optional The type of evaluation, either 'WithinSession', 'CrossSession' or 'CrossSubject'. Defaults to WithinSession. + paradigm : str, optional + The paradigm name. When provided, it namespaces saved models by paradigm. + suffix : str, optional + An optional run suffix used as an additional namespace for saved models. Returns ------- path_save: str @@ -238,24 +244,13 @@ def _create_save_path( if eval_type != "WithinSession": session = "" - if grid: - path_save = ( - Path(hdf5_path) - / f"GridSearch_{eval_type}" - / code - / f"{str(subject)}" - / str(session) - / str(name) - ) - else: - path_save = ( - Path(hdf5_path) - / f"Models_{eval_type}" - / code - / f"{str(subject)}" - / str(session) - / str(name) - ) + model_type = f"GridSearch_{eval_type}" if grid else f"Models_{eval_type}" + path_save = Path(hdf5_path) / model_type + if paradigm is not None: + path_save /= str(paradigm) + if suffix: + path_save /= str(suffix) + path_save /= Path(code) / str(subject) / str(session) / str(name) return str(path_save) else: diff --git a/moabb/tests/test_evaluations.py b/moabb/tests/test_evaluations.py index 24439aa24a..6ecc0745ab 100644 --- a/moabb/tests/test_evaluations.py +++ b/moabb/tests/test_evaluations.py @@ -3,6 +3,7 @@ import platform import warnings from collections import OrderedDict +from pathlib import Path import numpy as np import pandas as pd @@ -328,21 +329,24 @@ def test_eval_grid_search_optuna(self): def test_within_session_evaluation_save_model(self): res_test_path = "./res_test" + self.eval.suffix = "run_a" + process_pipeline = self.eval.paradigm.make_process_pipelines(dataset)[0] + list( + self.eval.evaluate( + dataset, pipelines, param_grid=None, process_pipeline=process_pipeline + ) + ) - # Get a list of all subdirectories inside 'res_test' - subdirectories = [ - d - for d in os.listdir(res_test_path) - if os.path.isdir(os.path.join(res_test_path, d)) - ] - - # Check if any of the subdirectories contain the partial name 'Model' - model_folder_exists = any("Model" in folder for folder in subdirectories) - - # Assert that at least one folder with the partial name 'Model' exists - assert model_folder_exists, ( - "No folder with partial name 'Model' found inside 'res_test' directory", + model_path = os.path.join( + res_test_path, + "Models_WithinSession", + type(self.eval.paradigm).__name__, + "run_a", ) + assert os.path.isdir(model_path), ( + "Saved models should be namespaced under their paradigm and suffix.", + ) + assert any(Path(model_path).rglob("fitted_model_*.pkl")) def test_lambda_warning(self): def explicit_kernel(x): @@ -1144,6 +1148,76 @@ def test_create_save_path(self): ) assert grid_save_path == expected_grid_path + def test_create_save_path_is_namespaced_by_paradigm_and_suffix(self): + save_path = create_save_path( + "base_path", + "evaluation_code", + 1, + "0", + "evaluation_name", + eval_type="WithinSession", + paradigm="MotorImagery", + suffix="run_a", + ) + + expected_path = os.path.join( + "base_path", + "Models_WithinSession", + "MotorImagery", + "run_a", + "evaluation_code", + "1", + "0", + "evaluation_name", + ) + assert save_path == expected_path + + other_run_path = create_save_path( + "base_path", + "evaluation_code", + 1, + "0", + "evaluation_name", + eval_type="WithinSession", + paradigm="SSVEP", + suffix="run_a", + ) + assert other_run_path != save_path + + other_suffix_path = create_save_path( + "base_path", + "evaluation_code", + 1, + "0", + "evaluation_name", + eval_type="WithinSession", + paradigm="MotorImagery", + suffix="run_b", + ) + assert other_suffix_path != save_path + + grid_save_path = create_save_path( + "base_path", + "evaluation_code", + 1, + "0", + "evaluation_name", + grid=True, + eval_type="WithinSession", + paradigm="MotorImagery", + suffix="run_a", + ) + assert grid_save_path == os.path.join( + "base_path", + "GridSearch_WithinSession", + "MotorImagery", + "run_a", + "evaluation_code", + "1", + "0", + "evaluation_name", + ) + def test_save_model_cv_with_pytorch_model(self): try: import torch