From cde505c0a537606427bbbf9ebe82d19ba74aa304 Mon Sep 17 00:00:00 2001 From: Marcus Wieder <31651017+wiederm@users.noreply.github.com> Date: Tue, 13 Aug 2024 10:41:25 +0200 Subject: [PATCH 01/10] Update training.py add _add_tags method --- modelforge/train/training.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/modelforge/train/training.py b/modelforge/train/training.py index 95e781e3..492eb6c7 100644 --- a/modelforge/train/training.py +++ b/modelforge/train/training.py @@ -1172,7 +1172,35 @@ def _replace_placeholder_in_experimental_name(self, experiment_name: str) -> str "{dataset_name}", self.dataset_parameter.dataset_name ) return experiment_name + + def _add_tags(self, tags: List[str]) -> List[str]: + """ + Add tags to the wandb tags. + + Parameters + ---------- + tags : List[str] + List of tags to add to the experiment. + + Returns + ------- + List[str] + List of tags for the experiment. + """ + + # add version + import modelforge + + tags.append(str(modelforge.__version__)) + # add dataset + tags.append(self.dataset_config.dataset_name) + # add potential name + tags.append(self.potential_config.potential_name) + # add information about what is included in the loss + str_loss_property = "-".join(self.training_config.loss_parameter.loss_property) + tags.append(f"loss-{str_loss_property}") + return tags from typing import List, Optional, Union From 4273a61933a60bd87b313ac60fd023d0b5da6f62 Mon Sep 17 00:00:00 2001 From: wiederm Date: Tue, 13 Aug 2024 12:00:52 +0200 Subject: [PATCH 02/10] fix --- modelforge/train/training.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/modelforge/train/training.py b/modelforge/train/training.py index 492eb6c7..b404a936 100644 --- a/modelforge/train/training.py +++ b/modelforge/train/training.py @@ -1172,7 +1172,7 @@ def _replace_placeholder_in_experimental_name(self, experiment_name: str) -> str "{dataset_name}", self.dataset_parameter.dataset_name ) return experiment_name - + def _add_tags(self, tags: List[str]) -> List[str]: """ Add tags to the wandb tags. @@ -1193,15 +1193,16 @@ def _add_tags(self, tags: List[str]) -> List[str]: tags.append(str(modelforge.__version__)) # add dataset - tags.append(self.dataset_config.dataset_name) + tags.append(self.dataset_parameter.dataset_name) # add potential name - tags.append(self.potential_config.potential_name) + tags.append(self.potential_parameter.potential_name) # add information about what is included in the loss - str_loss_property = "-".join(self.training_config.loss_parameter.loss_property) + str_loss_property = "-".join(self.training_parameter.loss_parameter.loss_property) tags.append(f"loss-{str_loss_property}") return tags + from typing import List, Optional, Union From 403d8d304424fd6d489bc6e1ad6b0fa42bb60e8e Mon Sep 17 00:00:00 2001 From: wiederm Date: Tue, 13 Aug 2024 12:27:57 +0200 Subject: [PATCH 03/10] add callable to evaluate device command line tool, update training_run.sh --- modelforge/utils/io.py | 28 ++++++++++++++++++++++++++++ scripts/perform_training.py | 3 ++- scripts/training_run.sh | 2 +- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/modelforge/utils/io.py b/modelforge/utils/io.py index 875f8b16..c6f6d20b 100644 --- a/modelforge/utils/io.py +++ b/modelforge/utils/io.py @@ -244,3 +244,31 @@ def check_import(module: str): imported_module = import_(module) del imported_module + + +from typing import Union, List + + +def parse_devices(value: str) -> Union[int, List[int]]: + """ + Parse the devices argument which can be either a single integer or a list of + integers. + + Parameters + ---------- + value : str + The input string representing either a single integer or a list of + integers. + + Returns + ------- + Union[int, List[int]] + Either a single integer or a list of integers. + """ + import ast + + if value.startswith("[") and value.endswith("]"): + # Safely evaluate the string as a Python literal (list of ints) + return ast.literal_eval(value) + else: + return int(value) diff --git a/scripts/perform_training.py b/scripts/perform_training.py index 20819cea..e2f497ec 100644 --- a/scripts/perform_training.py +++ b/scripts/perform_training.py @@ -3,6 +3,7 @@ import argparse from modelforge.train.training import read_config_and_train from typing import Union, List +from modelforge.utils.io import parse_devices parse = argparse.ArgumentParser(description="Perform Training Using Modelforge") @@ -25,7 +26,7 @@ ) parse.add_argument("--accelerator", type=str, help="Accelerator to use for training") parse.add_argument( - "--devices", type=Union[int, List[int]], help="Device(s) to use for training" + "--devices", type=parse_devices, help="Device(s) to use for training" ) parse.add_argument( "--number_of_nodes", type=int, help="Number of nodes to use for training" diff --git a/scripts/training_run.sh b/scripts/training_run.sh index e20cc75a..ae89f90b 100644 --- a/scripts/training_run.sh +++ b/scripts/training_run.sh @@ -2,4 +2,4 @@ # training run with a small number of epochs with default # parameters -python perform_training.py config.toml \ No newline at end of file +python perform_training.py --condensed_config_path config.toml From 9b1cdae9d92b06825f58d256e6c0cb16d26f9107 Mon Sep 17 00:00:00 2001 From: wiederm Date: Tue, 13 Aug 2024 12:41:02 +0200 Subject: [PATCH 04/10] fix device_index not found --- modelforge/train/training.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modelforge/train/training.py b/modelforge/train/training.py index b404a936..71dc81c3 100644 --- a/modelforge/train/training.py +++ b/modelforge/train/training.py @@ -1197,7 +1197,9 @@ def _add_tags(self, tags: List[str]) -> List[str]: # add potential name tags.append(self.potential_parameter.potential_name) # add information about what is included in the loss - str_loss_property = "-".join(self.training_parameter.loss_parameter.loss_property) + str_loss_property = "-".join( + self.training_parameter.loss_parameter.loss_property + ) tags.append(f"loss-{str_loss_property}") return tags @@ -1332,7 +1334,7 @@ def read_config( runtime_parameters.accelerator = accelerator log.info(f"Using accelerator: {accelerator}") if devices: - runtime_parameters.device_index = devices + runtime_parameters.devices = devices log.info(f"Using device index: {devices}") if number_of_nodes: runtime_parameters.number_of_nodes = number_of_nodes From 53cb45b8dd49cc170da45b99721d0a9786c0585f Mon Sep 17 00:00:00 2001 From: wiederm Date: Tue, 13 Aug 2024 13:04:44 +0200 Subject: [PATCH 05/10] prioritize CLI arguments --- modelforge/train/training.py | 219 +++++++++++++++++++++++------------ 1 file changed, 148 insertions(+), 71 deletions(-) diff --git a/modelforge/train/training.py b/modelforge/train/training.py index 71dc81c3..ee5b6e4d 100644 --- a/modelforge/train/training.py +++ b/modelforge/train/training.py @@ -1206,6 +1206,97 @@ def _add_tags(self, tags: List[str]) -> List[str]: from typing import List, Optional, Union +def read_config( + condensed_config_path: Optional[str] = None, + training_parameter_path: Optional[str] = None, + dataset_parameter_path: Optional[str] = None, + potential_parameter_path: Optional[str] = None, + runtime_parameter_path: Optional[str] = None, + accelerator: Optional[str] = None, + devices: Optional[Union[int, List[int]]] = None, + number_of_nodes: Optional[int] = None, + experiment_name: Optional[str] = None, + save_dir: Optional[str] = None, + local_cache_dir: Optional[str] = None, + checkpoint_path: Optional[str] = None, + log_every_n_steps: Optional[int] = None, + simulation_environment: Optional[str] = None, +): + """ + Reads one or more TOML configuration files and loads them into the pydantic models. + + Parameters + ---------- + (Parameters as described earlier...) + + Returns + ------- + Tuple + Tuple containing the training, dataset, potential, and runtime parameters. + """ + import toml + + # Initialize the config dictionaries + training_config_dict = {} + dataset_config_dict = {} + potential_config_dict = {} + runtime_config_dict = {} + + if condensed_config_path is not None: + config = toml.load(condensed_config_path) + log.info(f"Reading config from : {condensed_config_path}") + + training_config_dict = config.get("training", {}) + dataset_config_dict = config.get("dataset", {}) + potential_config_dict = config.get("potential", {}) + runtime_config_dict = config.get("runtime", {}) + + else: + if training_parameter_path: + training_config_dict = toml.load(training_parameter_path).get("training", {}) + if dataset_parameter_path: + dataset_config_dict = toml.load(dataset_parameter_path).get("dataset", {}) + if potential_parameter_path: + potential_config_dict = toml.load(potential_parameter_path).get("potential", {}) + if runtime_parameter_path: + runtime_config_dict = toml.load(runtime_parameter_path).get("runtime", {}) + + # Override runtime configuration with command-line arguments if provided + runtime_overrides = { + "accelerator": accelerator, + "devices": devices, + "number_of_nodes": number_of_nodes, + "experiment_name": experiment_name, + "save_dir": save_dir, + "local_cache_dir": local_cache_dir, + "checkpoint_path": checkpoint_path, + "log_every_n_steps": log_every_n_steps, + "simulation_environment": simulation_environment, + } + + for key, value in runtime_overrides.items(): + if value is not None: + runtime_config_dict[key] = value + + # Load and instantiate the data classes with the merged configuration + from modelforge.potential import _Implemented_NNP_Parameters + from modelforge.dataset.dataset import DatasetParameters + from modelforge.train.parameters import TrainingParameters, RuntimeParameters + + potential_name = potential_config_dict["potential_name"] + PotentialParameters = _Implemented_NNP_Parameters.get_neural_network_parameter_class(potential_name) + + dataset_parameters = DatasetParameters(**dataset_config_dict) + training_parameters = TrainingParameters(**training_config_dict) + runtime_parameters = RuntimeParameters(**runtime_config_dict) + potential_parameter = PotentialParameters(**potential_config_dict) + + return ( + training_parameters, + dataset_parameters, + potential_parameter, + runtime_parameters, + ) def read_config( @@ -1224,14 +1315,17 @@ def read_config( log_every_n_steps: Optional[int] = None, simulation_environment: Optional[str] = None, ): + """ - Reads one or more TOML configuration files and loads them into the pydantic models + Reads one or more TOML configuration files and loads them into the pydantic + models Parameters ---------- condensed_config_path : str, optional - Path to the TOML configuration that contains all parameters for the dataset, potential, training, and runtime parameters. - Any other provided configuration files will be ignored. + Path to the TOML configuration that contains all parameters for the + dataset, potential, training, and runtime parameters. Any other provided + configuration files will be ignored. training_parameter_path : str, optional Path to the TOML file defining the training parameters. dataset_parameter_path : str, optional @@ -1239,36 +1333,52 @@ def read_config( potential_parameter_path : str, optional Path to the TOML file defining the potential parameters. runtime_parameter_path : str, optional - Path to the TOML file defining the runtime parameters. If this is not provided, the code will attempt to use - the runtime parameters provided as arguments. + Path to the TOML file defining the runtime parameters. If this is not + provided, the code will attempt to use the runtime parameters provided + as arguments. accelerator : str, optional - Accelerator type to use. If provided, this overrides the accelerator type in the runtime_defaults configuration. + Accelerator type to use. If provided, this overrides the accelerator + type in the runtime_defaults configuration. devices : int|List[int], optional - Device index/indices to use. If provided, this overrides the devices in the runtime_defaults configuration. + Device index/indices to use. If provided, this overrides the devices in + the runtime_defaults configuration. number_of_nodes : int, optional - Number of nodes to use. If provided, this overrides the number of nodes in the runtime_defaults configuration. + Number of nodes to use. If provided, this overrides the number of nodes + in the runtime_defaults configuration. experiment_name : str, optional - Name of the experiment. If provided, this overrides the experiment name in the runtime_defaults configuration. + Name of the experiment. If provided, this overrides the experiment name + in the runtime_defaults configuration. save_dir : str, optional - Directory to save the model. If provided, this overrides the save directory in the runtime_defaults configuration. + Directory to save the model. If provided, this overrides the save + directory in the runtime_defaults configuration. local_cache_dir : str, optional - Local cache directory. If provided, this overrides the local cache directory in the runtime_defaults configuration. + Local cache directory. If provided, this overrides the local cache + directory in the runtime_defaults configuration. checkpoint_path : str, optional - Path to the checkpoint file. If provided, this overrides the checkpoint path in the runtime_defaults configuration. + Path to the checkpoint file. If provided, this overrides the checkpoint + path in the runtime_defaults configuration. log_every_n_steps : int, optional - Number of steps to log. If provided, this overrides the log_every_n_steps in the runtime_defaults configuration. + Number of steps to log. If provided, this overrides the + log_every_n_steps in the runtime_defaults configuration. simulation_environment : str, optional - Simulation environment. If provided, this overrides the simulation environment in the runtime_defaults configuration. + Simulation environment. If provided, this overrides the simulation + environment in the runtime_defaults configuration. Returns ------- Tuple - Tuple containing the training, dataset, potential, and runtime parameters. + Tuple containing the training, dataset, potential, and runtime + parameters. """ import toml - use_runtime_variables_instead_of_toml = False + # Initialize the config dictionaries + training_config_dict = {} + dataset_config_dict = {} + potential_config_dict = {} + runtime_config_dict = {} + if condensed_config_path is not None: config = toml.load(condensed_config_path) log.info(f"Reading config from : {condensed_config_path}") @@ -1285,32 +1395,32 @@ def read_config( raise ValueError("Dataset configuration not provided.") if potential_parameter_path is None: raise ValueError("Potential configuration not provided.") + if runtime_parameter_path is None: + raise ValueError("Runtime configuration not provided.") training_config_dict = toml.load(training_parameter_path)["training"] dataset_config_dict = toml.load(dataset_parameter_path)["dataset"] potential_config_dict = toml.load(potential_parameter_path)["potential"] - - # if the runtime_parameter_path is not defined, let us see if runtime variables are passed - if runtime_parameter_path is None: - use_runtime_variables_instead_of_toml = True - log.info( - "Runtime configuration not provided. The code will try to use runtime arguments." - ) - # we can just create a dict with the runtime variables; the pydantic model will then validate them - runtime_config_dict = { - "save_dir": save_dir, - "experiment_name": experiment_name, - "local_cache_dir": local_cache_dir, - "checkpoint_path": checkpoint_path, - "log_every_n_steps": log_every_n_steps, - "simulation_environment": simulation_environment, - "accelerator": accelerator, - "devices": devices, - "number_of_nodes": number_of_nodes, - } - else: - runtime_config_dict = toml.load(runtime_parameter_path)["runtime"] - + runtime_config_dict = toml.load(runtime_parameter_path)["potential"] + + # Override runtime configuration with command-line arguments if provided + runtime_overrides = { + "accelerator": accelerator, + "devices": devices, + "number_of_nodes": number_of_nodes, + "experiment_name": experiment_name, + "save_dir": save_dir, + "local_cache_dir": local_cache_dir, + "checkpoint_path": checkpoint_path, + "log_every_n_steps": log_every_n_steps, + "simulation_environment": simulation_environment, + } + + for key, value in runtime_overrides.items(): + if value is not None: + runtime_config_dict[key] = value + + # Load and instantiate the data classes with the merged configuration from modelforge.potential import _Implemented_NNP_Parameters from modelforge.dataset.dataset import DatasetParameters from modelforge.train.parameters import TrainingParameters, RuntimeParameters @@ -1325,39 +1435,6 @@ def read_config( runtime_parameters = RuntimeParameters(**runtime_config_dict) potential_parameter_paths = PotentialParameters(**potential_config_dict) - # if accelerator, devices, or number_of_nodes are provided, override the runtime_defaults parameters - # note, since these are being set in the runtime data model, they will be validated by the model - # if we use the runtime variables instead of the toml file, these have already been set so we can skip this step. - - if use_runtime_variables_instead_of_toml == False: - if accelerator: - runtime_parameters.accelerator = accelerator - log.info(f"Using accelerator: {accelerator}") - if devices: - runtime_parameters.devices = devices - log.info(f"Using device index: {devices}") - if number_of_nodes: - runtime_parameters.number_of_nodes = number_of_nodes - log.info(f"Using number of nodes: {number_of_nodes}") - if experiment_name: - runtime_parameters.experiment_name = experiment_name - log.info(f"Using experiment name: {experiment_name}") - if save_dir: - runtime_parameters.save_dir = save_dir - log.info(f"Using save directory: {save_dir}") - if local_cache_dir: - runtime_parameters.local_cache_dir = local_cache_dir - log.info(f"Using local cache directory: {local_cache_dir}") - if checkpoint_path: - runtime_parameters.checkpoint_path = checkpoint_path - log.info(f"Using checkpoint path: {checkpoint_path}") - if log_every_n_steps: - runtime_parameters.log_every_n_steps = log_every_n_steps - log.info(f"Logging every {log_every_n_steps} steps.") - if simulation_environment: - runtime_parameters.simulation_environment = simulation_environment - log.info(f"Using simulation environment: {simulation_environment}") - return ( training_parameters, dataset_parameters, From dd90193f304daa3aa4f6568475c40b98ebd89dbf Mon Sep 17 00:00:00 2001 From: wiederm Date: Tue, 13 Aug 2024 13:29:40 +0200 Subject: [PATCH 06/10] fix --- modelforge/train/training.py | 142 ----------------------------------- 1 file changed, 142 deletions(-) diff --git a/modelforge/train/training.py b/modelforge/train/training.py index ee5b6e4d..afabb97b 100644 --- a/modelforge/train/training.py +++ b/modelforge/train/training.py @@ -1299,148 +1299,6 @@ def read_config( ) -def read_config( - condensed_config_path: Optional[str] = None, - training_parameter_path: Optional[str] = None, - dataset_parameter_path: Optional[str] = None, - potential_parameter_path: Optional[str] = None, - runtime_parameter_path: Optional[str] = None, - accelerator: Optional[str] = None, - devices: Optional[Union[int, List[int]]] = None, - number_of_nodes: Optional[int] = None, - experiment_name: Optional[str] = None, - save_dir: Optional[str] = None, - local_cache_dir: Optional[str] = None, - checkpoint_path: Optional[str] = None, - log_every_n_steps: Optional[int] = None, - simulation_environment: Optional[str] = None, -): - - """ - Reads one or more TOML configuration files and loads them into the pydantic - models - - Parameters - ---------- - condensed_config_path : str, optional - Path to the TOML configuration that contains all parameters for the - dataset, potential, training, and runtime parameters. Any other provided - configuration files will be ignored. - training_parameter_path : str, optional - Path to the TOML file defining the training parameters. - dataset_parameter_path : str, optional - Path to the TOML file defining the dataset parameters. - potential_parameter_path : str, optional - Path to the TOML file defining the potential parameters. - runtime_parameter_path : str, optional - Path to the TOML file defining the runtime parameters. If this is not - provided, the code will attempt to use the runtime parameters provided - as arguments. - accelerator : str, optional - Accelerator type to use. If provided, this overrides the accelerator - type in the runtime_defaults configuration. - devices : int|List[int], optional - Device index/indices to use. If provided, this overrides the devices in - the runtime_defaults configuration. - number_of_nodes : int, optional - Number of nodes to use. If provided, this overrides the number of nodes - in the runtime_defaults configuration. - experiment_name : str, optional - Name of the experiment. If provided, this overrides the experiment name - in the runtime_defaults configuration. - save_dir : str, optional - Directory to save the model. If provided, this overrides the save - directory in the runtime_defaults configuration. - local_cache_dir : str, optional - Local cache directory. If provided, this overrides the local cache - directory in the runtime_defaults configuration. - checkpoint_path : str, optional - Path to the checkpoint file. If provided, this overrides the checkpoint - path in the runtime_defaults configuration. - log_every_n_steps : int, optional - Number of steps to log. If provided, this overrides the - log_every_n_steps in the runtime_defaults configuration. - simulation_environment : str, optional - Simulation environment. If provided, this overrides the simulation - environment in the runtime_defaults configuration. - - Returns - ------- - Tuple - Tuple containing the training, dataset, potential, and runtime - parameters. - - """ - import toml - - # Initialize the config dictionaries - training_config_dict = {} - dataset_config_dict = {} - potential_config_dict = {} - runtime_config_dict = {} - - if condensed_config_path is not None: - config = toml.load(condensed_config_path) - log.info(f"Reading config from : {condensed_config_path}") - - training_config_dict = config["training"] - dataset_config_dict = config["dataset"] - potential_config_dict = config["potential"] - runtime_config_dict = config["runtime"] - - else: - if training_parameter_path is None: - raise ValueError("Training configuration not provided.") - if dataset_parameter_path is None: - raise ValueError("Dataset configuration not provided.") - if potential_parameter_path is None: - raise ValueError("Potential configuration not provided.") - if runtime_parameter_path is None: - raise ValueError("Runtime configuration not provided.") - - training_config_dict = toml.load(training_parameter_path)["training"] - dataset_config_dict = toml.load(dataset_parameter_path)["dataset"] - potential_config_dict = toml.load(potential_parameter_path)["potential"] - runtime_config_dict = toml.load(runtime_parameter_path)["potential"] - - # Override runtime configuration with command-line arguments if provided - runtime_overrides = { - "accelerator": accelerator, - "devices": devices, - "number_of_nodes": number_of_nodes, - "experiment_name": experiment_name, - "save_dir": save_dir, - "local_cache_dir": local_cache_dir, - "checkpoint_path": checkpoint_path, - "log_every_n_steps": log_every_n_steps, - "simulation_environment": simulation_environment, - } - - for key, value in runtime_overrides.items(): - if value is not None: - runtime_config_dict[key] = value - - # Load and instantiate the data classes with the merged configuration - from modelforge.potential import _Implemented_NNP_Parameters - from modelforge.dataset.dataset import DatasetParameters - from modelforge.train.parameters import TrainingParameters, RuntimeParameters - - potential_name = potential_config_dict["potential_name"] - PotentialParameters = ( - _Implemented_NNP_Parameters.get_neural_network_parameter_class(potential_name) - ) - - dataset_parameters = DatasetParameters(**dataset_config_dict) - training_parameters = TrainingParameters(**training_config_dict) - runtime_parameters = RuntimeParameters(**runtime_config_dict) - potential_parameter_paths = PotentialParameters(**potential_config_dict) - - return ( - training_parameters, - dataset_parameters, - potential_parameter_paths, - runtime_parameters, - ) def read_config_and_train( From be2fa5a587de2f5dd9cf3a22c90f156b6d936a62 Mon Sep 17 00:00:00 2001 From: wiederm Date: Tue, 13 Aug 2024 13:38:06 +0200 Subject: [PATCH 07/10] fix device device_index_must_be_positive check --- modelforge/train/parameters.py | 5 +++-- modelforge/train/training.py | 3 --- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/modelforge/train/parameters.py b/modelforge/train/parameters.py index 489f056a..95bf3569 100644 --- a/modelforge/train/parameters.py +++ b/modelforge/train/parameters.py @@ -346,6 +346,7 @@ def device_index_must_be_positive(cls, v) -> Union[int, List[int]]: for device in v: if device < 0: raise ValueError("device_index must be positive") - if v < 0: - raise ValueError("device_index must be positive") + else: + if v < 0: + raise ValueError("device_index must be positive") return v diff --git a/modelforge/train/training.py b/modelforge/train/training.py index afabb97b..18b90a91 100644 --- a/modelforge/train/training.py +++ b/modelforge/train/training.py @@ -1298,9 +1298,6 @@ def read_config( runtime_parameters, ) - - - def read_config_and_train( condensed_config_path: Optional[str] = None, training_parameter_path: Optional[str] = None, From 02fa63983c2625834a0a8a24531f6bdd22289d43 Mon Sep 17 00:00:00 2001 From: chrisiacovella Date: Tue, 13 Aug 2024 09:37:52 -0700 Subject: [PATCH 08/10] skip download from figshare test, as it seems to timeout (and then fail) often on the CI. I'm planning on refactoring the code to remove this functionality, as we do not need a specialized function for this, but can just provide more info than querying the API which seems problematic at times. --- modelforge/tests/test_remote.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modelforge/tests/test_remote.py b/modelforge/tests/test_remote.py index e6aaf4bd..fbb1d1c0 100644 --- a/modelforge/tests/test_remote.py +++ b/modelforge/tests/test_remote.py @@ -71,6 +71,9 @@ def test_download_from_url(prep_temp_dir): ) +@pytest.mark.skip( + reason="This test seems to time out on the CI frequently. Will be refactoring and not need this soon." +) def test_download_from_figshare(prep_temp_dir): url = "https://figshare.com/ndownloader/files/22247589" name = download_from_figshare( From 53d853f0cc0050d87528dd239fce5feaa2b40d41 Mon Sep 17 00:00:00 2001 From: chrisiacovella Date: Tue, 13 Aug 2024 09:57:57 -0700 Subject: [PATCH 09/10] Quick change to device parsing; if a comma appears in the string, perform list evaluation, so input can be of form "[0,1]", "(0,1)" or just "0,1" --- modelforge/utils/io.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modelforge/utils/io.py b/modelforge/utils/io.py index c6f6d20b..2ba90ca8 100644 --- a/modelforge/utils/io.py +++ b/modelforge/utils/io.py @@ -267,8 +267,9 @@ def parse_devices(value: str) -> Union[int, List[int]]: """ import ast - if value.startswith("[") and value.endswith("]"): + # if multiple comma delimited values are passed, split them into a list + if "," in value: # Safely evaluate the string as a Python literal (list of ints) - return ast.literal_eval(value) + return list(ast.literal_eval(value)) else: return int(value) From dedf9d4d8deebed9c34937b1e23d5fc99c4e5d79 Mon Sep 17 00:00:00 2001 From: chrisiacovella Date: Tue, 13 Aug 2024 11:20:43 -0700 Subject: [PATCH 10/10] changed back so it better covers cases --- modelforge/utils/io.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modelforge/utils/io.py b/modelforge/utils/io.py index 2ba90ca8..8f0eab42 100644 --- a/modelforge/utils/io.py +++ b/modelforge/utils/io.py @@ -268,7 +268,7 @@ def parse_devices(value: str) -> Union[int, List[int]]: import ast # if multiple comma delimited values are passed, split them into a list - if "," in value: + if value.startswith("[") and value.endswith("]"): # Safely evaluate the string as a Python literal (list of ints) return list(ast.literal_eval(value)) else: