From e15906bbcd6e2b7a9fb85c9b0db41d63b59c7ae6 Mon Sep 17 00:00:00 2001 From: Matthew Drozt Date: Tue, 30 Nov 2021 14:14:36 -0600 Subject: [PATCH 1/8] Remove pandas from experiment.py --- smartsim/experiment.py | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/smartsim/experiment.py b/smartsim/experiment.py index 44a6a8b34..6a2a58646 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -29,7 +29,7 @@ from os import getcwd from pprint import pformat -import pandas as pd +from tabulate import tabulate from tqdm import trange from .control import Controller, Manifest @@ -413,25 +413,24 @@ def summary(self): The summary will show each instance that has been launched and completed in this ``Experiment`` - :return: pandas Dataframe of ``Experiment`` history - :rtype: pd.DataFrame + :return: tabulate string of ``Experiment`` history + :rtype: str """ - index = 0 - df = pd.DataFrame( - columns=[ - "Name", - "Entity-Type", - "JobID", - "RunID", - "Time", - "Status", - "Returncode", - ] - ) + values = [] + headers=[ + "Name", + "Entity-Type", + "JobID", + "RunID", + "Time", + "Status", + "Returncode", + ] + # TODO should this include running jobs? for job in self._control._jobs.completed.values(): for run in range(job.history.runs + 1): - df.loc[index] = [ + values.append([ job.entity.name, job.entity.type, job.history.jids[run], @@ -439,9 +438,8 @@ def summary(self): job.history.job_times[run], job.history.statuses[run], job.history.returns[run], - ] - index += 1 - return df + ]) + return tabulate(values, headers, showindex=True) def _launch_summary(self, manifest): """Experiment pre-launch summary of entities that will be launched From 86c03b143c219268abc27f7b0c266c677e109db0 Mon Sep 17 00:00:00 2001 From: Matthew Drozt Date: Tue, 30 Nov 2021 14:36:22 -0600 Subject: [PATCH 2/8] Update requirements to include tabulate and remove pd --- requirements-dev.txt | 2 +- requirements.txt | 2 +- setup.cfg | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 4fbe6373a..34111ea2e 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -7,7 +7,7 @@ numpy>=1.18.2 toml>=0.10.1 tqdm>=4.50.2 psutil>=5.7.2 -pandas>=1.1.3 +tabulate>=0.8.9 black>=20.8b1 isort>=5.6.4 pylint>=2.6.0 diff --git a/requirements.txt b/requirements.txt index 920facc7a..5d4dbd4a9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ psutil>=5.7.2 coloredlogs==10.0 -pandas>=1.1.3 +tabulate>=0.8.9 smartredis>=0.1.1 redis==3.5.3 redis-py-cluster==2.1.3 diff --git a/setup.cfg b/setup.cfg index 767a3d6eb..78268b4d6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -33,7 +33,7 @@ include_package_data = True install_requires = psutil>=5.7.2 coloredlogs==10.0 - pandas>=1.1.3 + tabulate>=0.8.9 smartredis>=0.1.1 redis-py-cluster==2.1.3 redis==3.5.3 From f60357fafd637c9b90b026a68a1aa01ad442c783 Mon Sep 17 00:00:00 2001 From: Matthew Drozt Date: Fri, 3 Dec 2021 13:37:54 -0600 Subject: [PATCH 3/8] Change table fmt to match pd.df, run black --- smartsim/experiment.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/smartsim/experiment.py b/smartsim/experiment.py index 6a2a58646..00f43a673 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -417,7 +417,7 @@ def summary(self): :rtype: str """ values = [] - headers=[ + headers = [ "Name", "Entity-Type", "JobID", @@ -430,16 +430,18 @@ def summary(self): # TODO should this include running jobs? for job in self._control._jobs.completed.values(): for run in range(job.history.runs + 1): - values.append([ - job.entity.name, - job.entity.type, - job.history.jids[run], - run, - job.history.job_times[run], - job.history.statuses[run], - job.history.returns[run], - ]) - return tabulate(values, headers, showindex=True) + values.append( + [ + job.entity.name, + job.entity.type, + job.history.jids[run], + run, + job.history.job_times[run], + job.history.statuses[run], + job.history.returns[run], + ] + ) + return tabulate(values, headers, showindex=True, tablefmt="plain") def _launch_summary(self, manifest): """Experiment pre-launch summary of entities that will be launched From d2a854e57e9f25490b28d76820c47d09a3f1b41c Mon Sep 17 00:00:00 2001 From: Matthew Drozt Date: Fri, 3 Dec 2021 14:00:28 -0600 Subject: [PATCH 4/8] ake style --- smartsim/entity/ensemble.py | 9 +++------ smartsim/entity/model.py | 16 ++++++++++------ tests/backends/run_sklearn_onnx.py | 1 - tests/backends/run_tf.py | 2 +- tests/backends/run_torch.py | 1 - tests/test_configs/smartredis/consumer.py | 1 - tests/test_configs/smartredis/producer.py | 1 - tests/test_ensemble.py | 6 +++++- tests/test_smartredis.py | 3 +-- 9 files changed, 20 insertions(+), 20 deletions(-) diff --git a/smartsim/entity/ensemble.py b/smartsim/entity/ensemble.py index 74ebdc0e1..c02b4d2fe 100644 --- a/smartsim/entity/ensemble.py +++ b/smartsim/entity/ensemble.py @@ -86,7 +86,7 @@ def __init__( :rtype: ``Ensemble`` """ self.params = init_default({}, params, dict) - self.params_as_args = init_default({}, params_as_args, (list,str)) + self.params_as_args = init_default({}, params_as_args, (list, str)) self._key_prefixing_enabled = True self.batch_settings = init_default({}, batch_settings, BatchSettings) self.run_settings = init_default({}, run_settings, RunSettings) @@ -113,9 +113,7 @@ def _initialize_entities(self, **kwargs): param_names, params = self._read_model_parameters() # Compute all combinations of model parameters and arguments - all_model_params = strategy( - param_names, params, **kwargs - ) + all_model_params = strategy(param_names, params, **kwargs) if not isinstance(all_model_params, list): raise UserStrategyError(strategy) @@ -171,7 +169,6 @@ def _initialize_entities(self, **kwargs): else: logger.info("Empty ensemble created for batch launch") - def add_model(self, model): """Add a model to this ensemble @@ -300,4 +297,4 @@ def _read_model_parameters(self): "Incorrect type for ensemble parameters\n" + "Must be list, int, or string." ) - return param_names, parameters \ No newline at end of file + return param_names, parameters diff --git a/smartsim/entity/model.py b/smartsim/entity/model.py index a89a287df..235c3d289 100644 --- a/smartsim/entity/model.py +++ b/smartsim/entity/model.py @@ -25,6 +25,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. from smartsim.error.errors import SSConfigError + from ..error import EntityExistsError from ..utils.helpers import cat_arg_and_value, init_default from .entity import SmartSimEntity @@ -119,15 +120,18 @@ def attach_generator_files(self, to_copy=None, to_symlink=None, to_configure=Non self.files = EntityFiles(to_configure, to_copy, to_symlink) def params_to_args(self): - """Convert parameters to command line arguments and update run settings. - """ + """Convert parameters to command line arguments and update run settings.""" for param in self.params_as_args: if not param in self.params: - raise SSConfigError(f"Tried to convert {param} to command line argument " + - f"for Model {self.name}, but its value was not found in model params") + raise SSConfigError( + f"Tried to convert {param} to command line argument " + + f"for Model {self.name}, but its value was not found in model params" + ) if self.run_settings is None: - raise SSConfigError(f"Tried to configure command line parameter for Model {self.name}, " + - "but no RunSettings are set.") + raise SSConfigError( + f"Tried to configure command line parameter for Model {self.name}, " + + "but no RunSettings are set." + ) self.run_settings.add_exe_args(cat_arg_and_value(param, self.params[param])) def __eq__(self, other): diff --git a/tests/backends/run_sklearn_onnx.py b/tests/backends/run_sklearn_onnx.py index 157e5a815..f2c898a54 100644 --- a/tests/backends/run_sklearn_onnx.py +++ b/tests/backends/run_sklearn_onnx.py @@ -5,7 +5,6 @@ from sklearn.ensemble import RandomForestRegressor from sklearn.linear_model import LinearRegression from sklearn.model_selection import train_test_split - from smartredis import Client diff --git a/tests/backends/run_tf.py b/tests/backends/run_tf.py index 1003ab778..82151ef96 100644 --- a/tests/backends/run_tf.py +++ b/tests/backends/run_tf.py @@ -1,9 +1,9 @@ import os import numpy as np +from smartredis import Client from tensorflow import keras -from smartredis import Client from smartsim.tf import freeze_model diff --git a/tests/backends/run_torch.py b/tests/backends/run_torch.py index 8e3ff585f..0d4bfa4cc 100644 --- a/tests/backends/run_torch.py +++ b/tests/backends/run_torch.py @@ -4,7 +4,6 @@ import torch import torch.nn as nn import torch.nn.functional as F - from smartredis import Client diff --git a/tests/test_configs/smartredis/consumer.py b/tests/test_configs/smartredis/consumer.py index 9cfdecc08..d3b90d517 100644 --- a/tests/test_configs/smartredis/consumer.py +++ b/tests/test_configs/smartredis/consumer.py @@ -5,7 +5,6 @@ import numpy as np import torch import torch.nn as nn - from smartredis import Client if __name__ == "__main__": diff --git a/tests/test_configs/smartredis/producer.py b/tests/test_configs/smartredis/producer.py index baf8f16d6..2f1c00690 100644 --- a/tests/test_configs/smartredis/producer.py +++ b/tests/test_configs/smartredis/producer.py @@ -5,7 +5,6 @@ import numpy as np import torch import torch.nn as nn - from smartredis import Client diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index 9df3e6fa8..65f173a86 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -140,7 +140,11 @@ def test_arg_and_model_params_step(): rs_copy = deepcopy(rs) rs_orig_args = rs_copy.exe_args ensemble = Ensemble( - "step", params, params_as_args=["H", "g_param"], run_settings=rs_copy, perm_strat="step" + "step", + params, + params_as_args=["H", "g_param"], + run_settings=rs_copy, + perm_strat="step", ) assert len(ensemble) == 2 diff --git a/tests/test_smartredis.py b/tests/test_smartredis.py index 19e884690..eaf4f0397 100644 --- a/tests/test_smartredis.py +++ b/tests/test_smartredis.py @@ -22,9 +22,8 @@ shouldrun = True try: - import torch - import smartredis + import torch except ImportError: shouldrun = False From ad94a40708a230b08d931ff263d2ab0520693c33 Mon Sep 17 00:00:00 2001 From: Matthew Drozt Date: Tue, 7 Dec 2021 12:20:22 -0600 Subject: [PATCH 5/8] refactor test to work with str from summary --- tests/on_wlm/test_simple_entity_launch.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/on_wlm/test_simple_entity_launch.py b/tests/on_wlm/test_simple_entity_launch.py index 8e28f7fc5..d8ffd63c4 100644 --- a/tests/on_wlm/test_simple_entity_launch.py +++ b/tests/on_wlm/test_simple_entity_launch.py @@ -77,16 +77,19 @@ def test_summary(fileutils, wlmutils): assert exp.get_status(bad)[0] == constants.STATUS_FAILED assert exp.get_status(sleep)[0] == constants.STATUS_COMPLETED - summary_df = exp.summary() - print(summary_df) - row = summary_df.loc[0] + summary_str = exp.summary() + rows = [s.split() for s in summary_str.split('\n')] + headers = ["Index"] + rows.pop(0) + + print(summary_str) + row = { head: val for head, val in zip(headers, rows[0]) } assert sleep.name == row["Name"] assert sleep.type == row["Entity-Type"] assert 0 == int(row["RunID"]) assert 0 == int(row["Returncode"]) - row_1 = summary_df.loc[1] + row_1 = { head: val for head, val in zip(headers, rows[1]) } assert bad.name == row_1["Name"] assert bad.type == row_1["Entity-Type"] From d3aaab5133f59fc3b93fb89646595c4da74c006a Mon Sep 17 00:00:00 2001 From: Matthew Drozt Date: Tue, 7 Dec 2021 14:06:46 -0600 Subject: [PATCH 6/8] add summary test to test_testexperiment.py, refactor test_simple_entity_launch.py summary test --- tests/on_wlm/test_simple_entity_launch.py | 11 +++++------ tests/test_experiment.py | 24 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/tests/on_wlm/test_simple_entity_launch.py b/tests/on_wlm/test_simple_entity_launch.py index d8ffd63c4..277a4a8c6 100644 --- a/tests/on_wlm/test_simple_entity_launch.py +++ b/tests/on_wlm/test_simple_entity_launch.py @@ -78,19 +78,18 @@ def test_summary(fileutils, wlmutils): assert exp.get_status(sleep)[0] == constants.STATUS_COMPLETED summary_str = exp.summary() - rows = [s.split() for s in summary_str.split('\n')] - headers = ["Index"] + rows.pop(0) - print(summary_str) - row = { head: val for head, val in zip(headers, rows[0]) } + rows = [s.split() for s in summary_str.split("\n")] + headers = ["Index"] + rows.pop(0) + + row = dict(zip(headers, rows[0])) assert sleep.name == row["Name"] assert sleep.type == row["Entity-Type"] assert 0 == int(row["RunID"]) assert 0 == int(row["Returncode"]) - row_1 = { head: val for head, val in zip(headers, rows[1]) } - + row_1 = dict(zip(headers, rows[1])) assert bad.name == row_1["Name"] assert bad.type == row_1["Entity-Type"] assert 0 == int(row_1["RunID"]) diff --git a/tests/test_experiment.py b/tests/test_experiment.py index bfa1de124..fe9078eab 100644 --- a/tests/test_experiment.py +++ b/tests/test_experiment.py @@ -99,3 +99,27 @@ def test_poll(fileutils): exp.start(model, block=False) exp.poll(interval=1) exp.stop(model) + + +def test_summary(fileutils): + exp_name = "test_exp_summary" + exp = Experiment(exp_name) + test_dir = fileutils.make_test_dir(exp_name) + m = exp.create_model( + "model", path=test_dir, run_settings=RunSettings("echo", "Hello") + ) + exp.start(m) + summary_str = exp.summary() + print(summary_str) + + summary_lines = summary_str.split("\n") + assert 2 == len(summary_lines) + + headers, values = [s.split() for s in summary_lines] + headers = ["Index"] + headers + + row = dict(zip(headers, values)) + assert m.name == row["Name"] + assert m.type == row["Entity-Type"] + assert 0 == int(row["RunID"]) + assert 0 == int(row["Returncode"]) From 71a3d7137fe854f5a9879e586a3fc9dd23c6b2cf Mon Sep 17 00:00:00 2001 From: Matthew Drozt Date: Thu, 9 Dec 2021 00:09:24 -0600 Subject: [PATCH 7/8] Add format to summary as an optional parameter --- smartsim/experiment.py | 7 +++++-- tests/on_wlm/test_simple_entity_launch.py | 2 +- tests/test_experiment.py | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/smartsim/experiment.py b/smartsim/experiment.py index 9b24d6d5b..f6a57db12 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -461,12 +461,15 @@ def reconnect_orchestrator(self, checkpoint): logger.error(e) raise - def summary(self): + def summary(self, format="github"): """Return a summary of the ``Experiment`` The summary will show each instance that has been launched and completed in this ``Experiment`` + :param format: the style in which the summary table is formatted, + defaults to "github" + :type format: str, optional :return: tabulate string of ``Experiment`` history :rtype: str """ @@ -495,7 +498,7 @@ def summary(self): job.history.returns[run], ] ) - return tabulate(values, headers, showindex=True, tablefmt="plain") + return tabulate(values, headers, showindex=True, tablefmt=format) def _launch_summary(self, manifest): """Experiment pre-launch summary of entities that will be launched diff --git a/tests/on_wlm/test_simple_entity_launch.py b/tests/on_wlm/test_simple_entity_launch.py index 277a4a8c6..417290437 100644 --- a/tests/on_wlm/test_simple_entity_launch.py +++ b/tests/on_wlm/test_simple_entity_launch.py @@ -77,7 +77,7 @@ def test_summary(fileutils, wlmutils): assert exp.get_status(bad)[0] == constants.STATUS_FAILED assert exp.get_status(sleep)[0] == constants.STATUS_COMPLETED - summary_str = exp.summary() + summary_str = exp.summary(format="plain") print(summary_str) rows = [s.split() for s in summary_str.split("\n")] diff --git a/tests/test_experiment.py b/tests/test_experiment.py index fe9078eab..7aed231a0 100644 --- a/tests/test_experiment.py +++ b/tests/test_experiment.py @@ -109,7 +109,7 @@ def test_summary(fileutils): "model", path=test_dir, run_settings=RunSettings("echo", "Hello") ) exp.start(m) - summary_str = exp.summary() + summary_str = exp.summary(format="plain") print(summary_str) summary_lines = summary_str.split("\n") From dd52f34ce1acf348d2fbb6d64de532f10dbf14cc Mon Sep 17 00:00:00 2001 From: Matthew Drozt Date: Thu, 9 Dec 2021 11:44:55 -0600 Subject: [PATCH 8/8] add tabulate format link to docstring --- smartsim/experiment.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/smartsim/experiment.py b/smartsim/experiment.py index f6a57db12..397dc0423 100644 --- a/smartsim/experiment.py +++ b/smartsim/experiment.py @@ -468,6 +468,8 @@ def summary(self, format="github"): launched and completed in this ``Experiment`` :param format: the style in which the summary table is formatted, + for a full list of styles see: + https://github.com/astanin/python-tabulate#table-format, defaults to "github" :type format: str, optional :return: tabulate string of ``Experiment`` history