From 7318c34d86c1cc126773cf92859ac3113259759c Mon Sep 17 00:00:00 2001 From: Eneko Martin Martinez Date: Mon, 12 Jul 2021 09:19:12 +0200 Subject: [PATCH 1/3] Close files opened with openpyxl --- .travis.yml | 1 + pysd/_version.py | 2 +- pysd/py_backend/external.py | 4 ++++ tests/unit_test_external.py | 30 ++++++++++++++++++++++++++++-- 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 57508d53..8982ab48 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,7 @@ install: - pip install cython - pip install --upgrade pip setuptools wheel - pip install -e . + - pip install psutil - pip install pytest pytest-cov - pip install coveralls # command to run tests diff --git a/pysd/_version.py b/pysd/_version.py index 29654eec..2d986fc5 100644 --- a/pysd/_version.py +++ b/pysd/_version.py @@ -1 +1 @@ -__version__ = "1.8.0" +__version__ = "1.8.1" diff --git a/pysd/py_backend/external.py b/pysd/py_backend/external.py index 0e26849b..436799bd 100644 --- a/pysd/py_backend/external.py +++ b/pysd/py_backend/external.py @@ -53,6 +53,10 @@ def clean(cls): """ Clean the dictionary of read files """ + for file in cls._Excels_opyxl.values(): + # close files open directly with openpyxls + file.close() + # files open with pandas are automatically closed cls._Excels, cls._Excels_opyxl = {}, {} diff --git a/tests/unit_test_external.py b/tests/unit_test_external.py index 0ba933ed..b6164737 100644 --- a/tests/unit_test_external.py +++ b/tests/unit_test_external.py @@ -66,6 +66,33 @@ def test_read_clean_opyxl(self): self.assertEqual(list(pysd.external.Excels._Excels_opyxl), []) + def test_close_file(self): + """ + Test for checking if excel files were closed + """ + import pysd + import psutil + + p = psutil.Process() + + # number of files already open + n_files = len(p.open_files()) + + file_name = "data/input.xlsx" + sheet_name = "Vertical" + sheet_name2 = "Horizontal" + + # reading files + pysd.external.Excels.read(file_name, sheet_name) + pysd.external.Excels.read(file_name, sheet_name2) + pysd.external.Excels.read_opyxl(file_name) + + self.assertGreater(len(p.open_files()), n_files) + + # clean + pysd.external.Excels.clean() + self.assertEqual(len(p.open_files()), n_files) + class TestExternalMethods(unittest.TestCase): """ @@ -3083,7 +3110,6 @@ def test_data_interp_hnnm_keep(self): self.assertTrue(data.data.equals(expected)) - def test_lookup_data_attr(self): """ Test for keep in series when the series is not @@ -3119,4 +3145,4 @@ def test_lookup_data_attr(self): datL.initialize() self.assertTrue(hasattr(datD, 'time_row_or_cols')) - self.assertTrue(hasattr(datL, 'x_row_or_cols')) \ No newline at end of file + self.assertTrue(hasattr(datL, 'x_row_or_cols')) From 5ae3e383b82695b2d90c622ccd189ace4732bb57 Mon Sep 17 00:00:00 2001 From: Eneko Martin Martinez Date: Tue, 13 Jul 2021 11:14:52 +0200 Subject: [PATCH 2/3] Correct bug with subscripting --- pysd/py_backend/utils.py | 5 ++--- tests/unit_test_utils.py | 14 +++++++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/pysd/py_backend/utils.py b/pysd/py_backend/utils.py index 03931c1c..0df26058 100644 --- a/pysd/py_backend/utils.py +++ b/pysd/py_backend/utils.py @@ -11,6 +11,7 @@ import regex as re import progressbar +import numpy as np import xarray as xr # used to create python safe names @@ -688,9 +689,7 @@ def rearrange(data, dims, coords): if data.shape == shape: # Allows switching dimensions names and transpositions return xr.DataArray(data=data.values, coords=coords, dims=dims) - elif len(shape) == len(data.shape) and all( - [shape[i] < data.shape[i] for i in range(len(shape))] - ): + elif np.prod(shape) < np.prod(data.shape): # Allows subscripting a subrange return data.rename( {dim: new_dim for dim, new_dim in zip(data.dims, dims)} diff --git a/tests/unit_test_utils.py b/tests/unit_test_utils.py index cab9cddd..755085ae 100644 --- a/tests/unit_test_utils.py +++ b/tests/unit_test_utils.py @@ -438,11 +438,16 @@ def test_rearrange(self): _subscript_dict = { 'd1': ['a', 'b', 'c'], 'd2': ['b', 'c'], - 'd3': ['b', 'c'] + 'd3': ['b', 'c'], + 'd4': ['e', 'f'] } xr_input_subdim = xr.DataArray([1, 4, 2], {'d1': ['a', 'b', 'c']}, ['d1']) + xr_input_subdim2 = xr.DataArray([[1, 4, 2], [3, 4, 5]], + {'d1': ['a', 'b', 'c'], + 'd4': ['e', 'f']}, + ['d4', 'd1']) xr_input_updim = xr.DataArray([1, 4], {'d2': ['b', 'c']}, ['d2']) xr_input_switch = xr.DataArray([[1, 4], [8, 5]], @@ -452,6 +457,10 @@ def test_rearrange(self): xr_out_subdim = xr.DataArray([4, 2], {'d2': ['b', 'c']}, ['d2']) + xr_out_subdim2 = xr.DataArray([[4, 2], [4, 5]], + {'d2': ['b', 'c'], + 'd4': ['e', 'f']}, + ['d4', 'd2']) xr_out_updim = xr.DataArray([[1, 1], [4, 4]], {'d2': ['b', 'c'], 'd3': ['b', 'c']}, ['d2', 'd3']) @@ -464,6 +473,9 @@ def test_rearrange(self): self.assertTrue(xr_out_subdim.equals( rearrange(xr_input_subdim, ['d2'], _subscript_dict))) + self.assertTrue(xr_out_subdim2.equals( + rearrange(xr_input_subdim2, ['d4', 'd2'], _subscript_dict))) + self.assertTrue(xr_out_updim.equals( rearrange(xr_input_updim, ['d2', 'd3'], _subscript_dict))) From d437b5b06ab373a893f065f5fcd0d09f0273daa7 Mon Sep 17 00:00:00 2001 From: Eneko Martin Martinez Date: Tue, 13 Jul 2021 13:04:37 +0200 Subject: [PATCH 3/3] Stop the model when time >= final_time --- pysd/py_backend/functions.py | 66 ++++++++++++++++-------- tests/integration_test_vensim_pathway.py | 6 ++- tests/test-models | 2 +- tests/unit_test_pysd.py | 4 +- 4 files changed, 52 insertions(+), 26 deletions(-) diff --git a/pysd/py_backend/functions.py b/pysd/py_backend/functions.py index 661a53df..fe0a9e1e 100644 --- a/pysd/py_backend/functions.py +++ b/pysd/py_backend/functions.py @@ -1243,13 +1243,17 @@ def _build_euler_timeseries(self, return_timestamps=None, final_time=None): except IndexError: # return_timestamps is an empty list # model default final time or passed argument value - t_f = self.final_time + t_f = self.components.final_time() if final_time is not None: t_f = max(final_time, t_f) ts = np.arange( - t_0, t_f+self.time_step/2, self.time_step, dtype=np.float64) + t_0, + t_f+self.components.time_step()/2, + self.components.time_step(), + dtype=np.float64 + ) # Add the returned time series into the integration array. # Best we can do for now. This does change the integration ever @@ -1278,11 +1282,10 @@ def _format_return_timestamps(self, return_timestamps=None): # Vensim's standard is to expect that the data set includes # the `final time`, so we have to add an extra period to # make sure we get that value in what numpy's `arange` gives us. - return np.arange( self.time(), - self.final_time + self.saveper/2, - self.saveper, dtype=float + self.components.final_time() + self.components.saveper()/2, + self.components.saveper(), dtype=float ) try: @@ -1377,23 +1380,35 @@ def run(self, params=None, return_columns=None, return_timestamps=None, self.progress = progress + # TODO move control variables to a class + if params is None: + params = {} + if final_time: + params['final_time'] = final_time + elif return_timestamps is not None: + params['final_time'] =\ + self._format_return_timestamps(return_timestamps)[-1] + if time_step: + params['time_step'] = time_step + if saveper: + params['saveper'] = saveper + # END TODO + if params: self.set_components(params) self.set_initial_condition(initial_condition) + # TODO move control variables to a class # save control variables - self.initial_time = self.time() - self.final_time = final_time or self.components.final_time() - self.time_step = time_step or self.components.time_step() - self.saveper = saveper or max(self.time_step, - self.components.saveper()) - # need to take bigger saveper if time_step is > saveper + replace = { + 'initial_time': self.time() + } + # END TODO return_timestamps = self._format_return_timestamps(return_timestamps) t_series = self._build_euler_timeseries(return_timestamps, final_time) - self.final_time = t_series[-1] if return_columns is None or isinstance(return_columns, str): return_columns = self._default_return_columns(return_columns) @@ -1410,10 +1425,9 @@ def run(self, params=None, return_columns=None, return_timestamps=None, res = self._integrate(t_series, capture_elements['step'], return_timestamps) - self._add_run_elements(res, capture_elements['run']) + self._add_run_elements(res, capture_elements['run'], replace=replace) return_df = utils.make_flat_df(res, return_addresses, flatten_output) - return_df.index = return_timestamps return return_df @@ -1562,8 +1576,7 @@ def _integrate(self, time_steps, capture_elements, return_timestamps): outputs: list of dictionaries """ - outputs = pd.DataFrame(index=return_timestamps, - columns=capture_elements) + outputs = pd.DataFrame(columns=capture_elements) if self.progress: # initialize progress bar @@ -1580,6 +1593,10 @@ def _integrate(self, time_steps, capture_elements, return_timestamps): self.time.update(t2) # this will clear the stepwise caches self.components.cache.reset(t2) progressbar.update() + # TODO move control variables to a class and automatically stop + # when updating time + if self.time() >= self.components.final_time(): + break # need to add one more time step, because we run only the state # updates in the previous loop and thus may be one short. @@ -1591,7 +1608,7 @@ def _integrate(self, time_steps, capture_elements, return_timestamps): return outputs - def _add_run_elements(self, df, capture_elements): + def _add_run_elements(self, df, capture_elements, replace={}): """ Adds constant elements to a dataframe. @@ -1603,6 +1620,10 @@ def _add_run_elements(self, df, capture_elements): capture_elements: list List of constant elements + replace: dict + Ouputs values to replace. + TODO: move control variables to a class and avoid this. + Returns ------- None @@ -1612,16 +1633,17 @@ def _add_run_elements(self, df, capture_elements): for element in capture_elements: df[element] = [getattr(self.components, element)()] * nt + # TODO: move control variables to a class and avoid this. # update initial time values in df (necessary if initial_conditions) - for it in ['initial_time', 'final_time', 'saveper', 'time_step']: + for it, value in replace.items(): if it in df: - df[it] = getattr(self, it) + df[it] = value elif it.upper() in df: - df[it.upper()] = getattr(self, it) + df[it.upper()] = value elif it.replace('_', ' ') in df: - df[it.replace('_', ' ')] = getattr(self, it) + df[it.replace('_', ' ')] = value elif it.replace('_', ' ').upper() in df: - df[it.replace('_', ' ').upper()] = getattr(self, it) + df[it.replace('_', ' ').upper()] = value def ramp(time, slope, start, finish=0): diff --git a/tests/integration_test_vensim_pathway.py b/tests/integration_test_vensim_pathway.py index 7949247f..c34f867a 100644 --- a/tests/integration_test_vensim_pathway.py +++ b/tests/integration_test_vensim_pathway.py @@ -50,7 +50,6 @@ def test_delay_fixed(self): output, canon = runner('test-models/tests/delay_fixed/test_delay_fixed.mdl') assert_frames_close(output, canon, rtol=rtol) - @unittest.skip('to be fixed #225') def test_delay_numeric_error(self): # issue https://github.com/JamesPHoughton/pysd/issues/225 output, canon = runner('test-models/tests/delay_numeric_error/test_delay_numeric_error.mdl') @@ -72,6 +71,11 @@ def test_delays(self): output, canon = runner('test-models/tests/delays/test_delays.mdl') assert_frames_close(output, canon, rtol=rtol) + def test_dynamic_final_time(self): + # issue https://github.com/JamesPHoughton/pysd/issues/278 + output, canon = runner('test-models/tests/dynamic_final_time/test_dynamic_final_time.mdl') + assert_frames_close(output, canon, rtol=rtol) + def test_euler_step_vs_saveper(self): output, canon = runner('test-models/tests/euler_step_vs_saveper/test_euler_step_vs_saveper.mdl') assert_frames_close(output, canon, rtol=rtol) diff --git a/tests/test-models b/tests/test-models index eb88705c..04907a20 160000 --- a/tests/test-models +++ b/tests/test-models @@ -1 +1 @@ -Subproject commit eb88705c0d68444dc54a1542cf47d233bd44f9a7 +Subproject commit 04907a209d09fceadc35fe900f067d19aa7d3c9d diff --git a/tests/unit_test_pysd.py b/tests/unit_test_pysd.py index c3120e80..a47d07af 100644 --- a/tests/unit_test_pysd.py +++ b/tests/unit_test_pysd.py @@ -1590,8 +1590,8 @@ def test__build_euler_timeseries(self): model = pysd.read_vensim(test_model) model.components.initial_time = lambda: 3 - model.final_time = 50 - model.time_step = 1 + model.components.final_time = lambda: 50 + model.components.time_step = lambda: 1 model.initialize() actual = list(model._build_euler_timeseries(return_timestamps=[10]))