[DRAFT] Protocol redesign #57
Conversation
…g train from CLI maintained.
… other searches . added search space items to EEGNet.yaml file. added search.py in utils, added run_sweep.py,
There was a problem hiding this comment.
Here are some initial comments from a cursory review. Things look generally good, but I wonder if the definition of the hyperparameter sweep configuration might be worth some further thought.
As noted in the meeting this could also support weights and biases.
| @@ -165,3 +215,29 @@ model: !new:models.EEGNet.EEGNet | |||
| dense_max_norm: !ref <dense_max_norm> | |||
| dropout: !ref <dropout> | |||
| dense_n_neurons: !ref <n_classes> | |||
|
|
|||
| # Search Space | |||
There was a problem hiding this comment.
I like the search space concept!
I wonder if there's a way to reference the search space in the rest of the yaml file -- like:
dropout: !ref <search_space.dropout>
There was a problem hiding this comment.
This might make it slightly clearer with how it works rather than having to use overrides. You might still have to reload the file each time, depending on how the reference worked exactly.
| class ExperimentRunner: | ||
| """Manages multiple MOABB experiment runs.""" | ||
|
|
||
| def __init__(self, args: list): |
There was a problem hiding this comment.
maybe the __init__ can take the relevant arguments directly and you can add a function like from_commandline to parse the command line and construct an object. e.g.
def __init__(self, hparams, data_folder, output_folder, nsubj, nsess):
"""do input validation or whatever"""
@classmethod
def from_commandline_args(cls, args):
"""Load arguments from command line"""
kwargs = self.parse_args(args)
return cls(**kwargs)
def parse_args(self, args):
parser = argparse.ArgumentParser()
parser.add_argument(...)
...
args = parser.parse_args(args)
return args.__dict__|
|
||
| """ | ||
| class MOABBBrain(sb.Brain): | ||
| """Modified Brain class for MOABB experiments with new data format.""" | ||
|
|
||
| def init_model(self, model): |
There was a problem hiding this comment.
Maybe model initialization should be done in the model files themselves?
| @@ -253,174 +387,109 @@ def check_if_best( | |||
|
|
|||
|
|
|||
| def run_experiment(hparams, run_opts, datasets): | |||
| """This function performs a single training (e.g., single cross-validation fold)""" | |||
| idx_examples = np.arange(datasets["train"].dataset.tensors[0].shape[0]) | |||
| """Run a single experiment with the new data format.""" | |||
There was a problem hiding this comment.
"New" data format may not mean anything to someone reading this file. Perhaps you can either explicitly write out the data format, or just state more clearly what the function does.
| return base_config, search_space | ||
|
|
||
|
|
||
| def load_search_space_only(yaml_path): |
There was a problem hiding this comment.
I wonder if there's a better way to do this somehow. If the search space can't be integrated with the other parameters as suggested above, maybe this is just better as a separate file?
This pull request contains the integration of the new dataio files and paradigms. It looks to replace the bash files that run the experiment and the hyper-parameter optimization with new python files. The main changes pertain to the train.py file that loads the data using dataio files, new run_experiments.py file, and later a new run_hparam_optimization.py file that integrates with WANDB.