From 7d4c3741437f5d83dd96838e2f414b328cdf8360 Mon Sep 17 00:00:00 2001 From: Sarah Date: Thu, 5 Dec 2019 15:54:13 +0000 Subject: [PATCH 01/47] rationalising unit tests --- tests/test_model_units.py | 44 ---------- tests/test_unit_conversion.py | 152 +++++++++++++++++++--------------- 2 files changed, 84 insertions(+), 112 deletions(-) delete mode 100644 tests/test_model_units.py diff --git a/tests/test_model_units.py b/tests/test_model_units.py deleted file mode 100644 index 79f184c4..00000000 --- a/tests/test_model_units.py +++ /dev/null @@ -1,44 +0,0 @@ -import pytest - -from cellmlmanip import units - - -# TODO some tests here are repeats and may not be necessary -class TestModelUnits: - def test_symbols(self, simple_units_model): - """ Tests the Model.get_symbol_by_cmeta_id function.""" - symbol = simple_units_model.get_symbol_by_cmeta_id("a") - assert symbol.is_Symbol - symbol = simple_units_model.get_symbol_by_cmeta_id("b") - assert symbol.is_Symbol - - def test_units(self, simple_units_model): - """ Tests units read and calculated from a model. """ - symbol_a = simple_units_model.get_symbol_by_cmeta_id("a") - equation = simple_units_model.get_equations_for([symbol_a], strip_units=False) - assert simple_units_model.units.summarise_units(equation[0].lhs) == 'ms' - assert simple_units_model.units.summarise_units(equation[0].rhs) == 'ms' - - symbol_b = simple_units_model.get_symbol_by_cmeta_id("b") - equation = simple_units_model.get_equations_for([symbol_b]) - assert simple_units_model.units.summarise_units(equation[1].lhs) == 'per_ms' - assert simple_units_model.units.summarise_units(equation[1].rhs) == '1 / ms' - assert simple_units_model.units.is_unit_equal(simple_units_model.units.summarise_units(equation[1].lhs), - simple_units_model.units.summarise_units(equation[1].rhs)) - - def test_bad_units(self, bad_units_model): - """ Tests units read and calculated from an inconsistent model. """ - symbol_a = bad_units_model.get_symbol_by_cmeta_id("a") - symbol_b = bad_units_model.get_symbol_by_cmeta_id("b") - equation = bad_units_model.get_equations_for([symbol_b], strip_units=False) - assert len(equation) == 2 - assert equation[0].lhs == symbol_a - assert bad_units_model.units.summarise_units(equation[0].lhs) == 'ms' - with pytest.raises(units.UnitError): - # cellml file states a (ms) = 1 (ms) + 1 (second) - bad_units_model.units.summarise_units(equation[0].rhs) - - assert equation[1].lhs == symbol_b - with pytest.raises(units.UnitError): - # cellml file states b (per_ms) = power(a (ms), 1 (second)) - bad_units_model.units.summarise_units(equation[1].rhs) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 5a5af30e..23576672 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -1,80 +1,96 @@ import pytest +from cellmlmanip import units from . import shared -def test_add_preferred_custom_unit_name(simple_ode_model): - """ Tests Units.add_preferred_custom_unit_name() function. """ - time_var = simple_ode_model.get_symbol_by_ontology_term(shared.OXMETA, "time") - assert str(simple_ode_model.units.summarise_units(time_var)) == "ms" - simple_ode_model.units.add_preferred_custom_unit_name('millisecond', [{'prefix': 'milli', 'units': 'second'}]) - assert str(simple_ode_model.units.summarise_units(time_var)) == "millisecond" - # add_custom_unit does not allow adding already existing units but add_preferred_custom_unit_name does since we - # cannot know in advance if a model will already have the unit named this way. To test this we add the same unit - # again - simple_ode_model.units.add_preferred_custom_unit_name('millisecond', [{'prefix': 'milli', 'units': 'second'}]) - assert str(simple_ode_model.units.summarise_units(time_var)) == "millisecond" +class TestUnitConversion: + def test_add_preferred_custom_unit_name(self, simple_ode_model): + """ Tests Units.add_preferred_custom_unit_name() function. """ + time_var = simple_ode_model.get_symbol_by_ontology_term(shared.OXMETA, "time") + assert str(simple_ode_model.units.summarise_units(time_var)) == "ms" + simple_ode_model.units.add_preferred_custom_unit_name('millisecond', [{'prefix': 'milli', 'units': 'second'}]) + assert str(simple_ode_model.units.summarise_units(time_var)) == "millisecond" + # add_custom_unit does not allow adding already existing units but add_preferred_custom_unit_name does since we + # cannot know in advance if a model will already have the unit named this way. To test this we add the same unit + # again + simple_ode_model.units.add_preferred_custom_unit_name('millisecond', [{'prefix': 'milli', 'units': 'second'}]) + assert str(simple_ode_model.units.summarise_units(time_var)) == "millisecond" + def test_conversion_factor_original(self, simple_units_model): + """ Tests Units.get_conversion_factor() function. """ + symbol_b1 = simple_units_model.get_symbol_by_cmeta_id("b_1") + equation = simple_units_model.get_equations_for([symbol_b1]) + factor = simple_units_model.units.get_conversion_factor( + quantity=1 * simple_units_model.units.summarise_units(equation[0].lhs), + to_unit=simple_units_model.units.ureg('us').units) + assert factor == 1000 -def test_conversion_factor_original(simple_units_model): - """ Tests Units.get_conversion_factor() function. """ - symbol_b1 = simple_units_model.get_symbol_by_cmeta_id("b_1") - equation = simple_units_model.get_equations_for([symbol_b1]) - factor = simple_units_model.units.get_conversion_factor( - quantity=1 * simple_units_model.units.summarise_units(equation[0].lhs), - to_unit=simple_units_model.units.ureg('us').units) - assert factor == 1000 + def test_conversion_factor_bad_types(self, simple_units_model): + """ Tests Units.get_conversion_factor() function for + cases when arguments are missing or incorrectly typed.""" + symbol_b1 = simple_units_model.get_symbol_by_cmeta_id("b_1") + equation = simple_units_model.get_equations_for([symbol_b1]) + expression = equation[0].lhs + to_unit = simple_units_model.units.ureg('us').units + from_unit = simple_units_model.units.summarise_units(expression) + quantity = 1 * from_unit + # no source unit + with pytest.raises(AssertionError, match='^No unit given as source.*'): + simple_units_model.units.get_conversion_factor(to_unit=to_unit) + with pytest.raises(AssertionError, match='^No unit given as source.*'): + simple_units_model.units.get_conversion_factor(to_unit) + # no target unit + with pytest.raises(TypeError): + simple_units_model.units.get_conversion_factor(from_unit=from_unit) + # multiple sources + with pytest.raises(AssertionError, match='^Multiple target.*'): + simple_units_model.units.get_conversion_factor(to_unit, from_unit=from_unit, quantity=quantity) + # incorrect types + with pytest.raises(AssertionError, match='^from_unit must be of type pint:Unit$'): + simple_units_model.units.get_conversion_factor(to_unit, from_unit=quantity) + with pytest.raises(AssertionError, match='^quantity must be of type pint:Quantity$'): + simple_units_model.units.get_conversion_factor(to_unit, quantity=from_unit) + with pytest.raises(AssertionError, match='^expression must be of type Sympy expression$'): + simple_units_model.units.get_conversion_factor(to_unit, expression=quantity) -def test_conversion_factor_bad_types(simple_units_model): - """ Tests Units.get_conversion_factor() function for - cases when arguments are missing or incorrectly typed.""" - symbol_b1 = simple_units_model.get_symbol_by_cmeta_id("b_1") - equation = simple_units_model.get_equations_for([symbol_b1]) - expression = equation[0].lhs - to_unit = simple_units_model.units.ureg('us').units - from_unit = simple_units_model.units.summarise_units(expression) - quantity = 1 * from_unit - # no source unit - with pytest.raises(AssertionError, match='^No unit given as source.*'): - simple_units_model.units.get_conversion_factor(to_unit=to_unit) - with pytest.raises(AssertionError, match='^No unit given as source.*'): - simple_units_model.units.get_conversion_factor(to_unit) + # unit to unit + assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1000 + # quantity to unit + assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, quantity=quantity) == 1000 + # expression to unit + assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1000 - # no target unit - with pytest.raises(TypeError): - simple_units_model.units.get_conversion_factor(from_unit=from_unit) - # multiple sources - with pytest.raises(AssertionError, match='^Multiple target.*'): - simple_units_model.units.get_conversion_factor(to_unit, from_unit=from_unit, quantity=quantity) - # incorrect types - with pytest.raises(AssertionError, match='^from_unit must be of type pint:Unit$'): - simple_units_model.units.get_conversion_factor(to_unit, from_unit=quantity) - with pytest.raises(AssertionError, match='^quantity must be of type pint:Quantity$'): - simple_units_model.units.get_conversion_factor(to_unit, quantity=from_unit) - with pytest.raises(AssertionError, match='^expression must be of type Sympy expression$'): - simple_units_model.units.get_conversion_factor(to_unit, expression=quantity) + def test_conversion_factor_same_units(self, simple_units_model): + """ Tests Units.get_conversion_factor() function when units are same + and conversion factor should be '1'. """ + symbol_b = simple_units_model.get_symbol_by_cmeta_id("b") + equation = simple_units_model.get_equations_for([symbol_b]) + expression = equation[1].rhs + to_unit = simple_units_model.units.ureg('per_ms').units + from_unit = simple_units_model.units.summarise_units(expression) + quantity = 1 * from_unit + # quantity to unit + assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, quantity=quantity) == 1 + # unit to unit + assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1 + # expression to unit + assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1 - # unit to unit - assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1000 - # quantity to unit - assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, quantity=quantity) == 1000 - # expression to unit - assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1000 + def test_bad_units(self, bad_units_model): + """ Tests units read and calculated from an inconsistent model. """ + symbol_a = bad_units_model.get_symbol_by_cmeta_id("a") + symbol_b = bad_units_model.get_symbol_by_cmeta_id("b") + equation = bad_units_model.get_equations_for([symbol_b], strip_units=False) + assert len(equation) == 2 + assert equation[0].lhs == symbol_a + assert bad_units_model.units.summarise_units(equation[0].lhs) == 'ms' + with pytest.raises(units.UnitError): + # cellml file states a (ms) = 1 (ms) + 1 (second) + bad_units_model.units.summarise_units(equation[0].rhs) - -def test_conversion_factor_same_units(simple_units_model): - """ Tests Units.get_conversion_factor() function when units are same - and conversion factor should be '1'. """ - symbol_b = simple_units_model.get_symbol_by_cmeta_id("b") - equation = simple_units_model.get_equations_for([symbol_b]) - expression = equation[1].rhs - to_unit = simple_units_model.units.ureg('per_ms').units - from_unit = simple_units_model.units.summarise_units(expression) - quantity = 1 * from_unit - # quantity to unit - assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, quantity=quantity) == 1 - # unit to unit - assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1 - # expression to unit - assert simple_units_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1 + assert equation[1].lhs == symbol_b + with pytest.raises(units.UnitError): + # cellml file states b (per_ms) = power(a (ms), 1 (second)) + bad_units_model.units.summarise_units(equation[1].rhs) From 0bbec40966a5d5cc1f7c076a0dc82eb0ed66f460 Mon Sep 17 00:00:00 2001 From: Sarah Date: Fri, 6 Dec 2019 14:33:03 +0000 Subject: [PATCH 02/47] beginning to work through converting units within a model --- cellmlmanip/model.py | 66 +++++++++++++++++++++++++++++++++++ tests/test_unit_conversion.py | 33 ++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 5e014f6c..9136e99c 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -607,6 +607,72 @@ def variables(self): """ Returns an iterator over this model's variable symbols. """ return self._name_to_symbol.values() + def add_input(self, name, units): + """ Adds an additional input for the variable name to the model with new units + and apply any necessary conversions/add additional equations. + + For example: + Original model + x = 1 [pA] + in [pA] + oxmeta: current + y = 1 / x + in [1/pA] + + add_input('x', 'nA') creates model + input_current = 1e-3 [nA] + in [nA] + oxmeta: current + x = input_current * 1e3 [pA/nA] + in [pA] + y = 1 / x + in [1/pA] + + :param name: name of variable whose units are to be changed + :param units: unit to convert to + + """ + # TODO what if units not in model/ how to tell user + if not self.units._is_unit_defined(units): + return + # TODO should I throw the exception or user seamlessly gets a non changed model + original_variable = None + try: + original_variable = self.get_symbol_by_name(name) + except KeyError: + # name does not exist in model + return + original_units = original_variable.units + original_cmeta_id = original_variable.cmeta_id + original_initial_value = self.get_initial_value(original_variable) + + # no conversion necessary + if original_units == units: + return + + # conversion_factor for old units to new units + to_quantity = self.units.ureg(units) +# from_units = self.units.ureg(original_units) + cf = self.units.get_conversion_factor(original_units, quantity=to_quantity) + + # 1. get unique name for new variable + new_name = name + 'converted' + while new_name in self.variables(): + new_name = new_name + '_a' + + # 2. if original has initial_value calculate new initial value + new_value = None + if original_initial_value: + new_value = original_initial_value / cf + + # 3. copy cmeta_id from original and remove from original + original_variable.cmeta_id = '' + self.add_variable(name=new_name, + units=units, + initial_value=new_value, + cmeta_id=original_cmeta_id) + self._invalidate_cache() + class NumberDummy(sympy.Dummy): """ diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 23576672..03559661 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -94,3 +94,36 @@ def test_bad_units(self, bad_units_model): with pytest.raises(units.UnitError): # cellml file states b (per_ms) = power(a (ms), 1 (second)) bad_units_model.units.summarise_units(equation[1].rhs) + + def test_add_input(self, basic_model): + """ Tests the Model.add_input function that changes units. """ + # original state + def test_original_state(basic_model): + assert len(basic_model.variables()) == 3 + symbol_a = basic_model.get_symbol_by_cmeta_id('sv11') + symbol_t = basic_model.get_symbol_by_cmeta_id('time') + assert basic_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert len(basic_model.equations) == 1 + return True + + assert test_original_state(basic_model) + # test no change in units + basic_model.add_input('env_ode$sv1', 'mV') + assert test_original_state(basic_model) + + # non-existent name + basic_model.add_input('env$sv1', 'V') + + # non-existent unit + basic_model.add_input('env_ode$sv1', 'V') + + basic_model.add_input('env_ode$sv1', 'volt') + assert len(basic_model.variables()) == 4 + # this needs to work without graph + # symbol_a = basic_model.get_symbol_by_cmeta_id('sv11') + symbol_t = basic_model.get_symbol_by_cmeta_id('time') + # assert basic_model.get_initial_value(symbol_a) == 0.002 + # assert symbol_a.units == 'volt' + assert symbol_t.units == 'ms' From b6f3699f011d189eec4e66ecc7904e226104a873 Mon Sep 17 00:00:00 2001 From: Sarah Date: Tue, 10 Dec 2019 12:58:51 +0000 Subject: [PATCH 03/47] convert initial value for new variable with converted units --- cellmlmanip/model.py | 14 ++++++-------- tests/test_unit_conversion.py | 27 +++++++++++++++++---------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 62af5042..54dbce2e 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -632,12 +632,12 @@ def add_input(self, name, units): in [1/pA] :param name: name of variable whose units are to be changed - :param units: unit to convert to + :param units: A `pint` units representation """ - # TODO what if units not in model/ how to tell user - if not self.units._is_unit_defined(units): - return + # check units is of type Unit + assert isinstance(units, self.units.ureg.Unit) + # TODO should I throw the exception or user seamlessly gets a non changed model original_variable = None try: @@ -654,9 +654,7 @@ def add_input(self, name, units): return # conversion_factor for old units to new units - to_quantity = self.units.ureg(units) -# from_units = self.units.ureg(original_units) - cf = self.units.get_conversion_factor(original_units, quantity=to_quantity) + cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) # 1. get unique name for new variable new_name = name + 'converted' @@ -666,7 +664,7 @@ def add_input(self, name, units): # 2. if original has initial_value calculate new initial value new_value = None if original_initial_value: - new_value = original_initial_value / cf + new_value = original_initial_value * cf # 3. copy cmeta_id from original and remove from original original_variable.cmeta_id = '' diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 03559661..30503f68 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -1,6 +1,7 @@ import pytest from cellmlmanip import units +from cellmlmanip.units import UnitStore from . import shared @@ -107,23 +108,29 @@ def test_original_state(basic_model): assert symbol_t.units == 'ms' assert len(basic_model.equations) == 1 return True + mV_unit = basic_model.get_units('mV') + volt_unit = basic_model.get_units('volt') assert test_original_state(basic_model) # test no change in units - basic_model.add_input('env_ode$sv1', 'mV') + basic_model.add_input('env_ode$sv1', mV_unit) assert test_original_state(basic_model) - # non-existent name - basic_model.add_input('env$sv1', 'V') - # non-existent unit - basic_model.add_input('env_ode$sv1', 'V') + # TODO what if unit not in model + unit_attributes = [{'prefix': -3, 'units': 'metre'}, + {'prefix': 'milli', 'exponent': -1, 'units': 'second'}, + {'multiplier': 2, 'units': 'kilograms'} + ] + qs = UnitStore(model=None) + qs.add_custom_unit('kg_ms_m', unit_attributes) + # unit_not_in_model = qs.get_units('kg_ms_m') + # basic_model.add_input('env$sv1', unit_not_in_model) - basic_model.add_input('env_ode$sv1', 'volt') + basic_model.add_input('env_ode$sv1', volt_unit) assert len(basic_model.variables()) == 4 - # this needs to work without graph - # symbol_a = basic_model.get_symbol_by_cmeta_id('sv11') + symbol_a = basic_model.get_symbol_by_cmeta_id('sv11') symbol_t = basic_model.get_symbol_by_cmeta_id('time') - # assert basic_model.get_initial_value(symbol_a) == 0.002 - # assert symbol_a.units == 'volt' + assert basic_model.get_initial_value(symbol_a) == 0.002 + assert symbol_a.units == 'volt' assert symbol_t.units == 'ms' From f2c966c55c94fe34c854d7e8af19333127746bab Mon Sep 17 00:00:00 2001 From: Sarah Date: Wed, 11 Dec 2019 15:48:28 +0000 Subject: [PATCH 04/47] tests for adding an input that it is a state variable --- cellmlmanip/model.py | 47 +++++++++++++++++++++++++++++------ tests/test_unit_conversion.py | 26 +++++++++++-------- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 54dbce2e..a0124111 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -7,8 +7,7 @@ import sympy from cellmlmanip.rdf import create_rdf_node -from cellmlmanip.units import UnitStore - +from cellmlmanip.units import UnitStore, UnitCalculator logger = logging.getLogger(__name__) @@ -639,6 +638,7 @@ def add_input(self, name, units): assert isinstance(units, self.units.ureg.Unit) # TODO should I throw the exception or user seamlessly gets a non changed model + # if unit not in model original_variable = None try: original_variable = self.get_symbol_by_name(name) @@ -653,11 +653,13 @@ def add_input(self, name, units): if original_units == units: return + state_symbols = self.get_state_symbols() + unit_calculator = UnitCalculator(self.units.ureg) # conversion_factor for old units to new units cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) # 1. get unique name for new variable - new_name = name + 'converted' + new_name = name + '_converted' while new_name in self.variables(): new_name = new_name + '_a' @@ -668,10 +670,41 @@ def add_input(self, name, units): # 3. copy cmeta_id from original and remove from original original_variable.cmeta_id = '' - self.add_variable(name=new_name, - units=units, - initial_value=new_value, - cmeta_id=original_cmeta_id) + new_variable = self.add_variable(name=new_name, + units=units, + initial_value=new_value, + cmeta_id=original_cmeta_id) + + # 4. add an equation for original variable + expression = sympy.Eq(original_variable, new_variable / cf) + self.add_equation(expression) + + # if state variable + if original_variable in state_symbols: + # find the derivative equation for this variable + eqn = None + for equation in self.equations: + if equation.args[0].is_Derivative: + if equation.args[0].args[0] == original_variable: + eqn = equation + # get deriv symbol + wrt_variable = eqn.args[0].args[1] + # create new variable for rhs + deriv_name = name + '_orig_deriv' + while deriv_name in self.variables(): + deriv_name = deriv_name + '_a' + deriv_units = unit_calculator.traverse(eqn.args[0]) + new_deriv_variable = self.add_variable(name=deriv_name, + units=deriv_units.units) + # add new equation for new deriv variable and remove existing one + expression = sympy.Eq(new_deriv_variable, eqn.args[1]) + self.add_equation(expression) + self.remove_equation(eqn) + # TODO replace any instance of deriv on rhs of any eqn with new deriv var + # new derivative + expression = sympy.Eq(sympy.Derivative(new_variable, wrt_variable), new_deriv_variable * cf) + self.add_equation(expression) + self._invalidate_cache() diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 30503f68..06ca7c07 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -1,7 +1,6 @@ import pytest from cellmlmanip import units -from cellmlmanip.units import UnitStore from . import shared @@ -107,7 +106,9 @@ def test_original_state(basic_model): assert symbol_a.units == 'mV' assert symbol_t.units == 'ms' assert len(basic_model.equations) == 1 + assert str(basic_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' return True + mV_unit = basic_model.get_units('mV') volt_unit = basic_model.get_units('volt') @@ -118,19 +119,22 @@ def test_original_state(basic_model): # non-existent unit # TODO what if unit not in model - unit_attributes = [{'prefix': -3, 'units': 'metre'}, - {'prefix': 'milli', 'exponent': -1, 'units': 'second'}, - {'multiplier': 2, 'units': 'kilograms'} - ] - qs = UnitStore(model=None) - qs.add_custom_unit('kg_ms_m', unit_attributes) - # unit_not_in_model = qs.get_units('kg_ms_m') - # basic_model.add_input('env$sv1', unit_not_in_model) + # change mV to V basic_model.add_input('env_ode$sv1', volt_unit) - assert len(basic_model.variables()) == 4 + assert len(basic_model.variables()) == 5 symbol_a = basic_model.get_symbol_by_cmeta_id('sv11') - symbol_t = basic_model.get_symbol_by_cmeta_id('time') assert basic_model.get_initial_value(symbol_a) == 0.002 assert symbol_a.units == 'volt' + assert symbol_a.name == 'env_ode$sv1_converted' + symbol_t = basic_model.get_symbol_by_cmeta_id('time') assert symbol_t.units == 'ms' + symbol_orig = basic_model.get_symbol_by_name('env_ode$sv1') + assert symbol_orig.units == 'mV' + symbol_derv = basic_model.get_symbol_by_name('env_ode$sv1_orig_deriv') + assert symbol_derv.units == 'mV / ms' + assert len(basic_model.equations) == 3 + assert str(basic_model.equations[0]) == 'Eq(_env_ode$sv1, 1000.0*_env_ode$sv1_converted)' + assert str(basic_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' + assert str(basic_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1_converted, _environment$time), ' \ + '0.001*_env_ode$sv1_orig_deriv)' From 8c0582c620d715c823de1c6cb4a9d46a23f13bcc Mon Sep 17 00:00:00 2001 From: Sarah Date: Wed, 11 Dec 2019 15:54:15 +0000 Subject: [PATCH 05/47] isort --- cellmlmanip/model.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index a0124111..9e40bffe 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -7,7 +7,8 @@ import sympy from cellmlmanip.rdf import create_rdf_node -from cellmlmanip.units import UnitStore, UnitCalculator +from cellmlmanip.units import UnitCalculator +from cellmlmanip.units import UnitStore logger = logging.getLogger(__name__) From 28adf49dd7f9d9f7c10a67c4d80ce04a60cdb242 Mon Sep 17 00:00:00 2001 From: Sarah Date: Wed, 11 Dec 2019 16:05:38 +0000 Subject: [PATCH 06/47] isort still wasnt happy --- cellmlmanip/model.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 9e40bffe..1ce2a10e 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -7,8 +7,7 @@ import sympy from cellmlmanip.rdf import create_rdf_node -from cellmlmanip.units import UnitCalculator -from cellmlmanip.units import UnitStore +from cellmlmanip.units import UnitCalculator, UnitStore logger = logging.getLogger(__name__) From ef3bd6527bb47d0c3d3f67fa2fafd200d1255b67 Mon Sep 17 00:00:00 2001 From: Sarah Date: Fri, 13 Dec 2019 12:05:21 +0000 Subject: [PATCH 07/47] changing units for the free variable --- cellmlmanip/model.py | 40 +++++++++++-- tests/test_unit_conversion.py | 104 ++++++++++++++++++++++++++-------- 2 files changed, 114 insertions(+), 30 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 9ec3a05d..e4051d4b 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -648,12 +648,12 @@ def add_input(self, name, units): # TODO should I throw the exception or user seamlessly gets a non changed model # if unit not in model - original_variable = None - try: - original_variable = self.get_symbol_by_name(name) - except KeyError: - # name does not exist in model - return + + # check variable is in model + original_variable = self.get_symbol_by_name(name) + if not original_variable: + raise KeyError('No variable with name "%s" found.' % name) + original_units = original_variable.units original_cmeta_id = original_variable.cmeta_id original_initial_value = self.get_initial_value(original_variable) @@ -663,6 +663,7 @@ def add_input(self, name, units): return state_symbols = self.get_state_symbols() + free_symbols = self.get_free_variable_symbol() unit_calculator = UnitCalculator(self.units.ureg) # conversion_factor for old units to new units cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) @@ -714,6 +715,33 @@ def add_input(self, name, units): expression = sympy.Eq(sympy.Derivative(new_variable, wrt_variable), new_deriv_variable * cf) self.add_equation(expression) + # if free variable + if original_variable == free_symbols: + # find the derivative equation for this variable + # todo free variable might appear in more than one deriv + eqn = None + for equation in self.equations: + if equation.args[0].is_Derivative: + if equation.args[0].args[1].args[0] == original_variable: + eqn = equation + # get deriv symbol + derivative_variable = eqn.args[0].args[0] + # create new variable for rhs + deriv_name = derivative_variable.name + '_orig_deriv' + while deriv_name in self.variables(): + deriv_name = deriv_name + '_a' + deriv_units = unit_calculator.traverse(eqn.args[0]) + new_deriv_variable = self.add_variable(name=deriv_name, + units=deriv_units.units) + # add new equation for new deriv variable and remove existing one + expression = sympy.Eq(new_deriv_variable, eqn.args[1]) + self.add_equation(expression) + self.remove_equation(eqn) + # TODO replace any instance of deriv on rhs of any eqn with new deriv var + # new derivative + expression = sympy.Eq(sympy.Derivative(derivative_variable, new_variable), new_deriv_variable / cf) + self.add_equation(expression) + self._invalidate_cache() diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 06ca7c07..d31773fc 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -5,6 +5,14 @@ class TestUnitConversion: + ############################################################### + # fixtures + + @pytest.fixture + def local_model(scope='function'): + """ Fixture to load a local copy of the basic_ode model that may get modified. """ + return shared.load_model('basic_ode') + def test_add_preferred_custom_unit_name(self, simple_ode_model): """ Tests Units.add_preferred_custom_unit_name() function. """ time_var = simple_ode_model.get_symbol_by_ontology_term(shared.OXMETA, "time") @@ -95,46 +103,94 @@ def test_bad_units(self, bad_units_model): # cellml file states b (per_ms) = power(a (ms), 1 (second)) bad_units_model.units.summarise_units(equation[1].rhs) - def test_add_input(self, basic_model): + def test_add_input_invalid_args(self, local_model): + mV_unit = local_model.get_units('mV') + # name does not exist in model + with pytest.raises(KeyError): + local_model.add_input('nonsense_name', mV_unit) + + def test_add_input_state_variable(self, local_model): """ Tests the Model.add_input function that changes units. """ # original state - def test_original_state(basic_model): - assert len(basic_model.variables()) == 3 - symbol_a = basic_model.get_symbol_by_cmeta_id('sv11') - symbol_t = basic_model.get_symbol_by_cmeta_id('time') - assert basic_model.get_initial_value(symbol_a) == 2.0 + def test_original_state(local_model): + assert len(local_model.variables()) == 3 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert local_model.get_initial_value(symbol_a) == 2.0 assert symbol_a.units == 'mV' assert symbol_t.units == 'ms' - assert len(basic_model.equations) == 1 - assert str(basic_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert len(local_model.equations) == 1 + assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' return True - mV_unit = basic_model.get_units('mV') - volt_unit = basic_model.get_units('volt') + mV_unit = local_model.get_units('mV') + volt_unit = local_model.get_units('volt') - assert test_original_state(basic_model) + assert test_original_state(local_model) # test no change in units - basic_model.add_input('env_ode$sv1', mV_unit) - assert test_original_state(basic_model) + local_model.add_input('env_ode$sv1', mV_unit) + assert test_original_state(local_model) # non-existent unit # TODO what if unit not in model # change mV to V - basic_model.add_input('env_ode$sv1', volt_unit) - assert len(basic_model.variables()) == 5 - symbol_a = basic_model.get_symbol_by_cmeta_id('sv11') - assert basic_model.get_initial_value(symbol_a) == 0.002 + local_model.add_input('env_ode$sv1', volt_unit) + assert len(local_model.variables()) == 5 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + assert local_model.get_initial_value(symbol_a) == 0.002 assert symbol_a.units == 'volt' assert symbol_a.name == 'env_ode$sv1_converted' - symbol_t = basic_model.get_symbol_by_cmeta_id('time') + symbol_t = local_model.get_symbol_by_cmeta_id('time') assert symbol_t.units == 'ms' - symbol_orig = basic_model.get_symbol_by_name('env_ode$sv1') + symbol_orig = local_model.get_symbol_by_name('env_ode$sv1') assert symbol_orig.units == 'mV' - symbol_derv = basic_model.get_symbol_by_name('env_ode$sv1_orig_deriv') + symbol_derv = local_model.get_symbol_by_name('env_ode$sv1_orig_deriv') assert symbol_derv.units == 'mV / ms' - assert len(basic_model.equations) == 3 - assert str(basic_model.equations[0]) == 'Eq(_env_ode$sv1, 1000.0*_env_ode$sv1_converted)' - assert str(basic_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' - assert str(basic_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1_converted, _environment$time), ' \ + assert len(local_model.equations) == 3 + assert str(local_model.equations[0]) == 'Eq(_env_ode$sv1, 1000.0*_env_ode$sv1_converted)' + assert str(local_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' + assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1_converted, _environment$time), ' \ '0.001*_env_ode$sv1_orig_deriv)' + + def test_add_input_free_variable(self, local_model): + """ Tests the Model.add_input function that changes units. """ + # original state + def test_original_state(local_model): + assert len(local_model.variables()) == 3 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert local_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert len(local_model.equations) == 1 + assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + return True + + ms_unit = local_model.get_units('ms') + second_unit = local_model.get_units('second') + + assert test_original_state(local_model) + # test no change in units + local_model.add_input('env_ode$time', ms_unit) + assert test_original_state(local_model) + + # change ms to s + local_model.add_input('environment$time', second_unit) + assert len(local_model.variables()) == 5 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + assert local_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_a.name == 'env_ode$sv1' + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert symbol_t.units == 'second' + assert symbol_t.name == 'environment$time_converted' + symbol_orig = local_model.get_symbol_by_name('env_ode$sv1') + assert symbol_orig.units == 'mV' + symbol_derv = local_model.get_symbol_by_name('env_ode$sv1_orig_deriv') + assert symbol_derv.units == 'mV / ms' + assert len(local_model.equations) == 3 + assert str(local_model.equations[0]) == 'Eq(_environment$time, 1000.0*_environment$time_converted)' + assert str(local_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' + assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1, _environment$time_converted), ' \ + '1000.0*_env_ode$sv1_orig_deriv)' From 150b9b082901693464c3b07e61f7acd38651fd0b Mon Sep 17 00:00:00 2001 From: Sarah Date: Fri, 13 Dec 2019 12:17:23 +0000 Subject: [PATCH 08/47] trying to make isort happy --- cellmlmanip/model.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index e4051d4b..440fe009 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -7,7 +7,7 @@ import sympy from cellmlmanip.rdf import create_rdf_node -from cellmlmanip.units import UnitCalculator, UnitStore +from cellmlmanip.units import UnitStore, UnitCalculator logger = logging.getLogger(__name__) @@ -817,4 +817,3 @@ def __init__(self, def __str__(self): return self.name - From 4a040086afdf4f49ee2a6af5aeca6d5c152096d3 Mon Sep 17 00:00:00 2001 From: Sarah Date: Fri, 13 Dec 2019 12:57:14 +0000 Subject: [PATCH 09/47] refactoring to make code easier to read first step - more will follow --- cellmlmanip/model.py | 56 ++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 440fe009..979a124e 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -654,9 +654,8 @@ def add_input(self, name, units): if not original_variable: raise KeyError('No variable with name "%s" found.' % name) + # todo what it variable has no cmeta_id original_units = original_variable.units - original_cmeta_id = original_variable.cmeta_id - original_initial_value = self.get_initial_value(original_variable) # no conversion necessary if original_units == units: @@ -667,27 +666,7 @@ def add_input(self, name, units): unit_calculator = UnitCalculator(self.units.ureg) # conversion_factor for old units to new units cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) - - # 1. get unique name for new variable - new_name = name + '_converted' - while new_name in self.variables(): - new_name = new_name + '_a' - - # 2. if original has initial_value calculate new initial value - new_value = None - if original_initial_value: - new_value = original_initial_value * cf - - # 3. copy cmeta_id from original and remove from original - original_variable.cmeta_id = '' - new_variable = self.add_variable(name=new_name, - units=units, - initial_value=new_value, - cmeta_id=original_cmeta_id) - - # 4. add an equation for original variable - expression = sympy.Eq(original_variable, new_variable / cf) - self.add_equation(expression) + new_variable = self._convert_variable_instance(original_variable, cf, units) # if state variable if original_variable in state_symbols: @@ -744,6 +723,37 @@ def add_input(self, name, units): self._invalidate_cache() + def _convert_variable_instance(self, original_variable, cf, units): + """ + Internal function to create new variable and an equation for it. + :param original_variable: VariableDummy object to be converted + :param cf: conversion factor + :param units: Unit object for new units + :return: the new variable created + """ + # 1. get unique name for new variable + new_name = original_variable.name + '_converted' + while new_name in self.variables(): + new_name = new_name + '_a' + + # 2. if original has initial_value calculate new initial value + new_value = None + if original_variable.initial_value: + new_value = original_variable.initial_value * cf + + # 3. copy cmeta_id from original and remove from original + new_variable = self.add_variable(name=new_name, + units=units, + initial_value=new_value, + cmeta_id=original_variable.cmeta_id) + original_variable.cmeta_id = '' + + # 4. add an equation for original variable + expression = sympy.Eq(original_variable, new_variable / cf) + self.add_equation(expression) + + return new_variable + class NumberDummy(sympy.Dummy): """ From ba355e616d9de57738b8c2a2b52789f33597aa82 Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 14 Dec 2019 10:55:14 +0000 Subject: [PATCH 10/47] converted a literal constant --- cellmlmanip/model.py | 12 ++- .../new_basic_ode_for_conversion_tests.cellml | 84 +++++++++++++++++++ tests/test_unit_conversion.py | 62 ++++++++++++++ 3 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 tests/cellml_files/new_basic_ode_for_conversion_tests.cellml diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 979a124e..697611ae 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -731,6 +731,7 @@ def _convert_variable_instance(self, original_variable, cf, units): :param units: Unit object for new units :return: the new variable created """ + eqns = self.get_equations_for([original_variable]) # 1. get unique name for new variable new_name = original_variable.name + '_converted' while new_name in self.variables(): @@ -748,7 +749,16 @@ def _convert_variable_instance(self, original_variable, cf, units): cmeta_id=original_variable.cmeta_id) original_variable.cmeta_id = '' - # 4. add an equation for original variable + # 4. check whether we had an eqn for original + # if so, remove it and replace with new_var = rhs * cf + for equation in self.equations: + if equation.is_Equality and equation.args[0] == original_variable: + expression = sympy.Eq(new_variable, equation.args[1] * cf) + self.add_equation(expression) + self.remove_equation(equation) + break + + # 5. add an equation for original variable expression = sympy.Eq(original_variable, new_variable / cf) self.add_equation(expression) diff --git a/tests/cellml_files/new_basic_ode_for_conversion_tests.cellml b/tests/cellml_files/new_basic_ode_for_conversion_tests.cellml new file mode 100644 index 00000000..9c3aa30a --- /dev/null +++ b/tests/cellml_files/new_basic_ode_for_conversion_tests.cellml @@ -0,0 +1,84 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + time + + sv1 + + 1 + + + + x + 1 + + + + y + + + 1 + x + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index d31773fc..1cc8d9d7 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -13,6 +13,11 @@ def local_model(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ return shared.load_model('basic_ode') + @pytest.fixture + def local_model_2(scope='function'): + """ Fixture to load a local copy of the basic_ode model that may get modified. """ + return shared.load_model('new_basic_ode_for_conversion_tests') + def test_add_preferred_custom_unit_name(self, simple_ode_model): """ Tests Units.add_preferred_custom_unit_name() function. """ time_var = simple_ode_model.get_symbol_by_ontology_term(shared.OXMETA, "time") @@ -194,3 +199,60 @@ def test_original_state(local_model): assert str(local_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1, _environment$time_converted), ' \ '1000.0*_env_ode$sv1_orig_deriv)' + + def test_add_input_literal_variable(self, local_model_2): + """ Tests the Model.add_input function that changes units. """ + # original state + def test_original_state(model): + assert len(model.variables()) == 5 + symbol_a = model.get_symbol_by_cmeta_id('sv11') + symbol_t = model.get_symbol_by_cmeta_id('time') + symbol_x = model.get_symbol_by_cmeta_id('current') + symbol_y = model.get_symbol_by_name('env_ode$y') + assert symbol_x.name == 'env_ode$x' + assert model.get_initial_value(symbol_a) == 2.0 + assert not model.get_initial_value(symbol_t) + assert not model.get_initial_value(symbol_x) + assert not model.get_initial_value(symbol_y) + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_x.units == 'pA' + assert symbol_y.units == 'per_pA' + assert len(model.equations) == 3 + assert str(model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(model.equations[1]) == 'Eq(_env_ode$x, _1.0)' + assert str(model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + return True + + pA_unit = local_model_2.get_units('pA') + nA_unit = local_model_2.get_units('nA') + + assert test_original_state(local_model_2) + # test no change in units + local_model_2.add_input('env_ode$x', pA_unit) + assert test_original_state(local_model_2) + + # change pA to nA + local_model_2.add_input('env_ode$x', nA_unit) + assert len(local_model_2.variables()) == 6 + symbol_a = local_model_2.get_symbol_by_cmeta_id('sv11') + symbol_t = local_model_2.get_symbol_by_cmeta_id('time') + symbol_x = local_model_2.get_symbol_by_cmeta_id('current') + assert symbol_x.name == 'env_ode$x_converted' + symbol_y = local_model_2.get_symbol_by_name('env_ode$y') + symbol_x_orig = local_model_2.get_symbol_by_name('env_ode$x') + assert local_model_2.get_initial_value(symbol_a) == 2.0 + assert not local_model_2.get_initial_value(symbol_t) + assert not local_model_2.get_initial_value(symbol_x) + assert not local_model_2.get_initial_value(symbol_y) + assert not local_model_2.get_initial_value(symbol_x_orig) + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_x.units == 'nA' + assert symbol_y.units == 'per_pA' + assert symbol_x_orig.units == 'pA' + assert len(local_model_2.equations) == 4 + assert str(local_model_2.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(local_model_2.equations[1]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + assert str(local_model_2.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' + assert str(local_model_2.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' From 3604ddf50082ec012b46da900add365231e42573 Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 14 Dec 2019 10:56:37 +0000 Subject: [PATCH 11/47] flake8 --- cellmlmanip/model.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 697611ae..aaf9c6a4 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -731,7 +731,6 @@ def _convert_variable_instance(self, original_variable, cf, units): :param units: Unit object for new units :return: the new variable created """ - eqns = self.get_equations_for([original_variable]) # 1. get unique name for new variable new_name = original_variable.name + '_converted' while new_name in self.variables(): From c3e8bde06dafed6e97d4a7f8c57399be8e4d7d17 Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 14 Dec 2019 12:32:36 +0000 Subject: [PATCH 12/47] refactoring to remove duplication of code --- cellmlmanip/model.py | 117 +++++++++++++++++++++------------- tests/test_unit_conversion.py | 3 + 2 files changed, 74 insertions(+), 46 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index aaf9c6a4..a53a3013 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -654,7 +654,6 @@ def add_input(self, name, units): if not original_variable: raise KeyError('No variable with name "%s" found.' % name) - # todo what it variable has no cmeta_id original_units = original_variable.units # no conversion necessary @@ -662,67 +661,93 @@ def add_input(self, name, units): return state_symbols = self.get_state_symbols() - free_symbols = self.get_free_variable_symbol() - unit_calculator = UnitCalculator(self.units.ureg) + free_symbol = self.get_free_variable_symbol() # conversion_factor for old units to new units cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) + + # create new variable and relevant equations new_variable = self._convert_variable_instance(original_variable, cf, units) # if state variable if original_variable in state_symbols: - # find the derivative equation for this variable - eqn = None - for equation in self.equations: - if equation.args[0].is_Derivative: - if equation.args[0].args[0] == original_variable: - eqn = equation - # get deriv symbol - wrt_variable = eqn.args[0].args[1] - # create new variable for rhs - deriv_name = name + '_orig_deriv' - while deriv_name in self.variables(): - deriv_name = deriv_name + '_a' - deriv_units = unit_calculator.traverse(eqn.args[0]) - new_deriv_variable = self.add_variable(name=deriv_name, - units=deriv_units.units) - # add new equation for new deriv variable and remove existing one - expression = sympy.Eq(new_deriv_variable, eqn.args[1]) - self.add_equation(expression) - self.remove_equation(eqn) + self._convert_state_variable_deriv(original_variable, new_variable, cf) # TODO replace any instance of deriv on rhs of any eqn with new deriv var - # new derivative - expression = sympy.Eq(sympy.Derivative(new_variable, wrt_variable), new_deriv_variable * cf) - self.add_equation(expression) # if free variable - if original_variable == free_symbols: - # find the derivative equation for this variable - # todo free variable might appear in more than one deriv - eqn = None + if original_variable == free_symbol: + # for each derivative wrt to free variable add necessary variables/equations for equation in self.equations: if equation.args[0].is_Derivative: if equation.args[0].args[1].args[0] == original_variable: - eqn = equation - # get deriv symbol - derivative_variable = eqn.args[0].args[0] - # create new variable for rhs - deriv_name = derivative_variable.name + '_orig_deriv' - while deriv_name in self.variables(): - deriv_name = deriv_name + '_a' - deriv_units = unit_calculator.traverse(eqn.args[0]) - new_deriv_variable = self.add_variable(name=deriv_name, - units=deriv_units.units) - # add new equation for new deriv variable and remove existing one - expression = sympy.Eq(new_deriv_variable, eqn.args[1]) - self.add_equation(expression) - self.remove_equation(eqn) + self._convert_free_variable_deriv(equation, new_variable, cf) # TODO replace any instance of deriv on rhs of any eqn with new deriv var - # new derivative - expression = sympy.Eq(sympy.Derivative(derivative_variable, new_variable), new_deriv_variable / cf) - self.add_equation(expression) self._invalidate_cache() + def _create_new_deriv_variable_and_equation(self, eqn, derivative_variable): + """ + Create a new variable and equation for the derivative. + :param eqn: the original derivative eqn + :param derivative_variable: the dependent variable + :return: new variable for the derivative + """ + # 1. create a new variable + deriv_name = derivative_variable.name + '_orig_deriv' + while deriv_name in self.variables(): + deriv_name = deriv_name + '_a' + deriv_units = UnitCalculator(self.units.ureg).traverse(eqn.args[0]) + new_deriv_variable = self.add_variable(name=deriv_name, + units=deriv_units.units) + + # 2. create new equation and remove original + expression = sympy.Eq(new_deriv_variable, eqn.args[1]) + self.add_equation(expression) + self.remove_equation(eqn) + + return new_deriv_variable + + def _convert_free_variable_deriv(self, eqn, new_variable, cf): + """ + Create relevant variables/equations when converting a free variable. + :param eqn: the derivative equation containing free variable + :param new_variable: the new variable representing the converted symbol + :param cf: conversion factor for unit conversion + """ + derivative_variable = eqn.args[0].args[0] + # 1. create a new variable/equation for derivative + new_deriv_variable = self._create_new_deriv_variable_and_equation(eqn, derivative_variable) + + # 3. create equation for derivative wrt new variable + expression = sympy.Eq(sympy.Derivative(derivative_variable, new_variable), new_deriv_variable / cf) + self.add_equation(expression) + + def _convert_state_variable_deriv(self, original_variable, new_variable, cf): + """ + Create relevant variables/equations when converting a state variable. + :param original_variable: the variable to be converted + :param new_variable: the new variable representing the converted symbol + :param cf: conversion factor for unit conversion + :return: + """ + # 1. find the derivative equation for this variable + # and get the free variable (wrt_variable) + eqn = None + for equation in self.equations: + if equation.args[0].is_Derivative: + if equation.args[0].args[0] == original_variable: + eqn = equation + break + + # get free variable symbol + wrt_variable = eqn.args[0].args[1] + + # 2. create new variable for derivative + new_deriv_variable = self._create_new_deriv_variable_and_equation(eqn, original_variable) + + # 3. add a new derivative equation + expression = sympy.Eq(sympy.Derivative(new_variable, wrt_variable), new_deriv_variable * cf) + self.add_equation(expression) + def _convert_variable_instance(self, original_variable, cf, units): """ Internal function to create new variable and an equation for it. diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 1cc8d9d7..b1d0acb6 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -256,3 +256,6 @@ def test_original_state(model): assert str(local_model_2.equations[1]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' assert str(local_model_2.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' assert str(local_model_2.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' + +# todo tests for replacing derivs +# todo tests for multiple free variable derivs From d15d24286600575545bf61ed101de8102330d65d Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 14 Dec 2019 12:38:48 +0000 Subject: [PATCH 13/47] isort maybe --- cellmlmanip/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index a53a3013..56156f19 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -7,7 +7,7 @@ import sympy from cellmlmanip.rdf import create_rdf_node -from cellmlmanip.units import UnitStore, UnitCalculator +from cellmlmanip.units import UnitCalculator, UnitStore logger = logging.getLogger(__name__) From da0eaa44cafbe9994205cedddf3f1339913fb9f6 Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 14 Dec 2019 14:38:39 +0000 Subject: [PATCH 14/47] sort units imports --- cellmlmanip/model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 56156f19..7da8b666 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -7,7 +7,7 @@ import sympy from cellmlmanip.rdf import create_rdf_node -from cellmlmanip.units import UnitCalculator, UnitStore +from cellmlmanip.units import UnitStore logger = logging.getLogger(__name__) @@ -695,7 +695,7 @@ def _create_new_deriv_variable_and_equation(self, eqn, derivative_variable): deriv_name = derivative_variable.name + '_orig_deriv' while deriv_name in self.variables(): deriv_name = deriv_name + '_a' - deriv_units = UnitCalculator(self.units.ureg).traverse(eqn.args[0]) + deriv_units = self.units.calculator.traverse(eqn.args[0]) new_deriv_variable = self.add_variable(name=deriv_name, units=deriv_units.units) From 68d886a577e3bf19ae8156d833bd188923d640f5 Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 14 Dec 2019 14:44:35 +0000 Subject: [PATCH 15/47] isort again ! --- cellmlmanip/model.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 7da8b666..c0117a41 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -6,8 +6,9 @@ import rdflib import sympy -from cellmlmanip.rdf import create_rdf_node from cellmlmanip.units import UnitStore +from cellmlmanip.rdf import create_rdf_node + logger = logging.getLogger(__name__) From 789cfd2b67229727a6e66f59ecca95ea5cd3d136 Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 14 Dec 2019 14:54:34 +0000 Subject: [PATCH 16/47] the imports are unchanged from last time it passed isort so ????? --- cellmlmanip/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index c0117a41..909bdfe5 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -6,8 +6,8 @@ import rdflib import sympy -from cellmlmanip.units import UnitStore from cellmlmanip.rdf import create_rdf_node +from cellmlmanip.units import UnitStore logger = logging.getLogger(__name__) From 1df8bd28ecd30e91252fcb6eb10347fdd83ea78b Mon Sep 17 00:00:00 2001 From: Sarah Date: Tue, 17 Dec 2019 10:28:07 +0000 Subject: [PATCH 17/47] replace any derivative on rhs where units have changed --- cellmlmanip/model.py | 22 +++- .../literals_for_conversion_tests.cellml | 84 +++++++++++++++ .../missing_units_for_conversion_tests.cellml | 81 ++++++++++++++ .../repeated_ode_for_conversion_tests.cellml | 96 +++++++++++++++++ tests/test_unit_conversion.py | 102 +++++++++++++----- 5 files changed, 356 insertions(+), 29 deletions(-) create mode 100644 tests/cellml_files/literals_for_conversion_tests.cellml create mode 100644 tests/cellml_files/missing_units_for_conversion_tests.cellml create mode 100644 tests/cellml_files/repeated_ode_for_conversion_tests.cellml diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 909bdfe5..f2cd741b 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -669,10 +669,11 @@ def add_input(self, name, units): # create new variable and relevant equations new_variable = self._convert_variable_instance(original_variable, cf, units) + deriv_variable = None + deriv_expression = None # if state variable if original_variable in state_symbols: - self._convert_state_variable_deriv(original_variable, new_variable, cf) - # TODO replace any instance of deriv on rhs of any eqn with new deriv var + [deriv_variable, deriv_expression] = self._convert_state_variable_deriv(original_variable, new_variable, cf) # if free variable if original_variable == free_symbol: @@ -681,7 +682,18 @@ def add_input(self, name, units): if equation.args[0].is_Derivative: if equation.args[0].args[1].args[0] == original_variable: self._convert_free_variable_deriv(equation, new_variable, cf) - # TODO replace any instance of deriv on rhs of any eqn with new deriv var + + if deriv_variable: + for equation in self.equations: + for argument in equation.rhs.args: + if deriv_expression == argument: + # add new equation + new_eqn = equation.subs(deriv_expression, deriv_variable) + self.add_equation(new_eqn) + self.remove_equation(equation) + break + + self._invalidate_cache() @@ -728,7 +740,8 @@ def _convert_state_variable_deriv(self, original_variable, new_variable, cf): :param original_variable: the variable to be converted :param new_variable: the new variable representing the converted symbol :param cf: conversion factor for unit conversion - :return: + :return: a tuple containing the new variable representing the original derivative + and the sympy expression for the original derivative function """ # 1. find the derivative equation for this variable # and get the free variable (wrt_variable) @@ -748,6 +761,7 @@ def _convert_state_variable_deriv(self, original_variable, new_variable, cf): # 3. add a new derivative equation expression = sympy.Eq(sympy.Derivative(new_variable, wrt_variable), new_deriv_variable * cf) self.add_equation(expression) + return [new_deriv_variable, eqn.args[0]] def _convert_variable_instance(self, original_variable, cf, units): """ diff --git a/tests/cellml_files/literals_for_conversion_tests.cellml b/tests/cellml_files/literals_for_conversion_tests.cellml new file mode 100644 index 00000000..9c3aa30a --- /dev/null +++ b/tests/cellml_files/literals_for_conversion_tests.cellml @@ -0,0 +1,84 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + time + + sv1 + + 1 + + + + x + 1 + + + + y + + + 1 + x + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/cellml_files/missing_units_for_conversion_tests.cellml b/tests/cellml_files/missing_units_for_conversion_tests.cellml new file mode 100644 index 00000000..d965729d --- /dev/null +++ b/tests/cellml_files/missing_units_for_conversion_tests.cellml @@ -0,0 +1,81 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + time + + sv1 + + 1 + + + + x + 1 + + + + y + + + 1 + x + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/cellml_files/repeated_ode_for_conversion_tests.cellml b/tests/cellml_files/repeated_ode_for_conversion_tests.cellml new file mode 100644 index 00000000..84edcedd --- /dev/null +++ b/tests/cellml_files/repeated_ode_for_conversion_tests.cellml @@ -0,0 +1,96 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + time + + sv1 + + 1 + + + + x + + + + + + time + + sv1 + + 3 + + + + + + + + time + + y + + 2 + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index b1d0acb6..eab063ec 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -13,10 +13,24 @@ def local_model(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ return shared.load_model('basic_ode') + + @pytest.fixture + def model_missing_units(scope='function'): + """ Fixture to load a local copy of the basic_ode model that may get modified. """ + return shared.load_model('missing_units_for_conversion_tests') + + + @pytest.fixture + def literals_model(scope='function'): + """ Fixture to load a local copy of the basic_ode model that may get modified. """ + return shared.load_model('literals_for_conversion_tests') + + @pytest.fixture - def local_model_2(scope='function'): + def multiode_model(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ - return shared.load_model('new_basic_ode_for_conversion_tests') + return shared.load_model('repeated_ode_for_conversion_tests') + def test_add_preferred_custom_unit_name(self, simple_ode_model): """ Tests Units.add_preferred_custom_unit_name() function. """ @@ -200,7 +214,7 @@ def test_original_state(local_model): assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1, _environment$time_converted), ' \ '1000.0*_env_ode$sv1_orig_deriv)' - def test_add_input_literal_variable(self, local_model_2): + def test_add_input_literal_variable(self, literals_model): """ Tests the Model.add_input function that changes units. """ # original state def test_original_state(model): @@ -224,38 +238,76 @@ def test_original_state(model): assert str(model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' return True - pA_unit = local_model_2.get_units('pA') - nA_unit = local_model_2.get_units('nA') + pA_unit = literals_model.get_units('pA') + nA_unit = literals_model.get_units('nA') - assert test_original_state(local_model_2) + assert test_original_state(literals_model) # test no change in units - local_model_2.add_input('env_ode$x', pA_unit) - assert test_original_state(local_model_2) + literals_model.add_input('env_ode$x', pA_unit) + assert test_original_state(literals_model) # change pA to nA - local_model_2.add_input('env_ode$x', nA_unit) - assert len(local_model_2.variables()) == 6 - symbol_a = local_model_2.get_symbol_by_cmeta_id('sv11') - symbol_t = local_model_2.get_symbol_by_cmeta_id('time') - symbol_x = local_model_2.get_symbol_by_cmeta_id('current') + literals_model.add_input('env_ode$x', nA_unit) + assert len(literals_model.variables()) == 6 + symbol_a = literals_model.get_symbol_by_cmeta_id('sv11') + symbol_t = literals_model.get_symbol_by_cmeta_id('time') + symbol_x = literals_model.get_symbol_by_cmeta_id('current') assert symbol_x.name == 'env_ode$x_converted' - symbol_y = local_model_2.get_symbol_by_name('env_ode$y') - symbol_x_orig = local_model_2.get_symbol_by_name('env_ode$x') - assert local_model_2.get_initial_value(symbol_a) == 2.0 - assert not local_model_2.get_initial_value(symbol_t) - assert not local_model_2.get_initial_value(symbol_x) - assert not local_model_2.get_initial_value(symbol_y) - assert not local_model_2.get_initial_value(symbol_x_orig) + symbol_y = literals_model.get_symbol_by_name('env_ode$y') + symbol_x_orig = literals_model.get_symbol_by_name('env_ode$x') + assert literals_model.get_initial_value(symbol_a) == 2.0 + assert not literals_model.get_initial_value(symbol_t) + assert not literals_model.get_initial_value(symbol_x) + assert not literals_model.get_initial_value(symbol_y) + assert not literals_model.get_initial_value(symbol_x_orig) assert symbol_a.units == 'mV' assert symbol_t.units == 'ms' assert symbol_x.units == 'nA' assert symbol_y.units == 'per_pA' assert symbol_x_orig.units == 'pA' - assert len(local_model_2.equations) == 4 - assert str(local_model_2.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(local_model_2.equations[1]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' - assert str(local_model_2.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' - assert str(local_model_2.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' + assert len(literals_model.equations) == 4 + assert str(literals_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(literals_model.equations[1]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + assert str(literals_model.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' + assert str(literals_model.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' + + + def test_multiple_odes(self, multiode_model): + """ Tests the Model.add_input function that changes units. """ + + # original state + def test_original_state(multiode_model): + assert len(multiode_model.variables()) == 5 + symbol_a = multiode_model.get_symbol_by_cmeta_id('sv11') + symbol_t = multiode_model.get_symbol_by_cmeta_id('time') + symbol_x = multiode_model.get_symbol_by_name('env_ode$x') + symbol_y = multiode_model.get_symbol_by_name('env_ode$y') + assert multiode_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert len(multiode_model.equations) == 3 + assert str(multiode_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(multiode_model.equations[1]) == \ + 'Eq(_env_ode$x, _3.0*Derivative(_env_ode$sv1, _environment$time))' + assert str(multiode_model.equations[2]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' + return True + + mV_unit = multiode_model.get_units('mV') + volt_unit = multiode_model.get_units('volt') + + assert test_original_state(multiode_model) + # test no change in units + multiode_model.add_input('env_ode$sv1', mV_unit) + assert test_original_state(multiode_model) + + # change mV to V + multiode_model.add_input('env_ode$sv1', volt_unit) + assert len(multiode_model.equations) == 5 + assert str(multiode_model.equations[4]) == 'Eq(_env_ode$x, _3.0*_env_ode$sv1_orig_deriv)' + assert str(multiode_model.equations[0]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' + assert str(multiode_model.equations[1]) == 'Eq(_env_ode$sv1, 1000.0*_env_ode$sv1_converted)' + assert str(multiode_model.equations[2]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' + assert str(multiode_model.equations[3]) == 'Eq(Derivative(_env_ode$sv1_converted, _environment$time), 0.001*_env_ode$sv1_orig_deriv)' # todo tests for replacing derivs # todo tests for multiple free variable derivs From 6f5d9383d3fcfbee37bd03287f1659db27ca45b7 Mon Sep 17 00:00:00 2001 From: Sarah Date: Tue, 17 Dec 2019 14:47:55 +0000 Subject: [PATCH 18/47] all aspects of add_input --- cellmlmanip/model.py | 43 +-- ...ed_ode_freevar_for_conversion_tests.cellml | 80 ++++++ tests/test_unit_conversion.py | 269 ++++++++++++++++-- 3 files changed, 354 insertions(+), 38 deletions(-) create mode 100644 tests/cellml_files/repeated_ode_freevar_for_conversion_tests.cellml diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index f2cd741b..143d7c2b 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -669,34 +669,39 @@ def add_input(self, name, units): # create new variable and relevant equations new_variable = self._convert_variable_instance(original_variable, cf, units) - deriv_variable = None - deriv_expression = None + new_derivatives = [] # if state variable if original_variable in state_symbols: - [deriv_variable, deriv_expression] = self._convert_state_variable_deriv(original_variable, new_variable, cf) + new_derivatives.append(self._convert_state_variable_deriv(original_variable, new_variable, cf)) # if free variable if original_variable == free_symbol: # for each derivative wrt to free variable add necessary variables/equations - for equation in self.equations: + current_equations = self.equations.copy() + for equation in current_equations: if equation.args[0].is_Derivative: if equation.args[0].args[1].args[0] == original_variable: - self._convert_free_variable_deriv(equation, new_variable, cf) - - if deriv_variable: - for equation in self.equations: - for argument in equation.rhs.args: - if deriv_expression == argument: - # add new equation - new_eqn = equation.subs(deriv_expression, deriv_variable) - self.add_equation(new_eqn) - self.remove_equation(equation) - break - + new_derivatives.append(self._convert_free_variable_deriv(equation, new_variable, cf)) + for new_derivative in new_derivatives: + self._replace_derivatives(new_derivative) self._invalidate_cache() + def _replace_derivatives(self, new_derivative): + """ + Function to replace an instance of a derivative that occurs on the RHS of any equation + :param new_derivative: new variable representing the derivative + """ + for equation in self.equations: + for argument in equation.rhs.args: + if new_derivative['expression'] == argument: + # add new equation + new_eqn = equation.subs(new_derivative['expression'], new_derivative['variable']) + self.add_equation(new_eqn) + self.remove_equation(equation) + break + def _create_new_deriv_variable_and_equation(self, eqn, derivative_variable): """ Create a new variable and equation for the derivative. @@ -733,6 +738,7 @@ def _convert_free_variable_deriv(self, eqn, new_variable, cf): # 3. create equation for derivative wrt new variable expression = sympy.Eq(sympy.Derivative(derivative_variable, new_variable), new_deriv_variable / cf) self.add_equation(expression) + return {'variable': new_deriv_variable, 'expression': eqn.args[0]} def _convert_state_variable_deriv(self, original_variable, new_variable, cf): """ @@ -740,8 +746,7 @@ def _convert_state_variable_deriv(self, original_variable, new_variable, cf): :param original_variable: the variable to be converted :param new_variable: the new variable representing the converted symbol :param cf: conversion factor for unit conversion - :return: a tuple containing the new variable representing the original derivative - and the sympy expression for the original derivative function + :return: a dictionary containing the 'variable' and 'expression' for new derivative """ # 1. find the derivative equation for this variable # and get the free variable (wrt_variable) @@ -761,7 +766,7 @@ def _convert_state_variable_deriv(self, original_variable, new_variable, cf): # 3. add a new derivative equation expression = sympy.Eq(sympy.Derivative(new_variable, wrt_variable), new_deriv_variable * cf) self.add_equation(expression) - return [new_deriv_variable, eqn.args[0]] + return {'variable': new_deriv_variable, 'expression': eqn.args[0]} def _convert_variable_instance(self, original_variable, cf, units): """ diff --git a/tests/cellml_files/repeated_ode_freevar_for_conversion_tests.cellml b/tests/cellml_files/repeated_ode_freevar_for_conversion_tests.cellml new file mode 100644 index 00000000..c9fc727d --- /dev/null +++ b/tests/cellml_files/repeated_ode_freevar_for_conversion_tests.cellml @@ -0,0 +1,80 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + time + + sv1 + + 1 + + + + + + + time + + y + + 2 + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index eab063ec..6afd68ee 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -13,24 +13,25 @@ def local_model(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ return shared.load_model('basic_ode') - @pytest.fixture def model_missing_units(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ return shared.load_model('missing_units_for_conversion_tests') - @pytest.fixture def literals_model(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ return shared.load_model('literals_for_conversion_tests') - @pytest.fixture def multiode_model(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ return shared.load_model('repeated_ode_for_conversion_tests') + @pytest.fixture + def multiode_freevar_model(scope='function'): + """ Fixture to load a local copy of the basic_ode model that may get modified. """ + return shared.load_model('repeated_ode_freevar_for_conversion_tests') def test_add_preferred_custom_unit_name(self, simple_ode_model): """ Tests Units.add_preferred_custom_unit_name() function. """ @@ -129,7 +130,28 @@ def test_add_input_invalid_args(self, local_model): local_model.add_input('nonsense_name', mV_unit) def test_add_input_state_variable(self, local_model): - """ Tests the Model.add_input function that changes units. """ + """ Tests the Model.add_input function that changes units. + This particular test is when a state variable is being converted + e.g. + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + + ode(sv1, time) = 1{mV_per_ms}; + + convert sv11 from mV to V + + becomes + var{time} time: ms {pub: in}; + var{sv11} sv1_converted: V {init: 0.002}; + var sv1 mV {init: 2} + var sv1_orig_deriv mV_per_ms + + ode(y, time) = 2{mv_per_ms}; + sv1 = 1000 * sv1_converted + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1_converted, time) = 0.001 * sv1_orig_deriv + x = 3 * sv1_orig_deriv + """ # original state def test_original_state(local_model): assert len(local_model.variables()) == 3 @@ -150,9 +172,6 @@ def test_original_state(local_model): local_model.add_input('env_ode$sv1', mV_unit) assert test_original_state(local_model) - # non-existent unit - # TODO what if unit not in model - # change mV to V local_model.add_input('env_ode$sv1', volt_unit) assert len(local_model.variables()) == 5 @@ -164,8 +183,10 @@ def test_original_state(local_model): assert symbol_t.units == 'ms' symbol_orig = local_model.get_symbol_by_name('env_ode$sv1') assert symbol_orig.units == 'mV' + assert local_model.get_initial_value(symbol_orig) == 2.0 symbol_derv = local_model.get_symbol_by_name('env_ode$sv1_orig_deriv') assert symbol_derv.units == 'mV / ms' + assert not local_model.get_initial_value(symbol_derv) assert len(local_model.equations) == 3 assert str(local_model.equations[0]) == 'Eq(_env_ode$sv1, 1000.0*_env_ode$sv1_converted)' assert str(local_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' @@ -173,7 +194,26 @@ def test_original_state(local_model): '0.001*_env_ode$sv1_orig_deriv)' def test_add_input_free_variable(self, local_model): - """ Tests the Model.add_input function that changes units. """ + """ Tests the Model.add_input function that changes units. + This particular case tests changing a free variable + e.g. + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + + ode(sv1, time) = 1{mV_per_ms}; + + convert time from ms to s + + becomes + var time: ms; + var{time} time_converted: s; + var{sv11} sv1: mV {init: 2}; + var sv1_orig_deriv mV_per_ms + + time = 1000 * time_converted; + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1, time_converted) = 1000 * sv1_orig_deriv + """ # original state def test_original_state(local_model): assert len(local_model.variables()) == 3 @@ -215,7 +255,30 @@ def test_original_state(local_model): '1000.0*_env_ode$sv1_orig_deriv)' def test_add_input_literal_variable(self, literals_model): - """ Tests the Model.add_input function that changes units. """ + """ Tests the Model.add_input function that changes units. + This particular case tests changing a free variable + e.g. + var time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var{current} x: pA; + var y: per_pA; + + ode(sv1, time) = 1{mV_per_ms}; + x = 1{pA}; + y = 1{dimensionless}/x; + + change x from pA to nA + var time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var x: pA; + var y: per_pA; + var x_converted: nA + + ode(sv1, time) = 1 :mV_per_ms; + x = 1000 * x_converted: pA; + y = 1{dimensionless}/x; + x_converted = 0.001 * 1 : nA + """ # original state def test_original_state(model): assert len(model.variables()) == 5 @@ -271,24 +334,127 @@ def test_original_state(model): assert str(literals_model.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' assert str(literals_model.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' + def test_add_input_free_variable_multiple(self, multiode_freevar_model): + """ Tests the Model.add_input function that changes units. + This particular case tests changing a free variable + e.g. + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var y: mV {init: 3}; + ode(sv1, time) = 1{mV_per_ms}; + ode(y, time) = 2{mV_per_ms} + + convert time from ms to s + + becomes + var time: ms; + var{time} time_converted: s; + var{sv11} sv1: mV {init: 2}; + var sv1_orig_deriv mV_per_ms + var y : mV + var y_orig_deriv mV_per_ms + + time = 1000 * time_converted; + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1, time_converted) = 1000 * sv1_orig_deriv + y_orig_deriv = 2{mV_per_ms} + ode(y, time_converted) = 1000 * y_orig_deriv + """ + # original state + def test_original_state(multiode_freevar_model): + assert len(multiode_freevar_model.variables()) == 4 + symbol_a = multiode_freevar_model.get_symbol_by_cmeta_id('sv11') + symbol_t = multiode_freevar_model.get_symbol_by_cmeta_id('time') + symbol_y = multiode_freevar_model.get_symbol_by_name('env_ode$y') + assert multiode_freevar_model.get_initial_value(symbol_a) == 2.0 + assert multiode_freevar_model.get_initial_value(symbol_y) == 3.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_y.units == 'mV' + assert len(multiode_freevar_model.equations) == 2 + assert str(multiode_freevar_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(multiode_freevar_model.equations[1]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' + return True - def test_multiple_odes(self, multiode_model): - """ Tests the Model.add_input function that changes units. """ + ms_unit = multiode_freevar_model.get_units('ms') + second_unit = multiode_freevar_model.get_units('second') + assert test_original_state(multiode_freevar_model) + # test no change in units + multiode_freevar_model.add_input('env_ode$time', ms_unit) + assert test_original_state(multiode_freevar_model) + + # change ms to s + multiode_freevar_model.add_input('environment$time', second_unit) + assert len(multiode_freevar_model.variables()) == 7 + symbol_a = multiode_freevar_model.get_symbol_by_cmeta_id('sv11') + assert multiode_freevar_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_a.name == 'env_ode$sv1' + symbol_t = multiode_freevar_model.get_symbol_by_cmeta_id('time') + assert symbol_t.units == 'second' + assert symbol_t.name == 'environment$time_converted' + symbol_orig = multiode_freevar_model.get_symbol_by_name('env_ode$sv1') + assert symbol_orig.units == 'mV' + symbol_derv = multiode_freevar_model.get_symbol_by_name('env_ode$sv1_orig_deriv') + assert symbol_derv.units == 'mV / ms' + symbol_orig_y = multiode_freevar_model.get_symbol_by_name('env_ode$y') + assert symbol_orig_y.units == 'mV' + symbol_derv_y = multiode_freevar_model.get_symbol_by_name('env_ode$y_orig_deriv') + assert symbol_derv_y.units == 'mV / ms' + assert len(multiode_freevar_model.equations) == 5 + assert str(multiode_freevar_model.equations[0]) == 'Eq(_environment$time, 1000.0*_environment$time_converted)' + assert str(multiode_freevar_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' + assert str(multiode_freevar_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1, ' \ + '_environment$time_converted), ' \ + '1000.0*_env_ode$sv1_orig_deriv)' + assert str(multiode_freevar_model.equations[3]) == 'Eq(_env_ode$y_orig_deriv, _2.0)' + assert str(multiode_freevar_model.equations[4]) == 'Eq(Derivative(_env_ode$y, _environment$time_converted), ' \ + '1000.0*_env_ode$y_orig_deriv)' + + def test_multiple_odes(self, multiode_model): + """ Tests the Model.add_input function that changes units. + This tests that any other uses of the derivative on the rhs of other equations + are replaced with the new variable representing the old derivative + e.g. + var time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var x: mV_per_ms; + var y: mV {init: 3}; + + ode(sv1, time) = 1{mV_per_ms}; + x = ode(sv1, time) * 3{mV_per_ms}; + ode(y, time) = 2{mV_per_ms}; + + change sv1 from mV to V + + becomes + var{time} time: ms {pub: in}; + var{sv11} sv1_converted: V {init: 0.002}; + var sv1 mV {init: 2} + var sv1_orig_deriv mV_per_ms + var x: mV_per_ms; + var y: mV {init: 3}; + var y_orig_deriv + + ode(y, time) = 2{mv_per_ms}; + sv1 = 100 * sv1_converted + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1_converted, time) = 0.001 * sv1_orig_deriv + x = 3 * sv1_orig_deriv + """ # original state def test_original_state(multiode_model): assert len(multiode_model.variables()) == 5 symbol_a = multiode_model.get_symbol_by_cmeta_id('sv11') symbol_t = multiode_model.get_symbol_by_cmeta_id('time') - symbol_x = multiode_model.get_symbol_by_name('env_ode$x') - symbol_y = multiode_model.get_symbol_by_name('env_ode$y') assert multiode_model.get_initial_value(symbol_a) == 2.0 assert symbol_a.units == 'mV' assert symbol_t.units == 'ms' assert len(multiode_model.equations) == 3 assert str(multiode_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(multiode_model.equations[1]) == \ - 'Eq(_env_ode$x, _3.0*Derivative(_env_ode$sv1, _environment$time))' + assert str(multiode_model.equations[1]) == 'Eq(_env_ode$x, ' \ + '_3.0*Derivative(_env_ode$sv1, _environment$time))' assert str(multiode_model.equations[2]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' return True @@ -303,11 +469,76 @@ def test_original_state(multiode_model): # change mV to V multiode_model.add_input('env_ode$sv1', volt_unit) assert len(multiode_model.equations) == 5 - assert str(multiode_model.equations[4]) == 'Eq(_env_ode$x, _3.0*_env_ode$sv1_orig_deriv)' assert str(multiode_model.equations[0]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' assert str(multiode_model.equations[1]) == 'Eq(_env_ode$sv1, 1000.0*_env_ode$sv1_converted)' assert str(multiode_model.equations[2]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' - assert str(multiode_model.equations[3]) == 'Eq(Derivative(_env_ode$sv1_converted, _environment$time), 0.001*_env_ode$sv1_orig_deriv)' + assert str(multiode_model.equations[3]) == 'Eq(Derivative(_env_ode$sv1_converted, _environment$time), ' \ + '0.001*_env_ode$sv1_orig_deriv)' + assert str(multiode_model.equations[4]) == 'Eq(_env_ode$x, _3.0*_env_ode$sv1_orig_deriv)' + + def test_multiple_odes_1(self, multiode_model): + """ Tests the Model.add_input function that changes units. + This tests that any other uses of the derivative on the rhs of other equations + are replaced with the new variable representing the old derivative + e.g. + var time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var x: mV_per_ms; + var y: mV {init: 3}; + + ode(sv1, time) = 1{mV_per_ms}; + x = ode(sv1, time) * 3{mV_per_ms}; + ode(y, time) = 2{mV_per_ms}; + + change time from ms to s + + becomes + var time: ms; + var{time} time_converted: s; + var{sv11} sv1: mV {init: 2}; + var sv1_orig_deriv mV_per_ms + var x: mV_per_ms; + var y: mV {init: 3}; + var y_orig_deriv: {mv_per_ms} + + ode(y, time) = 2{mv_per_ms}; + sv1 = 100 * sv1_converted + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1_converted, time) = 0.001 * sv1_orig_deriv + x = 3 * sv1_orig_deriv + """ + # original state + def test_original_state(multiode_model): + assert len(multiode_model.variables()) == 5 + symbol_a = multiode_model.get_symbol_by_cmeta_id('sv11') + symbol_t = multiode_model.get_symbol_by_cmeta_id('time') + assert multiode_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert len(multiode_model.equations) == 3 + assert str(multiode_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(multiode_model.equations[1]) == 'Eq(_env_ode$x, ' \ + '_3.0*Derivative(_env_ode$sv1, _environment$time))' + assert str(multiode_model.equations[2]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' + return True + + ms_unit = multiode_model.get_units('ms') + second_unit = multiode_model.get_units('second') + + assert test_original_state(multiode_model) + # test no change in units + multiode_model.add_input('env_ode$time', ms_unit) + assert test_original_state(multiode_model) + + # change ms to s + multiode_model.add_input('environment$time', second_unit) + assert len(multiode_model.equations) == 6 + assert str(multiode_model.equations[0]) == 'Eq(_environment$time, 1000.0*_environment$time_converted)' + assert str(multiode_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' + assert str(multiode_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1, _environment$time_converted), ' \ + '1000.0*_env_ode$sv1_orig_deriv)' + assert str(multiode_model.equations[3]) == 'Eq(_env_ode$y_orig_deriv, _2.0)' + assert str(multiode_model.equations[4]) == 'Eq(Derivative(_env_ode$y, _environment$time_converted), ' \ + '1000.0*_env_ode$y_orig_deriv)' + assert str(multiode_model.equations[5]) == 'Eq(_env_ode$x, _3.0*_env_ode$sv1_orig_deriv)' -# todo tests for replacing derivs -# todo tests for multiple free variable derivs From d25abe7aebc800f009c0dd693ed0536fd66f751a Mon Sep 17 00:00:00 2001 From: Sarah Date: Tue, 17 Dec 2019 17:09:21 +0000 Subject: [PATCH 19/47] add output --- cellmlmanip/model.py | 20 ++++-- tests/test_unit_conversion.py | 115 +++++++++++++++++++++++++++++++++- 2 files changed, 129 insertions(+), 6 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 143d7c2b..9710a09f 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -619,7 +619,7 @@ def variables(self): """ Returns an iterator over this model's variable symbols. """ return self._name_to_symbol.values() - def add_input(self, name, units): + def add_input(self, name, units, is_output=False): """ Adds an additional input for the variable name to the model with new units and apply any necessary conversions/add additional equations. @@ -642,14 +642,11 @@ def add_input(self, name, units): :param name: name of variable whose units are to be changed :param units: A `pint` units representation - + :param is_output: the variable is an output rather than an input """ # check units is of type Unit assert isinstance(units, self.units.ureg.Unit) - # TODO should I throw the exception or user seamlessly gets a non changed model - # if unit not in model - # check variable is in model original_variable = self.get_symbol_by_name(name) if not original_variable: @@ -669,6 +666,10 @@ def add_input(self, name, units): # create new variable and relevant equations new_variable = self._convert_variable_instance(original_variable, cf, units) + # if is output do not need to do additional changes for state/free symbols + if is_output: + return + new_derivatives = [] # if state variable if original_variable in state_symbols: @@ -683,11 +684,20 @@ def add_input(self, name, units): if equation.args[0].args[1].args[0] == original_variable: new_derivatives.append(self._convert_free_variable_deriv(equation, new_variable, cf)) + # replace any instances of derivative of rhs of other eqns with new derivative variable for new_derivative in new_derivatives: self._replace_derivatives(new_derivative) self._invalidate_cache() + def add_output(self, name, units): + """ Adds an additional output for the variable name to the model with new units + and apply any necessary conversions/add additional equations. + :param name: name of variable whose units are to be changed + :param units: A `pint` units representation + """ + self.add_input(name, units, is_output=True) + def _replace_derivatives(self, new_derivative): """ Function to replace an instance of a derivative that occurs on the RHS of any equation diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 6afd68ee..ed0343f0 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -256,7 +256,7 @@ def test_original_state(local_model): def test_add_input_literal_variable(self, literals_model): """ Tests the Model.add_input function that changes units. - This particular case tests changing a free variable + This particular case tests changing a literal variable/constant e.g. var time: ms {pub: in}; var{sv11} sv1: mV {init: 2}; @@ -542,3 +542,116 @@ def test_original_state(multiode_model): '1000.0*_env_ode$y_orig_deriv)' assert str(multiode_model.equations[5]) == 'Eq(_env_ode$x, _3.0*_env_ode$sv1_orig_deriv)' + def test_add_output(self, literals_model): + """ Tests the Model.add_output function that changes units. + e.g. + var time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var{current} x: pA; + var y: per_pA; + + ode(sv1, time) = 1{mV_per_ms}; + x = 1{pA}; + y = 1{dimensionless}/x; + + change x from pA to nA + var time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var x: pA; + var y: per_pA; + var{current} x_converted: nA + + ode(sv1, time) = 1 :mV_per_ms; + x = 1000 * x_converted: pA; + y = 1{dimensionless}/x; + x_converted = 0.001 * 1 : nA + """ + # original state + def test_original_state(model): + assert len(model.variables()) == 5 + symbol_a = model.get_symbol_by_cmeta_id('sv11') + symbol_t = model.get_symbol_by_cmeta_id('time') + symbol_x = model.get_symbol_by_cmeta_id('current') + symbol_y = model.get_symbol_by_name('env_ode$y') + assert symbol_x.name == 'env_ode$x' + assert model.get_initial_value(symbol_a) == 2.0 + assert not model.get_initial_value(symbol_t) + assert not model.get_initial_value(symbol_x) + assert not model.get_initial_value(symbol_y) + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_x.units == 'pA' + assert symbol_y.units == 'per_pA' + assert len(model.equations) == 3 + assert str(model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(model.equations[1]) == 'Eq(_env_ode$x, _1.0)' + assert str(model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + return True + + pA_unit = literals_model.get_units('pA') + nA_unit = literals_model.get_units('nA') + + assert test_original_state(literals_model) + # test no change in units + literals_model.add_output('env_ode$x', pA_unit) + assert test_original_state(literals_model) + + # change pA to nA + literals_model.add_output('env_ode$x', nA_unit) + assert len(literals_model.variables()) == 6 + symbol_a = literals_model.get_symbol_by_cmeta_id('sv11') + symbol_t = literals_model.get_symbol_by_cmeta_id('time') + symbol_x = literals_model.get_symbol_by_cmeta_id('current') + assert symbol_x.name == 'env_ode$x_converted' + symbol_y = literals_model.get_symbol_by_name('env_ode$y') + symbol_x_orig = literals_model.get_symbol_by_name('env_ode$x') + assert literals_model.get_initial_value(symbol_a) == 2.0 + assert not literals_model.get_initial_value(symbol_t) + assert not literals_model.get_initial_value(symbol_x) + assert not literals_model.get_initial_value(symbol_y) + assert not literals_model.get_initial_value(symbol_x_orig) + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_x.units == 'nA' + assert symbol_y.units == 'per_pA' + assert symbol_x_orig.units == 'pA' + assert len(literals_model.equations) == 4 + assert str(literals_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(literals_model.equations[1]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + assert str(literals_model.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' + assert str(literals_model.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' + + # def test_missing_units(self, model_missing_units, literals_model): + # """ Tests the Model.add_output function that changes units. + # In this case the model needs to add the unit it wants to change to. + # e.g. + # var time: ms {pub: in}; + # var{sv11} sv1: mV {init: 2}; + # var{current} x: pA; + # var y: per_pA; + # + # ode(sv1, time) = 1{mV_per_ms}; + # x = 1{pA}; + # y = 1{dimensionless}/x; + # + # change x from pA to nA + # var time: ms {pub: in}; + # var{sv11} sv1: mV {init: 2}; + # var x: pA; + # var y: per_pA; + # var{current} x_converted: nA + # + # ode(sv1, time) = 1 :mV_per_ms; + # x = 1000 * x_converted: pA; + # y = 1{dimensionless}/x; + # x_converted = 0.001 * 1 : nA + # """ + # pA_unit = model_missing_units.get_units('pA') + # nA_unit = literals_model.get_units('nA') + # + # # check nA not in missing_units + # with pytest.raises(KeyError): + # model_missing_units.get_units('nA') + # + # model_missing_units.add_input('env_ode$x', nA_unit) + # From 17f7adcf99487eee985dc5883a6fe28cf77c0074 Mon Sep 17 00:00:00 2001 From: Sarah Date: Thu, 19 Dec 2019 10:07:12 +0000 Subject: [PATCH 20/47] slight change in order of code --- cellmlmanip/model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 9710a09f..1852b8ea 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -658,8 +658,6 @@ def add_input(self, name, units, is_output=False): if original_units == units: return - state_symbols = self.get_state_symbols() - free_symbol = self.get_free_variable_symbol() # conversion_factor for old units to new units cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) @@ -670,6 +668,8 @@ def add_input(self, name, units, is_output=False): if is_output: return + state_symbols = self.get_state_symbols() + free_symbol = self.get_free_variable_symbol() new_derivatives = [] # if state variable if original_variable in state_symbols: From b7c0ca2e8ed54ee3f0c1c4da8004c32349221ae3 Mon Sep 17 00:00:00 2001 From: Sarah Date: Thu, 19 Dec 2019 10:07:12 +0000 Subject: [PATCH 21/47] Revert "slight change in order of code" This reverts commit 17f7adcf99487eee985dc5883a6fe28cf77c0074. --- cellmlmanip/model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 1852b8ea..9710a09f 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -658,6 +658,8 @@ def add_input(self, name, units, is_output=False): if original_units == units: return + state_symbols = self.get_state_symbols() + free_symbol = self.get_free_variable_symbol() # conversion_factor for old units to new units cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) @@ -668,8 +670,6 @@ def add_input(self, name, units, is_output=False): if is_output: return - state_symbols = self.get_state_symbols() - free_symbol = self.get_free_variable_symbol() new_derivatives = [] # if state variable if original_variable in state_symbols: From 4c6df83eb0dfeea21d1523483873bdbf8b944b83 Mon Sep 17 00:00:00 2001 From: Sarah Date: Thu, 19 Dec 2019 10:18:16 +0000 Subject: [PATCH 22/47] Auto stash before revert of "slight change in order of code" --- tests/test_unit_conversion.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index ed0343f0..74625ad7 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -132,25 +132,28 @@ def test_add_input_invalid_args(self, local_model): def test_add_input_state_variable(self, local_model): """ Tests the Model.add_input function that changes units. This particular test is when a state variable is being converted - e.g. - var{time} time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - ode(sv1, time) = 1{mV_per_ms}; + For example:: - convert sv11 from mV to V + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; - becomes - var{time} time: ms {pub: in}; - var{sv11} sv1_converted: V {init: 0.002}; - var sv1 mV {init: 2} - var sv1_orig_deriv mV_per_ms + ode(sv1, time) = 1{mV_per_ms}; - ode(y, time) = 2{mv_per_ms}; - sv1 = 1000 * sv1_converted - sv1_orig_deriv = 1{mV_per_ms} - ode(sv1_converted, time) = 0.001 * sv1_orig_deriv - x = 3 * sv1_orig_deriv + convert sv11 from mV to V + + creates model + var{time} time: ms {pub: in}; + var{sv11} sv1_converted: V {init: 0.002}; + var sv1 mV {init: 2} + var sv1_orig_deriv mV_per_ms + + ode(y, time) = 2{mv_per_ms}; + sv1 = 1000 * sv1_converted + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1_converted, time) = 0.001 * sv1_orig_deriv + x = 3 * sv1_orig_deriv """ # original state def test_original_state(local_model): From 3f7eef8e0308c4e70b9a99ac03823427bc2370c9 Mon Sep 17 00:00:00 2001 From: Sarah Date: Thu, 19 Dec 2019 17:31:37 +0000 Subject: [PATCH 23/47] sorted most differences between input/output got a bit tied in knots but this works (needs editing --- cellmlmanip/model.py | 76 +++++++--- tests/test_unit_conversion.py | 261 +++++++++++++++++++++++++++------- 2 files changed, 260 insertions(+), 77 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 9710a09f..5323671b 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -664,7 +664,7 @@ def add_input(self, name, units, is_output=False): cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) # create new variable and relevant equations - new_variable = self._convert_variable_instance(original_variable, cf, units) + new_variable = self._convert_variable_instance(original_variable, cf, units, is_output) # if is output do not need to do additional changes for state/free symbols if is_output: @@ -736,16 +736,18 @@ def _create_new_deriv_variable_and_equation(self, eqn, derivative_variable): def _convert_free_variable_deriv(self, eqn, new_variable, cf): """ - Create relevant variables/equations when converting a free variable. + Create relevant variables/equations when converting a free variable derivative. :param eqn: the derivative equation containing free variable - :param new_variable: the new variable representing the converted symbol - :param cf: conversion factor for unit conversion + :param new_variable: the new variable representing the converted symbol [new_units] + :param cf: conversion factor for unit conversion [new units/old units] """ - derivative_variable = eqn.args[0].args[0] - # 1. create a new variable/equation for derivative + derivative_variable = eqn.args[0].args[0] # units [x] + # 1. create a new variable/equation for original derivative + # will have units [x/old units] new_deriv_variable = self._create_new_deriv_variable_and_equation(eqn, derivative_variable) - # 3. create equation for derivative wrt new variable + # 2. create equation for derivative wrt new variable + # dx/dnewvar [x/new units] = new_deriv_var [x/old units] / cf [new units/old units] expression = sympy.Eq(sympy.Derivative(derivative_variable, new_variable), new_deriv_variable / cf) self.add_equation(expression) return {'variable': new_deriv_variable, 'expression': eqn.args[0]} @@ -753,9 +755,9 @@ def _convert_free_variable_deriv(self, eqn, new_variable, cf): def _convert_state_variable_deriv(self, original_variable, new_variable, cf): """ Create relevant variables/equations when converting a state variable. - :param original_variable: the variable to be converted - :param new_variable: the new variable representing the converted symbol - :param cf: conversion factor for unit conversion + :param original_variable: the variable to be converted [old units] + :param new_variable: the new variable representing the converted symbol [new units] + :param cf: conversion factor for unit conversion [new units/old units] :return: a dictionary containing the 'variable' and 'expression' for new derivative """ # 1. find the derivative equation for this variable @@ -768,23 +770,27 @@ def _convert_state_variable_deriv(self, original_variable, new_variable, cf): break # get free variable symbol + # units [x] wrt_variable = eqn.args[0].args[1] - # 2. create new variable for derivative + # 1. create a new variable/equation for original derivative + # will have units [old units/x] new_deriv_variable = self._create_new_deriv_variable_and_equation(eqn, original_variable) - # 3. add a new derivative equation + # 2. add a new derivative equation + # dnewvar/dx [new units/x] = new_deriv_var [old units/x] * cf [new units/old units] expression = sympy.Eq(sympy.Derivative(new_variable, wrt_variable), new_deriv_variable * cf) self.add_equation(expression) return {'variable': new_deriv_variable, 'expression': eqn.args[0]} - def _convert_variable_instance(self, original_variable, cf, units): + def _convert_variable_instance(self, original_variable, cf, units, is_output): """ Internal function to create new variable and an equation for it. - :param original_variable: VariableDummy object to be converted - :param cf: conversion factor + :param original_variable: VariableDummy object to be converted [old units] + :param cf: conversion factor [new units/old units] :param units: Unit object for new units - :return: the new variable created + :param is_output: boolean are we adding input or output + :return: the new variable created [new units] """ # 1. get unique name for new variable new_name = original_variable.name + '_converted' @@ -804,17 +810,41 @@ def _convert_variable_instance(self, original_variable, cf, units): original_variable.cmeta_id = '' # 4. check whether we had an eqn for original - # if so, remove it and replace with new_var = rhs * cf + # if so, remove it and replace with new_var [new units] = rhs [old units] * cf [new units/old units] + original_had_eqn = False for equation in self.equations: if equation.is_Equality and equation.args[0] == original_variable: - expression = sympy.Eq(new_variable, equation.args[1] * cf) - self.add_equation(expression) - self.remove_equation(equation) + if is_output: + expression = sympy.Eq(new_variable, original_variable * cf) + self.add_equation(expression) + else: + expression = sympy.Eq(new_variable, equation.args[1] * cf) + self.add_equation(expression) + self.remove_equation(equation) + original_had_eqn = True break + if is_output: + print(new_variable.name + ' output') + else: + print(new_variable.name + ' not output') + if original_had_eqn: + print(new_variable.name + ' eqn') + else: + print(new_variable.name + ' not eqn') + # 5. add an equation for original variable if there wasnt one + # oldvar [old units] = newvar [new units] / cf [new units/old units] + if not is_output: + # if not original_had_eqn: + expression = sympy.Eq(original_variable, new_variable / cf) + self.add_equation(expression) + else: + if not original_had_eqn: + expression = sympy.Eq(new_variable, original_variable * cf) + self.add_equation(expression) + # else: + # expression = sympy.Eq(original_variable, new_variable / cf) + # self.add_equation(expression) - # 5. add an equation for original variable - expression = sympy.Eq(original_variable, new_variable / cf) - self.add_equation(expression) return new_variable diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 74625ad7..90a48407 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -149,11 +149,9 @@ def test_add_input_state_variable(self, local_model): var sv1 mV {init: 2} var sv1_orig_deriv mV_per_ms - ode(y, time) = 2{mv_per_ms}; sv1 = 1000 * sv1_converted sv1_orig_deriv = 1{mV_per_ms} ode(sv1_converted, time) = 0.001 * sv1_orig_deriv - x = 3 * sv1_orig_deriv """ # original state def test_original_state(local_model): @@ -165,6 +163,9 @@ def test_original_state(local_model): assert symbol_t.units == 'ms' assert len(local_model.equations) == 1 assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + state_symbols = local_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols return True mV_unit = local_model.get_units('mV') @@ -196,26 +197,32 @@ def test_original_state(local_model): assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1_converted, _environment$time), ' \ '0.001*_env_ode$sv1_orig_deriv)' + state_symbols = local_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + def test_add_input_free_variable(self, local_model): """ Tests the Model.add_input function that changes units. This particular case tests changing a free variable - e.g. - var{time} time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; + For example:: - ode(sv1, time) = 1{mV_per_ms}; + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + + ode(sv1, time) = 1{mV_per_ms}; convert time from ms to s becomes - var time: ms; - var{time} time_converted: s; - var{sv11} sv1: mV {init: 2}; - var sv1_orig_deriv mV_per_ms + var time: ms; + var{time} time_converted: s; + var{sv11} sv1: mV {init: 2}; + var sv1_orig_deriv mV_per_ms - time = 1000 * time_converted; - sv1_orig_deriv = 1{mV_per_ms} - ode(sv1, time_converted) = 1000 * sv1_orig_deriv + time = 1000 * time_converted; + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1, time_converted) = 1000 * sv1_orig_deriv """ # original state def test_original_state(local_model): @@ -260,27 +267,31 @@ def test_original_state(local_model): def test_add_input_literal_variable(self, literals_model): """ Tests the Model.add_input function that changes units. This particular case tests changing a literal variable/constant - e.g. - var time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - var{current} x: pA; - var y: per_pA; + For example:: - ode(sv1, time) = 1{mV_per_ms}; - x = 1{pA}; - y = 1{dimensionless}/x; + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var{current} x: pA; + var y: per_pA; - change x from pA to nA - var time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - var x: pA; - var y: per_pA; - var x_converted: nA - - ode(sv1, time) = 1 :mV_per_ms; - x = 1000 * x_converted: pA; - y = 1{dimensionless}/x; - x_converted = 0.001 * 1 : nA + ode(sv1, time) = 1{mV_per_ms}; + x = 1{pA}; + y = 1{dimensionless}/x; + + convert current from pA to nA + + becomes + var time: ms; + var{sv11} sv1: mV {init: 2}; + var{current} x_converted: nA; + var x : pA + var y: per_pA; + + ode(sv1, time) = 1{mV_per_ms}; + x_converted = 0.001 * 1{pA} + x = 1000* x_converted; + y = 1{dimensionless}/x; """ # original state def test_original_state(model): @@ -547,27 +558,32 @@ def test_original_state(multiode_model): def test_add_output(self, literals_model): """ Tests the Model.add_output function that changes units. - e.g. - var time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - var{current} x: pA; - var y: per_pA; - ode(sv1, time) = 1{mV_per_ms}; - x = 1{pA}; - y = 1{dimensionless}/x; + For example:: - change x from pA to nA - var time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - var x: pA; - var y: per_pA; - var{current} x_converted: nA - - ode(sv1, time) = 1 :mV_per_ms; - x = 1000 * x_converted: pA; - y = 1{dimensionless}/x; - x_converted = 0.001 * 1 : nA + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var{current} x: pA; + var y: per_pA; + + ode(sv1, time) = 1{mV_per_ms}; + x = 1{pA}; + y = 1{dimensionless}/x; + + convert current from pA to nA + + becomes + var time: ms; + var{sv11} sv1: mV {init: 2}; + var{current} x_converted: nA; + var x : pA + var y: per_pA; + + ode(sv1, time) = 1{mV_per_ms}; + x = 1{pA} + y = 1{dimensionless}/x; + x_converted = 0.001 * x """ # original state def test_original_state(model): @@ -589,6 +605,9 @@ def test_original_state(model): assert str(model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' assert str(model.equations[1]) == 'Eq(_env_ode$x, _1.0)' assert str(model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + state_symbols = model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols return True pA_unit = literals_model.get_units('pA') @@ -620,10 +639,144 @@ def test_original_state(model): assert symbol_x_orig.units == 'pA' assert len(literals_model.equations) == 4 assert str(literals_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(literals_model.equations[1]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' - assert str(literals_model.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' - assert str(literals_model.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' + assert str(literals_model.equations[1]) == 'Eq(_env_ode$x, _1.0)' + assert str(literals_model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + assert str(literals_model.equations[3]) == 'Eq(_env_ode$x_converted, 0.001*_env_ode$x)' + state_symbols = literals_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + + def test_add_output_state_variable(self, local_model): + """ Tests the Model.add_input function that changes units. + This particular test is when a state variable is being converted as an output + + For example:: + + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + + ode(sv1, time) = 1{mV_per_ms}; + + convert sv11 from mV to V + + creates model + var{time} time: ms {pub: in}; + var{sv11} sv1_converted: V {init: 0.002}; + var sv1 mV {init: 2} + + ode(sv1, time) = 1{mV_per_ms}; + sv1_converted = sv1 / 1000 + """ + # original state + def test_original_state(local_model): + assert len(local_model.variables()) == 3 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert local_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert len(local_model.equations) == 1 + assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + state_symbols = local_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + return True + mV_unit = local_model.get_units('mV') + volt_unit = local_model.get_units('volt') + + assert test_original_state(local_model) + # test no change in units + local_model.add_output('env_ode$sv1', mV_unit) + assert test_original_state(local_model) + + # change mV to V + local_model.add_output('env_ode$sv1', volt_unit) + assert len(local_model.variables()) == 4 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + assert local_model.get_initial_value(symbol_a) == 0.002 + assert symbol_a.units == 'volt' + assert symbol_a.name == 'env_ode$sv1_converted' + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert symbol_t.units == 'ms' + symbol_orig = local_model.get_symbol_by_name('env_ode$sv1') + assert symbol_orig.units == 'mV' + assert local_model.get_initial_value(symbol_orig) == 2.0 + assert len(local_model.equations) == 2 + assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(local_model.equations[1]) == 'Eq(_env_ode$sv1_converted, 0.001*_env_ode$sv1)' + state_symbols = local_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_orig in state_symbols + + # def test_add_output_free_variable(self, local_model): + # """ Tests the Model.add_input function that changes units. + # This particular test is when a free variable is being converted as an output + # + # For example:: + # + # Original model + # var{time} time: ms {pub: in}; + # var{sv11} sv1: mV {init: 2}; + # + # ode(sv1, time) = 1{mV_per_ms}; + # + # convert time from ms to s + # + # creates model + # var{time} time: ms {pub: in}; + # var{sv11} sv1: mV {init: 2}; + # var{time} time_converted: s + # + # ode(sv1, time) = 1{mV_per_ms}; + # time_converted = 0.001 * time + # """ + # + # # original state + # def test_original_state(local_model): + # assert len(local_model.variables()) == 3 + # symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + # symbol_t = local_model.get_symbol_by_cmeta_id('time') + # assert local_model.get_initial_value(symbol_a) == 2.0 + # assert symbol_a.units == 'mV' + # assert symbol_t.units == 'ms' + # assert len(local_model.equations) == 1 + # assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + # state_symbols = local_model.get_state_symbols() + # assert len(state_symbols) == 1 + # assert symbol_a in state_symbols + # assert local_model.get_free_variable_symbol() == symbol_t + # return True + # + # ms_unit = local_model.get_units('ms') + # second_unit = local_model.get_units('second') + # + # assert test_original_state(local_model) + # # test no change in units + # local_model.add_output('_environment$time', ms_unit) + # assert test_original_state(local_model) + # + # # change ms to s + # local_model.add_output('_environment$time', second_unit) + # assert len(local_model.variables()) == 3 + # symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + # assert local_model.get_initial_value(symbol_a) == 0.002 + # assert symbol_a.units == 'mV' + # assert symbol_a.name == 'env_ode$sv1_converted' + # symbol_t = local_model.get_symbol_by_cmeta_id('time') + # assert symbol_t.units == 'second' + # assert symbol_t.name == 'environment$time_converted' + # symbol_orig = local_model.get_symbol_by_name('env_ode$time') + # assert symbol_orig.units == 'ms' + # assert len(local_model.equations) == 2 + # assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + # assert str(local_model.equations[1]) == 'Eq(_environment$time_converted, 0.001*_environment$time)' + # state_symbols = local_model.get_state_symbols() + # assert len(state_symbols) == 1 + # assert symbol_a in state_symbols + # assert local_model.get_free_variable_symbol() == symbol_t + # # def test_missing_units(self, model_missing_units, literals_model): # """ Tests the Model.add_output function that changes units. # In this case the model needs to add the unit it wants to change to. From c200c42ff8f2d09dd36112179bcc2e2bc21f3c3c Mon Sep 17 00:00:00 2001 From: Sarah Date: Thu, 19 Dec 2019 17:47:41 +0000 Subject: [PATCH 24/47] add test for free variable as output --- cellmlmanip/model.py | 9 +-- tests/test_unit_conversion.py | 138 +++++++++++++++++----------------- 2 files changed, 70 insertions(+), 77 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 5323671b..a1343483 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -823,14 +823,7 @@ def _convert_variable_instance(self, original_variable, cf, units, is_output): self.remove_equation(equation) original_had_eqn = True break - if is_output: - print(new_variable.name + ' output') - else: - print(new_variable.name + ' not output') - if original_had_eqn: - print(new_variable.name + ' eqn') - else: - print(new_variable.name + ' not eqn') + # 5. add an equation for original variable if there wasnt one # oldvar [old units] = newvar [new units] / cf [new units/old units] if not is_output: diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 90a48407..a93361b9 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -241,7 +241,7 @@ def test_original_state(local_model): assert test_original_state(local_model) # test no change in units - local_model.add_input('env_ode$time', ms_unit) + local_model.add_input('environment$time', ms_unit) assert test_original_state(local_model) # change ms to s @@ -395,7 +395,7 @@ def test_original_state(multiode_freevar_model): assert test_original_state(multiode_freevar_model) # test no change in units - multiode_freevar_model.add_input('env_ode$time', ms_unit) + multiode_freevar_model.add_input('environment$time', ms_unit) assert test_original_state(multiode_freevar_model) # change ms to s @@ -710,73 +710,73 @@ def test_original_state(local_model): assert len(state_symbols) == 1 assert symbol_orig in state_symbols - # def test_add_output_free_variable(self, local_model): - # """ Tests the Model.add_input function that changes units. - # This particular test is when a free variable is being converted as an output - # - # For example:: - # - # Original model - # var{time} time: ms {pub: in}; - # var{sv11} sv1: mV {init: 2}; - # - # ode(sv1, time) = 1{mV_per_ms}; - # - # convert time from ms to s - # - # creates model - # var{time} time: ms {pub: in}; - # var{sv11} sv1: mV {init: 2}; - # var{time} time_converted: s - # - # ode(sv1, time) = 1{mV_per_ms}; - # time_converted = 0.001 * time - # """ - # - # # original state - # def test_original_state(local_model): - # assert len(local_model.variables()) == 3 - # symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - # symbol_t = local_model.get_symbol_by_cmeta_id('time') - # assert local_model.get_initial_value(symbol_a) == 2.0 - # assert symbol_a.units == 'mV' - # assert symbol_t.units == 'ms' - # assert len(local_model.equations) == 1 - # assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - # state_symbols = local_model.get_state_symbols() - # assert len(state_symbols) == 1 - # assert symbol_a in state_symbols - # assert local_model.get_free_variable_symbol() == symbol_t - # return True - # - # ms_unit = local_model.get_units('ms') - # second_unit = local_model.get_units('second') - # - # assert test_original_state(local_model) - # # test no change in units - # local_model.add_output('_environment$time', ms_unit) - # assert test_original_state(local_model) - # - # # change ms to s - # local_model.add_output('_environment$time', second_unit) - # assert len(local_model.variables()) == 3 - # symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - # assert local_model.get_initial_value(symbol_a) == 0.002 - # assert symbol_a.units == 'mV' - # assert symbol_a.name == 'env_ode$sv1_converted' - # symbol_t = local_model.get_symbol_by_cmeta_id('time') - # assert symbol_t.units == 'second' - # assert symbol_t.name == 'environment$time_converted' - # symbol_orig = local_model.get_symbol_by_name('env_ode$time') - # assert symbol_orig.units == 'ms' - # assert len(local_model.equations) == 2 - # assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - # assert str(local_model.equations[1]) == 'Eq(_environment$time_converted, 0.001*_environment$time)' - # state_symbols = local_model.get_state_symbols() - # assert len(state_symbols) == 1 - # assert symbol_a in state_symbols - # assert local_model.get_free_variable_symbol() == symbol_t - # + def test_add_output_free_variable(self, local_model): + """ Tests the Model.add_input function that changes units. + This particular test is when a free variable is being converted as an output + + For example:: + + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + + ode(sv1, time) = 1{mV_per_ms}; + + convert time from ms to s + + creates model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var{time} time_converted: s + + ode(sv1, time) = 1{mV_per_ms}; + time_converted = 0.001 * time + """ + + # original state + def test_original_state(local_model): + assert len(local_model.variables()) == 3 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert local_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert len(local_model.equations) == 1 + assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + state_symbols = local_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + assert local_model.get_free_variable_symbol() == symbol_t + return True + + ms_unit = local_model.get_units('ms') + second_unit = local_model.get_units('second') + + assert test_original_state(local_model) + # test no change in units + local_model.add_output('environment$time', ms_unit) + assert test_original_state(local_model) + + # change ms to s + local_model.add_output('environment$time', second_unit) + assert len(local_model.variables()) == 4 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + assert local_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_a.name == 'env_ode$sv1' + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert symbol_t.units == 'second' + assert symbol_t.name == 'environment$time_converted' + symbol_orig = local_model.get_symbol_by_name('environment$time') + assert symbol_orig.units == 'ms' + assert len(local_model.equations) == 2 + assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(local_model.equations[1]) == 'Eq(_environment$time_converted, 0.001*_environment$time)' + state_symbols = local_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + assert local_model.get_free_variable_symbol() == symbol_orig + # def test_missing_units(self, model_missing_units, literals_model): # """ Tests the Model.add_output function that changes units. # In this case the model needs to add the unit it wants to change to. From f6265ef4f3822f93b3e03a371758b371f64a1d5c Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 28 Dec 2019 11:43:14 +0000 Subject: [PATCH 25/47] remove unused test file --- .../new_basic_ode_for_conversion_tests.cellml | 84 ------------------- 1 file changed, 84 deletions(-) delete mode 100644 tests/cellml_files/new_basic_ode_for_conversion_tests.cellml diff --git a/tests/cellml_files/new_basic_ode_for_conversion_tests.cellml b/tests/cellml_files/new_basic_ode_for_conversion_tests.cellml deleted file mode 100644 index 9c3aa30a..00000000 --- a/tests/cellml_files/new_basic_ode_for_conversion_tests.cellml +++ /dev/null @@ -1,84 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - time - - sv1 - - 1 - - - - x - 1 - - - - y - - - 1 - x - - - - - - - - - - - - - - - - - - - - - - - - From 41148fed6113364bfb46b2ac19db4228557824fe Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 28 Dec 2019 11:44:02 +0000 Subject: [PATCH 26/47] refactor add_input/add_output into convert_variable(variable, unit, direction) --- cellmlmanip/model.py | 104 +++++++--------- tests/test_unit_conversion.py | 224 ++++++++++++++++++++-------------- 2 files changed, 177 insertions(+), 151 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index a1343483..9f8ff715 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -1,5 +1,6 @@ """Classes to represent a flattened CellML model and metadata about its variables.""" import logging +from enum import Enum from io import StringIO import networkx as nx @@ -9,14 +10,18 @@ from cellmlmanip.rdf import create_rdf_node from cellmlmanip.units import UnitStore - logger = logging.getLogger(__name__) - # Delimiter for variables name in Sympy expressions: SYMPY_SYMBOL_DELIMITER = '$' +class DataDirectionFlow(Enum): + """ Direction of data flow for converting units""" + INPUT = 1 + OUTPUT = 2 + + class Model(object): """ A componentless representation of a CellML model, containing a list of equations, units, and RDF metadata about @@ -34,6 +39,7 @@ class Model(object): :param name: the name of the model e.g. from ````. :param cmeta_id: An optional cmeta id, e.g. from ````. """ + def __init__(self, name, cmeta_id=None): self.name = name @@ -619,44 +625,20 @@ def variables(self): """ Returns an iterator over this model's variable symbols. """ return self._name_to_symbol.values() - def add_input(self, name, units, is_output=False): - """ Adds an additional input for the variable name to the model with new units - and apply any necessary conversions/add additional equations. - - For example: - Original model - x = 1 [pA] - in [pA] - oxmeta: current - y = 1 / x - in [1/pA] - - add_input('x', 'nA') creates model - input_current = 1e-3 [nA] - in [nA] - oxmeta: current - x = input_current * 1e3 [pA/nA] - in [pA] - y = 1 / x - in [1/pA] - - :param name: name of variable whose units are to be changed - :param units: A `pint` units representation - :param is_output: the variable is an output rather than an input + def convert_variable(self, variable, units, direction): """ - # check units is of type Unit - assert isinstance(units, self.units.ureg.Unit) - - # check variable is in model - original_variable = self.get_symbol_by_name(name) - if not original_variable: - raise KeyError('No variable with name "%s" found.' % name) - original_units = original_variable.units + :param variable: + :param units: + :param direction: + :return: + """ + self._check_arguments(variable, units, direction) + original_units = variable.units # no conversion necessary if original_units == units: - return + return variable state_symbols = self.get_state_symbols() free_symbol = self.get_free_variable_symbol() @@ -664,24 +646,24 @@ def add_input(self, name, units, is_output=False): cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) # create new variable and relevant equations - new_variable = self._convert_variable_instance(original_variable, cf, units, is_output) + new_variable = self._convert_variable_instance(variable, cf, units, direction) # if is output do not need to do additional changes for state/free symbols - if is_output: - return + if direction == DataDirectionFlow.OUTPUT: + return new_variable new_derivatives = [] # if state variable - if original_variable in state_symbols: - new_derivatives.append(self._convert_state_variable_deriv(original_variable, new_variable, cf)) + if variable in state_symbols: + new_derivatives.append(self._convert_state_variable_deriv(variable, new_variable, cf)) # if free variable - if original_variable == free_symbol: + if variable == free_symbol: # for each derivative wrt to free variable add necessary variables/equations current_equations = self.equations.copy() for equation in current_equations: if equation.args[0].is_Derivative: - if equation.args[0].args[1].args[0] == original_variable: + if equation.args[0].args[1].args[0] == variable: new_derivatives.append(self._convert_free_variable_deriv(equation, new_variable, cf)) # replace any instances of derivative of rhs of other eqns with new derivative variable @@ -690,13 +672,25 @@ def add_input(self, name, units, is_output=False): self._invalidate_cache() - def add_output(self, name, units): - """ Adds an additional output for the variable name to the model with new units - and apply any necessary conversions/add additional equations. - :param name: name of variable whose units are to be changed - :param units: A `pint` units representation + return new_variable + + def _check_arguments(self, variable, units, direction): + """ + + :param variable: + :param units: + :param direction: + :return: """ - self.add_input(name, units, is_output=True) + # variable should be a VariableDummy present in the model + # or an ontology term that references a variable in the model + assert isinstance(variable, VariableDummy) + + # units should be a pint Unit object + assert isinstance(units, self.units.ureg.Unit) + + # direction should be part of enum + assert isinstance(direction, DataDirectionFlow) def _replace_derivatives(self, new_derivative): """ @@ -783,7 +777,7 @@ def _convert_state_variable_deriv(self, original_variable, new_variable, cf): self.add_equation(expression) return {'variable': new_deriv_variable, 'expression': eqn.args[0]} - def _convert_variable_instance(self, original_variable, cf, units, is_output): + def _convert_variable_instance(self, original_variable, cf, units, direction): """ Internal function to create new variable and an equation for it. :param original_variable: VariableDummy object to be converted [old units] @@ -814,7 +808,7 @@ def _convert_variable_instance(self, original_variable, cf, units, is_output): original_had_eqn = False for equation in self.equations: if equation.is_Equality and equation.args[0] == original_variable: - if is_output: + if direction == DataDirectionFlow.OUTPUT: expression = sympy.Eq(new_variable, original_variable * cf) self.add_equation(expression) else: @@ -826,18 +820,13 @@ def _convert_variable_instance(self, original_variable, cf, units, is_output): # 5. add an equation for original variable if there wasnt one # oldvar [old units] = newvar [new units] / cf [new units/old units] - if not is_output: - # if not original_had_eqn: + if direction == DataDirectionFlow.INPUT: expression = sympy.Eq(original_variable, new_variable / cf) self.add_equation(expression) else: if not original_had_eqn: expression = sympy.Eq(new_variable, original_variable * cf) self.add_equation(expression) - # else: - # expression = sympy.Eq(original_variable, new_variable / cf) - # self.add_equation(expression) - return new_variable @@ -850,6 +839,7 @@ class NumberDummy(sympy.Dummy): Number dummies should never be created directly, but always via :meth:`Model.add_number()`. """ + # Sympy annoyingly overwrites __new__ def __new__(cls, value, *args, **kwargs): return super().__new__(cls, str(value)) @@ -873,6 +863,7 @@ class VariableDummy(sympy.Dummy): For the constructor arguments, see :meth:`Model.add_variable()`. """ + # Sympy annoyingly overwrites __new__ def __new__(cls, name, *args, **kwargs): return super().__new__(cls, name) @@ -885,7 +876,6 @@ def __init__(self, private_interface=None, order_added=None, cmeta_id=None): - self.name = name self.units = units self.initial_value = None if initial_value is None else float(initial_value) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index a93361b9..3e0a54b0 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -1,6 +1,7 @@ import pytest from cellmlmanip import units +from cellmlmanip.model import DataDirectionFlow from . import shared @@ -123,12 +124,6 @@ def test_bad_units(self, bad_units_model): # cellml file states b (per_ms) = power(a (ms), 1 (second)) bad_units_model.units.summarise_units(equation[1].rhs) - def test_add_input_invalid_args(self, local_model): - mV_unit = local_model.get_units('mV') - # name does not exist in model - with pytest.raises(KeyError): - local_model.add_input('nonsense_name', mV_unit) - def test_add_input_state_variable(self, local_model): """ Tests the Model.add_input function that changes units. This particular test is when a state variable is being converted @@ -170,14 +165,17 @@ def test_original_state(local_model): mV_unit = local_model.get_units('mV') volt_unit = local_model.get_units('volt') + original_var = local_model.get_symbol_by_name('env_ode$sv1') assert test_original_state(local_model) # test no change in units - local_model.add_input('env_ode$sv1', mV_unit) + newvar = local_model.convert_variable(original_var, mV_unit, DataDirectionFlow.INPUT) + assert newvar == original_var assert test_original_state(local_model) # change mV to V - local_model.add_input('env_ode$sv1', volt_unit) + newvar = local_model.convert_variable(original_var, volt_unit, DataDirectionFlow.INPUT) + assert newvar != original_var assert len(local_model.variables()) == 5 symbol_a = local_model.get_symbol_by_cmeta_id('sv11') assert local_model.get_initial_value(symbol_a) == 0.002 @@ -202,7 +200,7 @@ def test_original_state(local_model): assert symbol_a in state_symbols def test_add_input_free_variable(self, local_model): - """ Tests the Model.add_input function that changes units. + """ Tests the Model.convert_variable function that changes units of given variable. This particular case tests changing a free variable For example:: @@ -238,14 +236,15 @@ def test_original_state(local_model): ms_unit = local_model.get_units('ms') second_unit = local_model.get_units('second') + original_var = local_model.get_symbol_by_name('environment$time') assert test_original_state(local_model) # test no change in units - local_model.add_input('environment$time', ms_unit) + local_model.convert_variable(original_var, ms_unit, DataDirectionFlow.INPUT) assert test_original_state(local_model) # change ms to s - local_model.add_input('environment$time', second_unit) + local_model.convert_variable(original_var, second_unit, DataDirectionFlow.INPUT) assert len(local_model.variables()) == 5 symbol_a = local_model.get_symbol_by_cmeta_id('sv11') assert local_model.get_initial_value(symbol_a) == 2.0 @@ -265,7 +264,7 @@ def test_original_state(local_model): '1000.0*_env_ode$sv1_orig_deriv)' def test_add_input_literal_variable(self, literals_model): - """ Tests the Model.add_input function that changes units. + """ Tests the Model.convert_variable function that changes units of given variable. This particular case tests changing a literal variable/constant For example:: @@ -317,14 +316,15 @@ def test_original_state(model): pA_unit = literals_model.get_units('pA') nA_unit = literals_model.get_units('nA') + original_var = literals_model.get_symbol_by_name('env_ode$x') assert test_original_state(literals_model) # test no change in units - literals_model.add_input('env_ode$x', pA_unit) + literals_model.convert_variable(original_var, pA_unit, DataDirectionFlow.INPUT) assert test_original_state(literals_model) # change pA to nA - literals_model.add_input('env_ode$x', nA_unit) + literals_model.convert_variable(original_var, nA_unit, DataDirectionFlow.INPUT) assert len(literals_model.variables()) == 6 symbol_a = literals_model.get_symbol_by_cmeta_id('sv11') symbol_t = literals_model.get_symbol_by_cmeta_id('time') @@ -349,30 +349,33 @@ def test_original_state(model): assert str(literals_model.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' def test_add_input_free_variable_multiple(self, multiode_freevar_model): - """ Tests the Model.add_input function that changes units. - This particular case tests changing a free variable - e.g. - var{time} time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - var y: mV {init: 3}; - ode(sv1, time) = 1{mV_per_ms}; - ode(y, time) = 2{mV_per_ms} + """ Tests the Model.convert_variable function that changes units of given variable. + This particular case tests changing a free variable where there are multiple ode instances. + For example:: + + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var y: mV {init: 3}; + + ode(sv1, time) = 1{mV_per_ms}; + ode(y, time) = 2{mV_per_ms} convert time from ms to s - becomes - var time: ms; - var{time} time_converted: s; - var{sv11} sv1: mV {init: 2}; - var sv1_orig_deriv mV_per_ms - var y : mV - var y_orig_deriv mV_per_ms - - time = 1000 * time_converted; - sv1_orig_deriv = 1{mV_per_ms} - ode(sv1, time_converted) = 1000 * sv1_orig_deriv - y_orig_deriv = 2{mV_per_ms} - ode(y, time_converted) = 1000 * y_orig_deriv + becomes + var time: ms; + var{time} time_converted: s; + var{sv11} sv1: mV {init: 2}; + var sv1_orig_deriv mV_per_ms + var y : mV + var y_orig_deriv mV_per_ms + + time = 1000 * time_converted; + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1, time_converted) = 1000 * sv1_orig_deriv + y_orig_deriv = 2{mV_per_ms} + ode(y, time_converted) = 1000 * y_orig_deriv """ # original state def test_original_state(multiode_freevar_model): @@ -392,14 +395,15 @@ def test_original_state(multiode_freevar_model): ms_unit = multiode_freevar_model.get_units('ms') second_unit = multiode_freevar_model.get_units('second') + original_var = multiode_freevar_model.get_symbol_by_name('environment$time') assert test_original_state(multiode_freevar_model) # test no change in units - multiode_freevar_model.add_input('environment$time', ms_unit) + multiode_freevar_model.convert_variable(original_var, ms_unit, DataDirectionFlow.INPUT) assert test_original_state(multiode_freevar_model) # change ms to s - multiode_freevar_model.add_input('environment$time', second_unit) + multiode_freevar_model.convert_variable(original_var, second_unit, DataDirectionFlow.INPUT) assert len(multiode_freevar_model.variables()) == 7 symbol_a = multiode_freevar_model.get_symbol_by_cmeta_id('sv11') assert multiode_freevar_model.get_initial_value(symbol_a) == 2.0 @@ -427,35 +431,37 @@ def test_original_state(multiode_freevar_model): '1000.0*_env_ode$y_orig_deriv)' def test_multiple_odes(self, multiode_model): - """ Tests the Model.add_input function that changes units. + """ Tests the Model.convert_variable function that changes units of given variable. This tests that any other uses of the derivative on the rhs of other equations are replaced with the new variable representing the old derivative - e.g. - var time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - var x: mV_per_ms; - var y: mV {init: 3}; + For example:: - ode(sv1, time) = 1{mV_per_ms}; - x = ode(sv1, time) * 3{mV_per_ms}; - ode(y, time) = 2{mV_per_ms}; + Original model + var time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var x: mV_per_ms; + var y: mV {init: 3}; + + ode(sv1, time) = 1{mV_per_ms}; + x = ode(sv1, time) * 3{mV_per_ms}; + ode(y, time) = 2{mV_per_ms}; change sv1 from mV to V becomes - var{time} time: ms {pub: in}; - var{sv11} sv1_converted: V {init: 0.002}; - var sv1 mV {init: 2} - var sv1_orig_deriv mV_per_ms - var x: mV_per_ms; - var y: mV {init: 3}; - var y_orig_deriv - - ode(y, time) = 2{mv_per_ms}; - sv1 = 100 * sv1_converted - sv1_orig_deriv = 1{mV_per_ms} - ode(sv1_converted, time) = 0.001 * sv1_orig_deriv - x = 3 * sv1_orig_deriv + var{time} time: ms {pub: in}; + var{sv11} sv1_converted: V {init: 0.002}; + var sv1 mV {init: 2} + var sv1_orig_deriv mV_per_ms + var x: mV_per_ms; + var y: mV {init: 3}; + var y_orig_deriv + + ode(y, time) = 2{mv_per_ms}; + sv1 = 1000 * sv1_converted + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1_converted, time) = 0.001 * sv1_orig_deriv + x = 3 * sv1_orig_deriv """ # original state def test_original_state(multiode_model): @@ -474,14 +480,15 @@ def test_original_state(multiode_model): mV_unit = multiode_model.get_units('mV') volt_unit = multiode_model.get_units('volt') + original_var = multiode_model.get_symbol_by_name('env_ode$sv1') assert test_original_state(multiode_model) # test no change in units - multiode_model.add_input('env_ode$sv1', mV_unit) + multiode_model.convert_variable(original_var, mV_unit, DataDirectionFlow.INPUT) assert test_original_state(multiode_model) # change mV to V - multiode_model.add_input('env_ode$sv1', volt_unit) + multiode_model.convert_variable(original_var, volt_unit, DataDirectionFlow.INPUT) assert len(multiode_model.equations) == 5 assert str(multiode_model.equations[0]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' assert str(multiode_model.equations[1]) == 'Eq(_env_ode$sv1, 1000.0*_env_ode$sv1_converted)' @@ -491,35 +498,39 @@ def test_original_state(multiode_model): assert str(multiode_model.equations[4]) == 'Eq(_env_ode$x, _3.0*_env_ode$sv1_orig_deriv)' def test_multiple_odes_1(self, multiode_model): - """ Tests the Model.add_input function that changes units. + """ Tests the Model.convert_variable function that changes units of given variable. This tests that any other uses of the derivative on the rhs of other equations are replaced with the new variable representing the old derivative - e.g. - var time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - var x: mV_per_ms; - var y: mV {init: 3}; + For example:: + + Original model + var time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var x: mV_per_ms; + var y: mV {init: 3}; + + ode(sv1, time) = 1{mV_per_ms}; + x = ode(sv1, time) * 3{mV_per_ms}; + ode(y, time) = 2{mV_per_ms}; - ode(sv1, time) = 1{mV_per_ms}; - x = ode(sv1, time) * 3{mV_per_ms}; - ode(y, time) = 2{mV_per_ms}; change time from ms to s - becomes - var time: ms; - var{time} time_converted: s; - var{sv11} sv1: mV {init: 2}; - var sv1_orig_deriv mV_per_ms - var x: mV_per_ms; - var y: mV {init: 3}; - var y_orig_deriv: {mv_per_ms} - - ode(y, time) = 2{mv_per_ms}; - sv1 = 100 * sv1_converted - sv1_orig_deriv = 1{mV_per_ms} - ode(sv1_converted, time) = 0.001 * sv1_orig_deriv - x = 3 * sv1_orig_deriv + becomes + var time: ms; + var{time} time_converted: s; + var{sv11} sv1: mV {init: 2}; + var sv1_orig_deriv mV_per_ms + var x: mV_per_ms; + var y: mV {init: 3}; + var y_orig_deriv: {mv_per_ms} + + time_converted = 1000 * time + y_orig_deriv = 2{mV_per_ms} + ode(y, time_converted) = 1000 * y_orig_deriv; + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1, time_converted) = 1000 * sv1_orig_deriv + x = 3 * sv1_orig_deriv """ # original state def test_original_state(multiode_model): @@ -538,14 +549,15 @@ def test_original_state(multiode_model): ms_unit = multiode_model.get_units('ms') second_unit = multiode_model.get_units('second') + original_var = multiode_model.get_symbol_by_name('environment$time') assert test_original_state(multiode_model) # test no change in units - multiode_model.add_input('env_ode$time', ms_unit) + multiode_model.convert_variable(original_var, ms_unit, DataDirectionFlow.INPUT) assert test_original_state(multiode_model) # change ms to s - multiode_model.add_input('environment$time', second_unit) + multiode_model.convert_variable(original_var, second_unit, DataDirectionFlow.INPUT) assert len(multiode_model.equations) == 6 assert str(multiode_model.equations[0]) == 'Eq(_environment$time, 1000.0*_environment$time_converted)' assert str(multiode_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' @@ -612,14 +624,15 @@ def test_original_state(model): pA_unit = literals_model.get_units('pA') nA_unit = literals_model.get_units('nA') + original_var = literals_model.get_symbol_by_name('env_ode$x') assert test_original_state(literals_model) # test no change in units - literals_model.add_output('env_ode$x', pA_unit) + literals_model.convert_variable(original_var, pA_unit, DataDirectionFlow.OUTPUT) assert test_original_state(literals_model) # change pA to nA - literals_model.add_output('env_ode$x', nA_unit) + literals_model.convert_variable(original_var, nA_unit, DataDirectionFlow.OUTPUT) assert len(literals_model.variables()) == 6 symbol_a = literals_model.get_symbol_by_cmeta_id('sv11') symbol_t = literals_model.get_symbol_by_cmeta_id('time') @@ -647,7 +660,7 @@ def test_original_state(model): assert symbol_a in state_symbols def test_add_output_state_variable(self, local_model): - """ Tests the Model.add_input function that changes units. + """ Tests the Model.convert_variable function that changes units of given variable. This particular test is when a state variable is being converted as an output For example:: @@ -685,14 +698,15 @@ def test_original_state(local_model): mV_unit = local_model.get_units('mV') volt_unit = local_model.get_units('volt') + original_var = local_model.get_symbol_by_name('env_ode$sv1') assert test_original_state(local_model) # test no change in units - local_model.add_output('env_ode$sv1', mV_unit) + local_model.convert_variable(original_var, mV_unit, DataDirectionFlow.OUTPUT) assert test_original_state(local_model) # change mV to V - local_model.add_output('env_ode$sv1', volt_unit) + local_model.convert_variable(original_var, volt_unit, DataDirectionFlow.OUTPUT) assert len(local_model.variables()) == 4 symbol_a = local_model.get_symbol_by_cmeta_id('sv11') assert local_model.get_initial_value(symbol_a) == 0.002 @@ -711,7 +725,7 @@ def test_original_state(local_model): assert symbol_orig in state_symbols def test_add_output_free_variable(self, local_model): - """ Tests the Model.add_input function that changes units. + """ Tests the Model.convert_variable function that changes units of given variable. This particular test is when a free variable is being converted as an output For example:: @@ -751,14 +765,15 @@ def test_original_state(local_model): ms_unit = local_model.get_units('ms') second_unit = local_model.get_units('second') + original_var = local_model.get_symbol_by_name('environment$time') assert test_original_state(local_model) # test no change in units - local_model.add_output('environment$time', ms_unit) + local_model.convert_variable(original_var, ms_unit, DataDirectionFlow.OUTPUT) assert test_original_state(local_model) # change ms to s - local_model.add_output('environment$time', second_unit) + local_model.convert_variable(original_var, second_unit, DataDirectionFlow.OUTPUT) assert len(local_model.variables()) == 4 symbol_a = local_model.get_symbol_by_cmeta_id('sv11') assert local_model.get_initial_value(symbol_a) == 2.0 @@ -777,6 +792,27 @@ def test_original_state(local_model): assert symbol_a in state_symbols assert local_model.get_free_variable_symbol() == symbol_orig + def test_convert_variable_invalid_arguments(self, local_model): + unit = local_model.get_units('second') + variable = local_model.get_free_variable_symbol() + direction = DataDirectionFlow.INPUT + + with pytest.raises(AssertionError): + local_model.convert_variable('x', unit, direction) + + with pytest.raises(AssertionError): + local_model.convert_variable(variable, 'x', direction) + + with pytest.raises(AssertionError): + local_model.convert_variable(variable, unit, 'x') + + def test_noconversion_necessary(self, local_model): + unit = local_model.get_units('ms') + variable = local_model.get_free_variable_symbol() + direction = DataDirectionFlow.INPUT + + assert local_model.convert_variable(variable, unit, direction) == variable + # def test_missing_units(self, model_missing_units, literals_model): # """ Tests the Model.add_output function that changes units. # In this case the model needs to add the unit it wants to change to. From 424d65b1d05cfc2ab0fccaff89eb5ff54416e55d Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 28 Dec 2019 11:47:30 +0000 Subject: [PATCH 27/47] flake8 --- cellmlmanip/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 9f8ff715..ec03d0a6 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -649,7 +649,7 @@ def convert_variable(self, variable, units, direction): new_variable = self._convert_variable_instance(variable, cf, units, direction) # if is output do not need to do additional changes for state/free symbols - if direction == DataDirectionFlow.OUTPUT: + if direction == DataDirectionFlow.OUTPUT: return new_variable new_derivatives = [] From 07b7cbead82ca0dd0e2202905182c0a5719d03ec Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 28 Dec 2019 13:44:03 +0000 Subject: [PATCH 28/47] update docstrings --- cellmlmanip/model.py | 55 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index ec03d0a6..95c409c6 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -627,11 +627,50 @@ def variables(self): def convert_variable(self, variable, units, direction): """ + Changes the units of variable argument to the units supplied applying the direction of the + variable from the direction argument. - :param variable: - :param units: - :param direction: - :return: + For example:: + + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + + ode(sv1, time) = 1{mV_per_ms}; + + convert_variable(time, second, DataDirectionFlow.INPUT) + + becomes + var time: ms; + var{time} time_converted: s; + var{sv11} sv1: mV {init: 2}; + var sv1_orig_deriv mV_per_ms + + time = 1000 * time_converted; + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1, time_converted) = 1000 * sv1_orig_deriv + + + convert_variable(time, second, DataDirectionFlow.OUTPUT) + + creates model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var{time} time_converted: s + + ode(sv1, time) = 1{mV_per_ms}; + time_converted = 0.001 * time + + + :param variable: the variable to be converted + :param units: units to convert variable to (note if variable is already in these units, model remains + unchanged and the original variable is returned + :param direction: either DataDirectionFlow.INPUT; the variable to be changed is an input and all affected + equations will be adjusted + or DataDirectionFlow.OUTPUT; the variable to be changed is an output, equations + are unaffected apart from converting the actual output + :return: the original variable but with new units (or original unchanged if conversion was not necessary + or impossible """ self._check_arguments(variable, units, direction) original_units = variable.units @@ -676,8 +715,10 @@ def convert_variable(self, variable, units, direction): def _check_arguments(self, variable, units, direction): """ - - :param variable: + Checks the arguments of the convert_variable functions + :param variable: variable must be a VariableDummy object present in the model + or the string representation of an ontology term referring to + a VariableDummy object present in the model :param units: :param direction: :return: @@ -685,6 +726,8 @@ def _check_arguments(self, variable, units, direction): # variable should be a VariableDummy present in the model # or an ontology term that references a variable in the model assert isinstance(variable, VariableDummy) + assert variable.name in self._name_to_symbol + # units should be a pint Unit object assert isinstance(units, self.units.ureg.Unit) From 3100a219e6988ed4cf801961a2494fb41489929f Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 28 Dec 2019 14:08:40 +0000 Subject: [PATCH 29/47] tests input arguments are correct types --- cellmlmanip/model.py | 21 ++++++++++++++------- tests/test_unit_conversion.py | 13 ++++++++++++- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 95c409c6..ee5b4008 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -719,15 +719,22 @@ def _check_arguments(self, variable, units, direction): :param variable: variable must be a VariableDummy object present in the model or the string representation of an ontology term referring to a VariableDummy object present in the model - :param units: - :param direction: - :return: + :param units: units must be a pint Unit object + :param direction: must be part of DataDirectionFlow enumn + :throws: assertion error """ - # variable should be a VariableDummy present in the model + # variable should be a VariableDummy # or an ontology term that references a variable in the model - assert isinstance(variable, VariableDummy) - assert variable.name in self._name_to_symbol - + if isinstance(variable, str): + try: + var = self.get_symbol_by_cmeta_id(variable) + except KeyError: + raise AssertionError + name = var.name + else: + assert isinstance(variable, VariableDummy) + name = variable.name + assert name in self._name_to_symbol # units should be a pint Unit object assert isinstance(units, self.units.ureg.Unit) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 3e0a54b0..1d1111e3 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -797,8 +797,9 @@ def test_convert_variable_invalid_arguments(self, local_model): variable = local_model.get_free_variable_symbol() direction = DataDirectionFlow.INPUT + # arguments wrong types with pytest.raises(AssertionError): - local_model.convert_variable('x', unit, direction) + local_model.convert_variable('x', unit, direction) with pytest.raises(AssertionError): local_model.convert_variable(variable, 'x', direction) @@ -806,6 +807,16 @@ def test_convert_variable_invalid_arguments(self, local_model): with pytest.raises(AssertionError): local_model.convert_variable(variable, unit, 'x') + # variable not present in model + model = shared.load_model('literals_for_conversion_tests') + other_var = model.get_symbol_by_name('env_ode$x') + with pytest.raises(AssertionError): + local_model.convert_variable(other_var, unit, direction) + + # ontology term not present in model + with pytest.raises(AssertionError): + local_model.convert_variable('current', unit, direction) + def test_noconversion_necessary(self, local_model): unit = local_model.get_units('ms') variable = local_model.get_free_variable_symbol() From 05f9fb11b474d898b93540eee26eccf3d99f03b5 Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 28 Dec 2019 14:37:46 +0000 Subject: [PATCH 30/47] unit conversion not possible --- cellmlmanip/model.py | 10 +++++++--- tests/test_unit_conversion.py | 6 ++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index ee5b4008..f315b63c 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -671,6 +671,9 @@ def convert_variable(self, variable, units, direction): are unaffected apart from converting the actual output :return: the original variable but with new units (or original unchanged if conversion was not necessary or impossible + :throws: AssertionError if the arguments are of incorrect type + the variable does not exist in the model + DimensionalityError if the unit conversion is impossible """ self._check_arguments(variable, units, direction) original_units = variable.units @@ -679,11 +682,12 @@ def convert_variable(self, variable, units, direction): if original_units == units: return variable - state_symbols = self.get_state_symbols() - free_symbol = self.get_free_variable_symbol() - # conversion_factor for old units to new units + # conversion_factor for old units to new + # throws DimensionalityError if not possible cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) + state_symbols = self.get_state_symbols() + free_symbol = self.get_free_variable_symbol() # create new variable and relevant equations new_variable = self._convert_variable_instance(variable, cf, units, direction) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 1d1111e3..2d8b6067 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -1,4 +1,5 @@ import pytest +from pint import DimensionalityError from cellmlmanip import units from cellmlmanip.model import DataDirectionFlow @@ -796,6 +797,7 @@ def test_convert_variable_invalid_arguments(self, local_model): unit = local_model.get_units('second') variable = local_model.get_free_variable_symbol() direction = DataDirectionFlow.INPUT + bad_unit = local_model.get_units('mV') # arguments wrong types with pytest.raises(AssertionError): @@ -817,6 +819,10 @@ def test_convert_variable_invalid_arguments(self, local_model): with pytest.raises(AssertionError): local_model.convert_variable('current', unit, direction) + # unit conversion is impossible + with pytest.raises(DimensionalityError): + local_model.convert_variable(variable, bad_unit, direction) + def test_noconversion_necessary(self, local_model): unit = local_model.get_units('ms') variable = local_model.get_free_variable_symbol() From c327e21294b1ec166d1e699d2d6e0bd4a6c5b390 Mon Sep 17 00:00:00 2001 From: Sarah Date: Sat, 28 Dec 2019 15:03:33 +0000 Subject: [PATCH 31/47] finally fixed my isort issue --- cellmlmanip/model.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index f315b63c..16993782 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -10,6 +10,7 @@ from cellmlmanip.rdf import create_rdf_node from cellmlmanip.units import UnitStore + logger = logging.getLogger(__name__) # Delimiter for variables name in Sympy expressions: From ad764aabbd7007fdc762cfd4bd3d1720de2b83d4 Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 30 Dec 2019 12:37:57 +0000 Subject: [PATCH 32/47] use an ontology term to find the variable whose units need changing --- cellmlmanip/model.py | 24 ++- tests/test_unit_conversion.py | 334 +++++++++++++++++++++++++++++++++- 2 files changed, 341 insertions(+), 17 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 16993782..da7124ce 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -676,12 +676,12 @@ def convert_variable(self, variable, units, direction): the variable does not exist in the model DimensionalityError if the unit conversion is impossible """ - self._check_arguments(variable, units, direction) - original_units = variable.units + original_variable = self._check_arguments(variable, units, direction) + original_units = original_variable.units # no conversion necessary if original_units == units: - return variable + return original_variable # conversion_factor for old units to new # throws DimensionalityError if not possible @@ -690,7 +690,7 @@ def convert_variable(self, variable, units, direction): state_symbols = self.get_state_symbols() free_symbol = self.get_free_variable_symbol() # create new variable and relevant equations - new_variable = self._convert_variable_instance(variable, cf, units, direction) + new_variable = self._convert_variable_instance(original_variable, cf, units, direction) # if is output do not need to do additional changes for state/free symbols if direction == DataDirectionFlow.OUTPUT: @@ -698,16 +698,16 @@ def convert_variable(self, variable, units, direction): new_derivatives = [] # if state variable - if variable in state_symbols: - new_derivatives.append(self._convert_state_variable_deriv(variable, new_variable, cf)) + if original_variable in state_symbols: + new_derivatives.append(self._convert_state_variable_deriv(original_variable, new_variable, cf)) # if free variable - if variable == free_symbol: + if original_variable == free_symbol: # for each derivative wrt to free variable add necessary variables/equations current_equations = self.equations.copy() for equation in current_equations: if equation.args[0].is_Derivative: - if equation.args[0].args[1].args[0] == variable: + if equation.args[0].args[1].args[0] == original_variable: new_derivatives.append(self._convert_free_variable_deriv(equation, new_variable, cf)) # replace any instances of derivative of rhs of other eqns with new derivative variable @@ -725,20 +725,24 @@ def _check_arguments(self, variable, units, direction): or the string representation of an ontology term referring to a VariableDummy object present in the model :param units: units must be a pint Unit object - :param direction: must be part of DataDirectionFlow enumn + :param direction: must be part of DataDirectionFlow enum + :returns: VariableDummy object for variable :throws: assertion error """ # variable should be a VariableDummy # or an ontology term that references a variable in the model + returned_variable = None if isinstance(variable, str): try: var = self.get_symbol_by_cmeta_id(variable) except KeyError: raise AssertionError name = var.name + returned_variable = var else: assert isinstance(variable, VariableDummy) name = variable.name + returned_variable = variable assert name in self._name_to_symbol # units should be a pint Unit object @@ -747,6 +751,8 @@ def _check_arguments(self, variable, units, direction): # direction should be part of enum assert isinstance(direction, DataDirectionFlow) + return returned_variable + def _replace_derivatives(self, new_derivative): """ Function to replace an instance of a derivative that occurs on the RHS of any equation diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 2d8b6067..d87d8f52 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -137,7 +137,7 @@ def test_add_input_state_variable(self, local_model): ode(sv1, time) = 1{mV_per_ms}; - convert sv11 from mV to V + convert_variable(sv1, volt, DataDirectionFlow.INPUT) creates model var{time} time: ms {pub: in}; @@ -200,6 +200,81 @@ def test_original_state(local_model): assert len(state_symbols) == 1 assert symbol_a in state_symbols + def test_add_input_state_variable_ontology_term(self, local_model): + """ Tests the Model.add_input function that changes units. + This particular test is when a state variable is being converted using ontology term as argument + + For example:: + + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + + ode(sv1, time) = 1{mV_per_ms}; + + convert_variable('sv11', volt, DataDirectionFlow.INPUT) + + creates model + var{time} time: ms {pub: in}; + var{sv11} sv1_converted: V {init: 0.002}; + var sv1 mV {init: 2} + var sv1_orig_deriv mV_per_ms + + sv1 = 1000 * sv1_converted + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1_converted, time) = 0.001 * sv1_orig_deriv + """ + # original state + def test_original_state(local_model): + assert len(local_model.variables()) == 3 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert local_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert len(local_model.equations) == 1 + assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + state_symbols = local_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + return True + + mV_unit = local_model.get_units('mV') + volt_unit = local_model.get_units('volt') + original_var = local_model.get_symbol_by_name('env_ode$sv1') + + assert test_original_state(local_model) + # test no change in units + newvar = local_model.convert_variable('sv11', mV_unit, DataDirectionFlow.INPUT) + assert newvar == original_var + assert test_original_state(local_model) + + # change mV to V + newvar = local_model.convert_variable('sv11', volt_unit, DataDirectionFlow.INPUT) + assert newvar != original_var + assert len(local_model.variables()) == 5 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + assert local_model.get_initial_value(symbol_a) == 0.002 + assert symbol_a.units == 'volt' + assert symbol_a.name == 'env_ode$sv1_converted' + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert symbol_t.units == 'ms' + symbol_orig = local_model.get_symbol_by_name('env_ode$sv1') + assert symbol_orig.units == 'mV' + assert local_model.get_initial_value(symbol_orig) == 2.0 + symbol_derv = local_model.get_symbol_by_name('env_ode$sv1_orig_deriv') + assert symbol_derv.units == 'mV / ms' + assert not local_model.get_initial_value(symbol_derv) + assert len(local_model.equations) == 3 + assert str(local_model.equations[0]) == 'Eq(_env_ode$sv1, 1000.0*_env_ode$sv1_converted)' + assert str(local_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' + assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1_converted, _environment$time), ' \ + '0.001*_env_ode$sv1_orig_deriv)' + + state_symbols = local_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + def test_add_input_free_variable(self, local_model): """ Tests the Model.convert_variable function that changes units of given variable. This particular case tests changing a free variable @@ -211,7 +286,7 @@ def test_add_input_free_variable(self, local_model): ode(sv1, time) = 1{mV_per_ms}; - convert time from ms to s + convert_variable(time, second, DataDirectionFlow.INPUT) becomes var time: ms; @@ -264,6 +339,69 @@ def test_original_state(local_model): assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1, _environment$time_converted), ' \ '1000.0*_env_ode$sv1_orig_deriv)' + def test_add_input_free_variable_ontology_term(self, local_model): + """ Tests the Model.convert_variable function that changes units of given variable. + This particular case tests changing a free variable using the ontology term + For example:: + + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + + ode(sv1, time) = 1{mV_per_ms}; + + convert_variable('time', second, DataFlowDirection.INPUT) + + becomes + var time: ms; + var{time} time_converted: s; + var{sv11} sv1: mV {init: 2}; + var sv1_orig_deriv mV_per_ms + + time = 1000 * time_converted; + sv1_orig_deriv = 1{mV_per_ms} + ode(sv1, time_converted) = 1000 * sv1_orig_deriv + """ + # original state + def test_original_state(local_model): + assert len(local_model.variables()) == 3 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert local_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert len(local_model.equations) == 1 + assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + return True + + ms_unit = local_model.get_units('ms') + second_unit = local_model.get_units('second') + + assert test_original_state(local_model) + # test no change in units + local_model.convert_variable('time', ms_unit, DataDirectionFlow.INPUT) + assert test_original_state(local_model) + + # change ms to s + local_model.convert_variable('time', second_unit, DataDirectionFlow.INPUT) + assert len(local_model.variables()) == 5 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + assert local_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_a.name == 'env_ode$sv1' + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert symbol_t.units == 'second' + assert symbol_t.name == 'environment$time_converted' + symbol_orig = local_model.get_symbol_by_name('env_ode$sv1') + assert symbol_orig.units == 'mV' + symbol_derv = local_model.get_symbol_by_name('env_ode$sv1_orig_deriv') + assert symbol_derv.units == 'mV / ms' + assert len(local_model.equations) == 3 + assert str(local_model.equations[0]) == 'Eq(_environment$time, 1000.0*_environment$time_converted)' + assert str(local_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' + assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1, _environment$time_converted), ' \ + '1000.0*_env_ode$sv1_orig_deriv)' + def test_add_input_literal_variable(self, literals_model): """ Tests the Model.convert_variable function that changes units of given variable. This particular case tests changing a literal variable/constant @@ -279,7 +417,7 @@ def test_add_input_literal_variable(self, literals_model): x = 1{pA}; y = 1{dimensionless}/x; - convert current from pA to nA + convert_variable(x, nA, DataDirectionFlow.INPUT) becomes var time: ms; @@ -349,6 +487,90 @@ def test_original_state(model): assert str(literals_model.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' assert str(literals_model.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' + def test_add_input_literal_variable_ontology_term(self, literals_model): + """ Tests the Model.convert_variable function that changes units of given variable. + This particular case tests changing a literal variable/constant + For example:: + + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var{current} x: pA; + var y: per_pA; + + ode(sv1, time) = 1{mV_per_ms}; + x = 1{pA}; + y = 1{dimensionless}/x; + + convert_variable('current', nA, DataDirectionFlow.INPUT) + + becomes + var time: ms; + var{sv11} sv1: mV {init: 2}; + var{current} x_converted: nA; + var x : pA + var y: per_pA; + + ode(sv1, time) = 1{mV_per_ms}; + x_converted = 0.001 * 1{pA} + x = 1000* x_converted; + y = 1{dimensionless}/x; + """ + # original state + def test_original_state(model): + assert len(model.variables()) == 5 + symbol_a = model.get_symbol_by_cmeta_id('sv11') + symbol_t = model.get_symbol_by_cmeta_id('time') + symbol_x = model.get_symbol_by_cmeta_id('current') + symbol_y = model.get_symbol_by_name('env_ode$y') + assert symbol_x.name == 'env_ode$x' + assert model.get_initial_value(symbol_a) == 2.0 + assert not model.get_initial_value(symbol_t) + assert not model.get_initial_value(symbol_x) + assert not model.get_initial_value(symbol_y) + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_x.units == 'pA' + assert symbol_y.units == 'per_pA' + assert len(model.equations) == 3 + assert str(model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(model.equations[1]) == 'Eq(_env_ode$x, _1.0)' + assert str(model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + return True + + pA_unit = literals_model.get_units('pA') + nA_unit = literals_model.get_units('nA') + + assert test_original_state(literals_model) + # test no change in units + literals_model.convert_variable('current', pA_unit, DataDirectionFlow.INPUT) + assert test_original_state(literals_model) + + # change pA to nA + literals_model.convert_variable('current', nA_unit, DataDirectionFlow.INPUT) + assert len(literals_model.variables()) == 6 + symbol_a = literals_model.get_symbol_by_cmeta_id('sv11') + symbol_t = literals_model.get_symbol_by_cmeta_id('time') + symbol_x = literals_model.get_symbol_by_cmeta_id('current') + assert symbol_x.name == 'env_ode$x_converted' + symbol_y = literals_model.get_symbol_by_name('env_ode$y') + symbol_x_orig = literals_model.get_symbol_by_name('env_ode$x') + assert literals_model.get_initial_value(symbol_a) == 2.0 + assert not literals_model.get_initial_value(symbol_t) + assert not literals_model.get_initial_value(symbol_x) + assert not literals_model.get_initial_value(symbol_y) + assert not literals_model.get_initial_value(symbol_x_orig) + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_x.units == 'nA' + assert symbol_y.units == 'per_pA' + assert symbol_x_orig.units == 'pA' + assert len(literals_model.equations) == 4 + assert str(literals_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(literals_model.equations[1]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + assert str(literals_model.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' + assert str(literals_model.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' + def test_add_input_free_variable_multiple(self, multiode_freevar_model): """ Tests the Model.convert_variable function that changes units of given variable. This particular case tests changing a free variable where there are multiple ode instances. @@ -362,7 +584,7 @@ def test_add_input_free_variable_multiple(self, multiode_freevar_model): ode(sv1, time) = 1{mV_per_ms}; ode(y, time) = 2{mV_per_ms} - convert time from ms to s + convert_variable(time, second, DataDirectionFlow.INPUT) becomes var time: ms; @@ -447,7 +669,7 @@ def test_multiple_odes(self, multiode_model): x = ode(sv1, time) * 3{mV_per_ms}; ode(y, time) = 2{mV_per_ms}; - change sv1 from mV to V + convert_variable(sv1, volt, DataDirectionFlow.INPUT) becomes var{time} time: ms {pub: in}; @@ -515,7 +737,7 @@ def test_multiple_odes_1(self, multiode_model): ode(y, time) = 2{mV_per_ms}; - change time from ms to s + convert_variable(time, second, DataDirectionFlow.INPUT) becomes var time: ms; @@ -570,7 +792,8 @@ def test_original_state(multiode_model): assert str(multiode_model.equations[5]) == 'Eq(_env_ode$x, _3.0*_env_ode$sv1_orig_deriv)' def test_add_output(self, literals_model): - """ Tests the Model.add_output function that changes units. + """ Tests the Model.convert_variable function that changes units of given variable. + This tests the case when variable to be changed is an OUTPUT For example:: @@ -584,7 +807,7 @@ def test_add_output(self, literals_model): x = 1{pA}; y = 1{dimensionless}/x; - convert current from pA to nA + convert_variable(x, nA, DataDirectionFlow.OUTPUT) becomes var time: ms; @@ -660,6 +883,97 @@ def test_original_state(model): assert len(state_symbols) == 1 assert symbol_a in state_symbols + def test_add_output_onotology_term(self, literals_model): + """ Tests the Model.convert_variable function that changes units of given variable. + This tests the case when variable to be changed is an OUTPUT using an ontology term + + For example:: + + Original model + var{time} time: ms {pub: in}; + var{sv11} sv1: mV {init: 2}; + var{current} x: pA; + var y: per_pA; + + ode(sv1, time) = 1{mV_per_ms}; + x = 1{pA}; + y = 1{dimensionless}/x; + + convert_variable('current', nA, DataDirectionFlow.OUTPUT) + + becomes + var time: ms; + var{sv11} sv1: mV {init: 2}; + var{current} x_converted: nA; + var x : pA + var y: per_pA; + + ode(sv1, time) = 1{mV_per_ms}; + x = 1{pA} + y = 1{dimensionless}/x; + x_converted = 0.001 * x + """ + # original state + def test_original_state(model): + assert len(model.variables()) == 5 + symbol_a = model.get_symbol_by_cmeta_id('sv11') + symbol_t = model.get_symbol_by_cmeta_id('time') + symbol_x = model.get_symbol_by_cmeta_id('current') + symbol_y = model.get_symbol_by_name('env_ode$y') + assert symbol_x.name == 'env_ode$x' + assert model.get_initial_value(symbol_a) == 2.0 + assert not model.get_initial_value(symbol_t) + assert not model.get_initial_value(symbol_x) + assert not model.get_initial_value(symbol_y) + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_x.units == 'pA' + assert symbol_y.units == 'per_pA' + assert len(model.equations) == 3 + assert str(model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(model.equations[1]) == 'Eq(_env_ode$x, _1.0)' + assert str(model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + state_symbols = model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + return True + + pA_unit = literals_model.get_units('pA') + nA_unit = literals_model.get_units('nA') + + assert test_original_state(literals_model) + # test no change in units + literals_model.convert_variable('current', pA_unit, DataDirectionFlow.OUTPUT) + assert test_original_state(literals_model) + + # change pA to nA + literals_model.convert_variable('current', nA_unit, DataDirectionFlow.OUTPUT) + assert len(literals_model.variables()) == 6 + symbol_a = literals_model.get_symbol_by_cmeta_id('sv11') + symbol_t = literals_model.get_symbol_by_cmeta_id('time') + symbol_x = literals_model.get_symbol_by_cmeta_id('current') + assert symbol_x.name == 'env_ode$x_converted' + symbol_y = literals_model.get_symbol_by_name('env_ode$y') + symbol_x_orig = literals_model.get_symbol_by_name('env_ode$x') + assert literals_model.get_initial_value(symbol_a) == 2.0 + assert not literals_model.get_initial_value(symbol_t) + assert not literals_model.get_initial_value(symbol_x) + assert not literals_model.get_initial_value(symbol_y) + assert not literals_model.get_initial_value(symbol_x_orig) + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_x.units == 'nA' + assert symbol_y.units == 'per_pA' + assert symbol_x_orig.units == 'pA' + assert len(literals_model.equations) == 4 + assert str(literals_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(literals_model.equations[1]) == 'Eq(_env_ode$x, _1.0)' + assert str(literals_model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + assert str(literals_model.equations[3]) == 'Eq(_env_ode$x_converted, 0.001*_env_ode$x)' + state_symbols = literals_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + def test_add_output_state_variable(self, local_model): """ Tests the Model.convert_variable function that changes units of given variable. This particular test is when a state variable is being converted as an output @@ -794,6 +1108,8 @@ def test_original_state(local_model): assert local_model.get_free_variable_symbol() == symbol_orig def test_convert_variable_invalid_arguments(self, local_model): + """ Tests the Model.convert_variable() function when involid arguments are passed. + """ unit = local_model.get_units('second') variable = local_model.get_free_variable_symbol() direction = DataDirectionFlow.INPUT @@ -824,6 +1140,8 @@ def test_convert_variable_invalid_arguments(self, local_model): local_model.convert_variable(variable, bad_unit, direction) def test_noconversion_necessary(self, local_model): + """ Tests the Model.convert_variable() when no conversion is necessary. + """ unit = local_model.get_units('ms') variable = local_model.get_free_variable_symbol() direction = DataDirectionFlow.INPUT From fcd4016efdcf375b553026672ef11bb83f4925e5 Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 30 Dec 2019 13:50:04 +0000 Subject: [PATCH 33/47] create helper function for unique names --- cellmlmanip/model.py | 15 ++++--- tests/cellml_files/silly_names.cellml | 61 +++++++++++++++++++++++++++ tests/test_unit_conversion.py | 51 ++++++++++++++++++++++ 3 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 tests/cellml_files/silly_names.cellml diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index da7124ce..37b3b127 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -775,9 +775,7 @@ def _create_new_deriv_variable_and_equation(self, eqn, derivative_variable): :return: new variable for the derivative """ # 1. create a new variable - deriv_name = derivative_variable.name + '_orig_deriv' - while deriv_name in self.variables(): - deriv_name = deriv_name + '_a' + deriv_name = self._get_unique_name(derivative_variable.name + '_orig_deriv') deriv_units = self.units.calculator.traverse(eqn.args[0]) new_deriv_variable = self.add_variable(name=deriv_name, units=deriv_units.units) @@ -848,9 +846,7 @@ def _convert_variable_instance(self, original_variable, cf, units, direction): :return: the new variable created [new units] """ # 1. get unique name for new variable - new_name = original_variable.name + '_converted' - while new_name in self.variables(): - new_name = new_name + '_a' + new_name = self._get_unique_name(original_variable.name + '_converted') # 2. if original has initial_value calculate new initial value new_value = None @@ -891,6 +887,13 @@ def _convert_variable_instance(self, original_variable, cf, units, direction): return new_variable + def _get_unique_name(self, name): + for var in list(self.variables()): + if name == var.name: + name = self._get_unique_name(name + '_a') + break + return name + class NumberDummy(sympy.Dummy): """ diff --git a/tests/cellml_files/silly_names.cellml b/tests/cellml_files/silly_names.cellml new file mode 100644 index 00000000..b88f3a25 --- /dev/null +++ b/tests/cellml_files/silly_names.cellml @@ -0,0 +1,61 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + time + + sv1 + + 1 + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index d87d8f52..d533a2a4 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -35,6 +35,11 @@ def multiode_freevar_model(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ return shared.load_model('repeated_ode_freevar_for_conversion_tests') + @pytest.fixture + def silly_names(scope='function'): + """ Fixture to load a local copy of the basic_ode model that may get modified. """ + return shared.load_model('silly_names') + def test_add_preferred_custom_unit_name(self, simple_ode_model): """ Tests Units.add_preferred_custom_unit_name() function. """ time_var = simple_ode_model.get_symbol_by_ontology_term(shared.OXMETA, "time") @@ -1148,6 +1153,52 @@ def test_noconversion_necessary(self, local_model): assert local_model.convert_variable(variable, unit, direction) == variable + def test_unique_names(self, silly_names): + # original state + def test_original_state(silly_names): + assert len(silly_names.variables()) == 5 + symbol_a = silly_names.get_symbol_by_cmeta_id('sv11') + symbol_t = silly_names.get_symbol_by_cmeta_id('time') + assert silly_names.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert silly_names.get_symbol_by_name('env_ode$sv1_converted') + assert silly_names.get_symbol_by_name('env_ode$sv1_orig_deriv') + assert len(silly_names.equations) == 1 + assert str(silly_names.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + state_symbols = silly_names.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + assert silly_names.get_free_variable_symbol() == symbol_t + return True + + volt_unit = silly_names.get_units('volt') + original_var = silly_names.get_symbol_by_name('env_ode$sv1') + + assert test_original_state(silly_names) + # change mV to V + silly_names.convert_variable(original_var, volt_unit, DataDirectionFlow.INPUT) + assert len(silly_names.variables()) == 7 + symbol_a = silly_names.get_symbol_by_cmeta_id('sv11') + assert silly_names.get_initial_value(symbol_a) == 0.002 + assert symbol_a.units == 'volt' + assert symbol_a.name == 'env_ode$sv1_converted_a' + symbol_t = silly_names.get_symbol_by_cmeta_id('time') + assert symbol_t.units == 'ms' + assert symbol_t.name == 'environment$time' + symbol_orig = silly_names.get_symbol_by_name('env_ode$sv1') + assert symbol_orig.units == 'mV' + assert silly_names.get_symbol_by_name('env_ode$sv1_converted') + assert silly_names.get_symbol_by_name('env_ode$sv1_orig_deriv') + symbol_derv = silly_names.get_symbol_by_name('env_ode$sv1_orig_deriv_a') + assert symbol_derv.units == 'mV / ms' + assert not silly_names.get_initial_value(symbol_derv) + assert len(silly_names.equations) == 3 + assert str(silly_names.equations[0]) == 'Eq(_env_ode$sv1, 1000.0*_env_ode$sv1_converted_a)' + assert str(silly_names.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv_a, _1.0)' + assert str(silly_names.equations[2]) == 'Eq(Derivative(_env_ode$sv1_converted_a, _environment$time), ' \ + '0.001*_env_ode$sv1_orig_deriv_a)' + # def test_missing_units(self, model_missing_units, literals_model): # """ Tests the Model.add_output function that changes units. # In this case the model needs to add the unit it wants to change to. From 5f1cd2dabcfde56556d7c368b1a6728e70ae8274 Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 30 Dec 2019 16:18:19 +0000 Subject: [PATCH 34/47] some doc string tidying --- cellmlmanip/model.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 37b3b127..e5288f59 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -670,8 +670,8 @@ def convert_variable(self, variable, units, direction): equations will be adjusted or DataDirectionFlow.OUTPUT; the variable to be changed is an output, equations are unaffected apart from converting the actual output - :return: the original variable but with new units (or original unchanged if conversion was not necessary - or impossible + :return: the original variable with new units (or original unchanged if conversion was not necessary + or impossible) :throws: AssertionError if the arguments are of incorrect type the variable does not exist in the model DimensionalityError if the unit conversion is impossible @@ -720,13 +720,14 @@ def convert_variable(self, variable, units, direction): def _check_arguments(self, variable, units, direction): """ - Checks the arguments of the convert_variable functions + Checks the arguments of the convert_variable functions. :param variable: variable must be a VariableDummy object present in the model or the string representation of an ontology term referring to a VariableDummy object present in the model :param units: units must be a pint Unit object :param direction: must be part of DataDirectionFlow enum - :returns: VariableDummy object for variable + :returns: VariableDummy object for variable (either + variable passed as argument or variable looked up by ontology term :throws: assertion error """ # variable should be a VariableDummy @@ -842,7 +843,7 @@ def _convert_variable_instance(self, original_variable, cf, units, direction): :param original_variable: VariableDummy object to be converted [old units] :param cf: conversion factor [new units/old units] :param units: Unit object for new units - :param is_output: boolean are we adding input or output + :param direction: enumeration value specifying input or output :return: the new variable created [new units] """ # 1. get unique name for new variable @@ -888,6 +889,10 @@ def _convert_variable_instance(self, original_variable, cf, units, direction): return new_variable def _get_unique_name(self, name): + """ Function to create a unique name within the model. + :param name: String Suggested unique name + :return: String unique name + """ for var in list(self.variables()): if name == var.name: name = self._get_unique_name(name + '_a') From ddba7a0d5c7db7409bb25e2917a1fcb546f57aca Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 30 Dec 2019 16:27:39 +0000 Subject: [PATCH 35/47] tweaks to docstrings --- tests/test_unit_conversion.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index d533a2a4..5c3f1fd0 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -131,7 +131,7 @@ def test_bad_units(self, bad_units_model): bad_units_model.units.summarise_units(equation[1].rhs) def test_add_input_state_variable(self, local_model): - """ Tests the Model.add_input function that changes units. + """ Tests the Model.convert_variable function that changes units. This particular test is when a state variable is being converted For example:: @@ -206,7 +206,7 @@ def test_original_state(local_model): assert symbol_a in state_symbols def test_add_input_state_variable_ontology_term(self, local_model): - """ Tests the Model.add_input function that changes units. + """ Tests the Model.convert_variable function that changes units. This particular test is when a state variable is being converted using ontology term as argument For example:: From 78e9a81d23cf78c00e9f2dfaf9450a0e6c92eb16 Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 6 Jan 2020 09:52:34 +0000 Subject: [PATCH 36/47] Remove using ontology term as argument --- cellmlmanip/model.py | 64 ++++--- tests/test_unit_conversion.py | 313 ---------------------------------- 2 files changed, 30 insertions(+), 347 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index e5288f59..48fc3b46 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -626,10 +626,21 @@ def variables(self): """ Returns an iterator over this model's variable symbols. """ return self._name_to_symbol.values() - def convert_variable(self, variable, units, direction): + def convert_variable(self, original_variable, units, direction): """ - Changes the units of variable argument to the units supplied applying the direction of the - variable from the direction argument. + Add a new linked version of the given variable in the desired units. + + If the variable already has the requested units, no changes are made and the original variable is returned. + Otherwise ``direction`` specifies how information flows between the new variable and the original, and + hence what new equation(s) are added to the model to perform the conversion. + If ``INPUT`` then the original variable takes its value from the newly added variable; + if ``OUTPUT`` then the opposite happens. + + Any ``cmeta:id`` attribute on the original variable is moved to the new one, + so ontology annotations will refer to the new variable. + + Similarly if the direction is ``INPUT`` then any initial value will be moved to the new variable + (and converted appropriately). For example:: @@ -663,20 +674,20 @@ def convert_variable(self, variable, units, direction): time_converted = 0.001 * time - :param variable: the variable to be converted - :param units: units to convert variable to (note if variable is already in these units, model remains - unchanged and the original variable is returned + :param original_variable: the VariableDummy object representing the variable in the model to be converted + :param units: a Pint unit object representing the units to convert variable to (note if variable is already + in these units, model remains unchanged and the original variable is returned :param direction: either DataDirectionFlow.INPUT; the variable to be changed is an input and all affected equations will be adjusted or DataDirectionFlow.OUTPUT; the variable to be changed is an output, equations are unaffected apart from converting the actual output - :return: the original variable with new units (or original unchanged if conversion was not necessary - or impossible) + :return: new variable with desired units, or original unchanged if conversion was not necessary :throws: AssertionError if the arguments are of incorrect type - the variable does not exist in the model + or the variable does not exist in the model DimensionalityError if the unit conversion is impossible """ - original_variable = self._check_arguments(variable, units, direction) + # assertion errors will be thrown here if arguments are incorrect type + self._check_arguments(original_variable, units, direction) original_units = original_variable.units # no conversion necessary @@ -722,38 +733,23 @@ def _check_arguments(self, variable, units, direction): """ Checks the arguments of the convert_variable functions. :param variable: variable must be a VariableDummy object present in the model - or the string representation of an ontology term referring to - a VariableDummy object present in the model - :param units: units must be a pint Unit object + :param units: units must be a pint Unit object in this model :param direction: must be part of DataDirectionFlow enum - :returns: VariableDummy object for variable (either - variable passed as argument or variable looked up by ontology term - :throws: assertion error - """ + :throws: AssertionError if the arguments are of incorrect type + or the variable does not exist in the model + """ # variable should be a VariableDummy - # or an ontology term that references a variable in the model - returned_variable = None - if isinstance(variable, str): - try: - var = self.get_symbol_by_cmeta_id(variable) - except KeyError: - raise AssertionError - name = var.name - returned_variable = var - else: - assert isinstance(variable, VariableDummy) - name = variable.name - returned_variable = variable - assert name in self._name_to_symbol + assert isinstance(variable, VariableDummy) - # units should be a pint Unit object + # variable must b in model + assert variable.name in self._name_to_symbol + + # units should be a pint Unit object in the registry for this model assert isinstance(units, self.units.ureg.Unit) # direction should be part of enum assert isinstance(direction, DataDirectionFlow) - return returned_variable - def _replace_derivatives(self, new_derivative): """ Function to replace an instance of a derivative that occurs on the RHS of any equation diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 5c3f1fd0..f990fc5d 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -205,81 +205,6 @@ def test_original_state(local_model): assert len(state_symbols) == 1 assert symbol_a in state_symbols - def test_add_input_state_variable_ontology_term(self, local_model): - """ Tests the Model.convert_variable function that changes units. - This particular test is when a state variable is being converted using ontology term as argument - - For example:: - - Original model - var{time} time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - - ode(sv1, time) = 1{mV_per_ms}; - - convert_variable('sv11', volt, DataDirectionFlow.INPUT) - - creates model - var{time} time: ms {pub: in}; - var{sv11} sv1_converted: V {init: 0.002}; - var sv1 mV {init: 2} - var sv1_orig_deriv mV_per_ms - - sv1 = 1000 * sv1_converted - sv1_orig_deriv = 1{mV_per_ms} - ode(sv1_converted, time) = 0.001 * sv1_orig_deriv - """ - # original state - def test_original_state(local_model): - assert len(local_model.variables()) == 3 - symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - symbol_t = local_model.get_symbol_by_cmeta_id('time') - assert local_model.get_initial_value(symbol_a) == 2.0 - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert len(local_model.equations) == 1 - assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - state_symbols = local_model.get_state_symbols() - assert len(state_symbols) == 1 - assert symbol_a in state_symbols - return True - - mV_unit = local_model.get_units('mV') - volt_unit = local_model.get_units('volt') - original_var = local_model.get_symbol_by_name('env_ode$sv1') - - assert test_original_state(local_model) - # test no change in units - newvar = local_model.convert_variable('sv11', mV_unit, DataDirectionFlow.INPUT) - assert newvar == original_var - assert test_original_state(local_model) - - # change mV to V - newvar = local_model.convert_variable('sv11', volt_unit, DataDirectionFlow.INPUT) - assert newvar != original_var - assert len(local_model.variables()) == 5 - symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - assert local_model.get_initial_value(symbol_a) == 0.002 - assert symbol_a.units == 'volt' - assert symbol_a.name == 'env_ode$sv1_converted' - symbol_t = local_model.get_symbol_by_cmeta_id('time') - assert symbol_t.units == 'ms' - symbol_orig = local_model.get_symbol_by_name('env_ode$sv1') - assert symbol_orig.units == 'mV' - assert local_model.get_initial_value(symbol_orig) == 2.0 - symbol_derv = local_model.get_symbol_by_name('env_ode$sv1_orig_deriv') - assert symbol_derv.units == 'mV / ms' - assert not local_model.get_initial_value(symbol_derv) - assert len(local_model.equations) == 3 - assert str(local_model.equations[0]) == 'Eq(_env_ode$sv1, 1000.0*_env_ode$sv1_converted)' - assert str(local_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' - assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1_converted, _environment$time), ' \ - '0.001*_env_ode$sv1_orig_deriv)' - - state_symbols = local_model.get_state_symbols() - assert len(state_symbols) == 1 - assert symbol_a in state_symbols - def test_add_input_free_variable(self, local_model): """ Tests the Model.convert_variable function that changes units of given variable. This particular case tests changing a free variable @@ -344,69 +269,6 @@ def test_original_state(local_model): assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1, _environment$time_converted), ' \ '1000.0*_env_ode$sv1_orig_deriv)' - def test_add_input_free_variable_ontology_term(self, local_model): - """ Tests the Model.convert_variable function that changes units of given variable. - This particular case tests changing a free variable using the ontology term - For example:: - - Original model - var{time} time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - - ode(sv1, time) = 1{mV_per_ms}; - - convert_variable('time', second, DataFlowDirection.INPUT) - - becomes - var time: ms; - var{time} time_converted: s; - var{sv11} sv1: mV {init: 2}; - var sv1_orig_deriv mV_per_ms - - time = 1000 * time_converted; - sv1_orig_deriv = 1{mV_per_ms} - ode(sv1, time_converted) = 1000 * sv1_orig_deriv - """ - # original state - def test_original_state(local_model): - assert len(local_model.variables()) == 3 - symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - symbol_t = local_model.get_symbol_by_cmeta_id('time') - assert local_model.get_initial_value(symbol_a) == 2.0 - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert len(local_model.equations) == 1 - assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - return True - - ms_unit = local_model.get_units('ms') - second_unit = local_model.get_units('second') - - assert test_original_state(local_model) - # test no change in units - local_model.convert_variable('time', ms_unit, DataDirectionFlow.INPUT) - assert test_original_state(local_model) - - # change ms to s - local_model.convert_variable('time', second_unit, DataDirectionFlow.INPUT) - assert len(local_model.variables()) == 5 - symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - assert local_model.get_initial_value(symbol_a) == 2.0 - assert symbol_a.units == 'mV' - assert symbol_a.name == 'env_ode$sv1' - symbol_t = local_model.get_symbol_by_cmeta_id('time') - assert symbol_t.units == 'second' - assert symbol_t.name == 'environment$time_converted' - symbol_orig = local_model.get_symbol_by_name('env_ode$sv1') - assert symbol_orig.units == 'mV' - symbol_derv = local_model.get_symbol_by_name('env_ode$sv1_orig_deriv') - assert symbol_derv.units == 'mV / ms' - assert len(local_model.equations) == 3 - assert str(local_model.equations[0]) == 'Eq(_environment$time, 1000.0*_environment$time_converted)' - assert str(local_model.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv, _1.0)' - assert str(local_model.equations[2]) == 'Eq(Derivative(_env_ode$sv1, _environment$time_converted), ' \ - '1000.0*_env_ode$sv1_orig_deriv)' - def test_add_input_literal_variable(self, literals_model): """ Tests the Model.convert_variable function that changes units of given variable. This particular case tests changing a literal variable/constant @@ -492,90 +354,6 @@ def test_original_state(model): assert str(literals_model.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' assert str(literals_model.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' - def test_add_input_literal_variable_ontology_term(self, literals_model): - """ Tests the Model.convert_variable function that changes units of given variable. - This particular case tests changing a literal variable/constant - For example:: - - Original model - var{time} time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - var{current} x: pA; - var y: per_pA; - - ode(sv1, time) = 1{mV_per_ms}; - x = 1{pA}; - y = 1{dimensionless}/x; - - convert_variable('current', nA, DataDirectionFlow.INPUT) - - becomes - var time: ms; - var{sv11} sv1: mV {init: 2}; - var{current} x_converted: nA; - var x : pA - var y: per_pA; - - ode(sv1, time) = 1{mV_per_ms}; - x_converted = 0.001 * 1{pA} - x = 1000* x_converted; - y = 1{dimensionless}/x; - """ - # original state - def test_original_state(model): - assert len(model.variables()) == 5 - symbol_a = model.get_symbol_by_cmeta_id('sv11') - symbol_t = model.get_symbol_by_cmeta_id('time') - symbol_x = model.get_symbol_by_cmeta_id('current') - symbol_y = model.get_symbol_by_name('env_ode$y') - assert symbol_x.name == 'env_ode$x' - assert model.get_initial_value(symbol_a) == 2.0 - assert not model.get_initial_value(symbol_t) - assert not model.get_initial_value(symbol_x) - assert not model.get_initial_value(symbol_y) - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert symbol_x.units == 'pA' - assert symbol_y.units == 'per_pA' - assert len(model.equations) == 3 - assert str(model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(model.equations[1]) == 'Eq(_env_ode$x, _1.0)' - assert str(model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' - return True - - pA_unit = literals_model.get_units('pA') - nA_unit = literals_model.get_units('nA') - - assert test_original_state(literals_model) - # test no change in units - literals_model.convert_variable('current', pA_unit, DataDirectionFlow.INPUT) - assert test_original_state(literals_model) - - # change pA to nA - literals_model.convert_variable('current', nA_unit, DataDirectionFlow.INPUT) - assert len(literals_model.variables()) == 6 - symbol_a = literals_model.get_symbol_by_cmeta_id('sv11') - symbol_t = literals_model.get_symbol_by_cmeta_id('time') - symbol_x = literals_model.get_symbol_by_cmeta_id('current') - assert symbol_x.name == 'env_ode$x_converted' - symbol_y = literals_model.get_symbol_by_name('env_ode$y') - symbol_x_orig = literals_model.get_symbol_by_name('env_ode$x') - assert literals_model.get_initial_value(symbol_a) == 2.0 - assert not literals_model.get_initial_value(symbol_t) - assert not literals_model.get_initial_value(symbol_x) - assert not literals_model.get_initial_value(symbol_y) - assert not literals_model.get_initial_value(symbol_x_orig) - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert symbol_x.units == 'nA' - assert symbol_y.units == 'per_pA' - assert symbol_x_orig.units == 'pA' - assert len(literals_model.equations) == 4 - assert str(literals_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(literals_model.equations[1]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' - assert str(literals_model.equations[2]) == 'Eq(_env_ode$x_converted, 0.001*_1.0)' - assert str(literals_model.equations[3]) == 'Eq(_env_ode$x, 1000.0*_env_ode$x_converted)' - def test_add_input_free_variable_multiple(self, multiode_freevar_model): """ Tests the Model.convert_variable function that changes units of given variable. This particular case tests changing a free variable where there are multiple ode instances. @@ -888,97 +666,6 @@ def test_original_state(model): assert len(state_symbols) == 1 assert symbol_a in state_symbols - def test_add_output_onotology_term(self, literals_model): - """ Tests the Model.convert_variable function that changes units of given variable. - This tests the case when variable to be changed is an OUTPUT using an ontology term - - For example:: - - Original model - var{time} time: ms {pub: in}; - var{sv11} sv1: mV {init: 2}; - var{current} x: pA; - var y: per_pA; - - ode(sv1, time) = 1{mV_per_ms}; - x = 1{pA}; - y = 1{dimensionless}/x; - - convert_variable('current', nA, DataDirectionFlow.OUTPUT) - - becomes - var time: ms; - var{sv11} sv1: mV {init: 2}; - var{current} x_converted: nA; - var x : pA - var y: per_pA; - - ode(sv1, time) = 1{mV_per_ms}; - x = 1{pA} - y = 1{dimensionless}/x; - x_converted = 0.001 * x - """ - # original state - def test_original_state(model): - assert len(model.variables()) == 5 - symbol_a = model.get_symbol_by_cmeta_id('sv11') - symbol_t = model.get_symbol_by_cmeta_id('time') - symbol_x = model.get_symbol_by_cmeta_id('current') - symbol_y = model.get_symbol_by_name('env_ode$y') - assert symbol_x.name == 'env_ode$x' - assert model.get_initial_value(symbol_a) == 2.0 - assert not model.get_initial_value(symbol_t) - assert not model.get_initial_value(symbol_x) - assert not model.get_initial_value(symbol_y) - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert symbol_x.units == 'pA' - assert symbol_y.units == 'per_pA' - assert len(model.equations) == 3 - assert str(model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(model.equations[1]) == 'Eq(_env_ode$x, _1.0)' - assert str(model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' - state_symbols = model.get_state_symbols() - assert len(state_symbols) == 1 - assert symbol_a in state_symbols - return True - - pA_unit = literals_model.get_units('pA') - nA_unit = literals_model.get_units('nA') - - assert test_original_state(literals_model) - # test no change in units - literals_model.convert_variable('current', pA_unit, DataDirectionFlow.OUTPUT) - assert test_original_state(literals_model) - - # change pA to nA - literals_model.convert_variable('current', nA_unit, DataDirectionFlow.OUTPUT) - assert len(literals_model.variables()) == 6 - symbol_a = literals_model.get_symbol_by_cmeta_id('sv11') - symbol_t = literals_model.get_symbol_by_cmeta_id('time') - symbol_x = literals_model.get_symbol_by_cmeta_id('current') - assert symbol_x.name == 'env_ode$x_converted' - symbol_y = literals_model.get_symbol_by_name('env_ode$y') - symbol_x_orig = literals_model.get_symbol_by_name('env_ode$x') - assert literals_model.get_initial_value(symbol_a) == 2.0 - assert not literals_model.get_initial_value(symbol_t) - assert not literals_model.get_initial_value(symbol_x) - assert not literals_model.get_initial_value(symbol_y) - assert not literals_model.get_initial_value(symbol_x_orig) - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert symbol_x.units == 'nA' - assert symbol_y.units == 'per_pA' - assert symbol_x_orig.units == 'pA' - assert len(literals_model.equations) == 4 - assert str(literals_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(literals_model.equations[1]) == 'Eq(_env_ode$x, _1.0)' - assert str(literals_model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' - assert str(literals_model.equations[3]) == 'Eq(_env_ode$x_converted, 0.001*_env_ode$x)' - state_symbols = literals_model.get_state_symbols() - assert len(state_symbols) == 1 - assert symbol_a in state_symbols - def test_add_output_state_variable(self, local_model): """ Tests the Model.convert_variable function that changes units of given variable. This particular test is when a state variable is being converted as an output From 1691e5fc1a42cb1aa249fe48b35e7485707464ec Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 6 Jan 2020 10:24:48 +0000 Subject: [PATCH 37/47] tweaks to docstrings --- cellmlmanip/model.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 48fc3b46..612b202f 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -688,14 +688,14 @@ def convert_variable(self, original_variable, units, direction): """ # assertion errors will be thrown here if arguments are incorrect type self._check_arguments(original_variable, units, direction) - original_units = original_variable.units + original_units = original_variable.units # no conversion necessary if original_units == units: return original_variable # conversion_factor for old units to new - # throws DimensionalityError if not possible + # throws DimensionalityError if unit conversion is not possible cf = self.units.get_conversion_factor(from_unit=original_units, to_unit=units) state_symbols = self.get_state_symbols() @@ -786,7 +786,7 @@ def _create_new_deriv_variable_and_equation(self, eqn, derivative_variable): def _convert_free_variable_deriv(self, eqn, new_variable, cf): """ - Create relevant variables/equations when converting a free variable derivative. + Create relevant variables/equations when converting a free variable within a derivative. :param eqn: the derivative equation containing free variable :param new_variable: the new variable representing the converted symbol [new_units] :param cf: conversion factor for unit conversion [new units/old units] From 8c6690fc61ec2d5ccd62c510683ebf8deffe3ea7 Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 6 Jan 2020 11:33:10 +0000 Subject: [PATCH 38/47] change name of check args function to specify which function it checks args for --- cellmlmanip/model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 612b202f..739dc5e0 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -687,7 +687,7 @@ def convert_variable(self, original_variable, units, direction): DimensionalityError if the unit conversion is impossible """ # assertion errors will be thrown here if arguments are incorrect type - self._check_arguments(original_variable, units, direction) + self._check_arguments_for_convert_variables(original_variable, units, direction) original_units = original_variable.units # no conversion necessary @@ -729,7 +729,7 @@ def convert_variable(self, original_variable, units, direction): return new_variable - def _check_arguments(self, variable, units, direction): + def _check_arguments_for_convert_variables(self, variable, units, direction): """ Checks the arguments of the convert_variable functions. :param variable: variable must be a VariableDummy object present in the model From 72c9105e0b9ec2af8d69769d97ce554a55c8c2c9 Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 6 Jan 2020 11:47:32 +0000 Subject: [PATCH 39/47] moved test that was about units but not conversion --- tests/test_unit_conversion.py | 12 ------------ tests/test_units.py | 12 ++++++++++++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index f990fc5d..f955048a 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -40,18 +40,6 @@ def silly_names(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ return shared.load_model('silly_names') - def test_add_preferred_custom_unit_name(self, simple_ode_model): - """ Tests Units.add_preferred_custom_unit_name() function. """ - time_var = simple_ode_model.get_symbol_by_ontology_term(shared.OXMETA, "time") - assert str(simple_ode_model.units.summarise_units(time_var)) == "ms" - simple_ode_model.units.add_preferred_custom_unit_name('millisecond', [{'prefix': 'milli', 'units': 'second'}]) - assert str(simple_ode_model.units.summarise_units(time_var)) == "millisecond" - # add_custom_unit does not allow adding already existing units but add_preferred_custom_unit_name does since we - # cannot know in advance if a model will already have the unit named this way. To test this we add the same unit - # again - simple_ode_model.units.add_preferred_custom_unit_name('millisecond', [{'prefix': 'milli', 'units': 'second'}]) - assert str(simple_ode_model.units.summarise_units(time_var)) == "millisecond" - def test_conversion_factor_original(self, simple_units_model): """ Tests Units.get_conversion_factor() function. """ symbol_b1 = simple_units_model.get_symbol_by_cmeta_id("b_1") diff --git a/tests/test_units.py b/tests/test_units.py index 76bcd4b5..9a9deaca 100644 --- a/tests/test_units.py +++ b/tests/test_units.py @@ -165,6 +165,18 @@ def test_make_pint_unit_definition(self, quantity_store): assert(quantity_store._make_pint_unit_definition('kg_mm_per_ms', unit_attributes) == 'kg_mm_per_ms=(meter * 1e-3)*(((second * 0.001))**-1)*(2 * kilogram)') + def test_add_preferred_custom_unit_name(self, simple_ode_model): + """ Tests Units.add_preferred_custom_unit_name() function. """ + time_var = simple_ode_model.get_symbol_by_ontology_term(shared.OXMETA, "time") + assert str(simple_ode_model.units.summarise_units(time_var)) == "ms" + simple_ode_model.units.add_preferred_custom_unit_name('millisecond', [{'prefix': 'milli', 'units': 'second'}]) + assert str(simple_ode_model.units.summarise_units(time_var)) == "millisecond" + # add_custom_unit does not allow adding already existing units but add_preferred_custom_unit_name does since we + # cannot know in advance if a model will already have the unit named this way. To test this we add the same unit + # again + simple_ode_model.units.add_preferred_custom_unit_name('millisecond', [{'prefix': 'milli', 'units': 'second'}]) + assert str(simple_ode_model.units.summarise_units(time_var)) == "millisecond" + # Test UnitCalculator class def test_unit_calculator(self, quantity_store): From fbd950c2d56a1d848e0d06ffd0b7f101b1fa7f0a Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 6 Jan 2020 12:18:42 +0000 Subject: [PATCH 40/47] if original var replaced by equation remove initial value --- cellmlmanip/model.py | 3 +++ tests/test_unit_conversion.py | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 739dc5e0..0713912a 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -856,6 +856,9 @@ def _convert_variable_instance(self, original_variable, cf, units, direction): initial_value=new_value, cmeta_id=original_variable.cmeta_id) original_variable.cmeta_id = '' + # if diurection is input; original var will be replaced by equation so do not need to store initial value + if direction == DataDirectionFlow.INPUT: + original_variable.initial_value = None # 4. check whether we had an eqn for original # if so, remove it and replace with new_var [new units] = rhs [old units] * cf [new units/old units] diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index f955048a..435bdcf8 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -135,7 +135,7 @@ def test_add_input_state_variable(self, local_model): creates model var{time} time: ms {pub: in}; var{sv11} sv1_converted: V {init: 0.002}; - var sv1 mV {init: 2} + var sv1 mV var sv1_orig_deriv mV_per_ms sv1 = 1000 * sv1_converted @@ -179,7 +179,7 @@ def test_original_state(local_model): assert symbol_t.units == 'ms' symbol_orig = local_model.get_symbol_by_name('env_ode$sv1') assert symbol_orig.units == 'mV' - assert local_model.get_initial_value(symbol_orig) == 2.0 + assert not local_model.get_initial_value(symbol_orig) symbol_derv = local_model.get_symbol_by_name('env_ode$sv1_orig_deriv') assert symbol_derv.units == 'mV / ms' assert not local_model.get_initial_value(symbol_derv) From ca9ce016e6d37993e62a43cbcb39523ed0305201 Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 6 Jan 2020 12:23:14 +0000 Subject: [PATCH 41/47] remove missing unit tests - to be sorted by fc --- .../missing_units_for_conversion_tests.cellml | 81 ------------------- tests/test_unit_conversion.py | 40 --------- 2 files changed, 121 deletions(-) delete mode 100644 tests/cellml_files/missing_units_for_conversion_tests.cellml diff --git a/tests/cellml_files/missing_units_for_conversion_tests.cellml b/tests/cellml_files/missing_units_for_conversion_tests.cellml deleted file mode 100644 index d965729d..00000000 --- a/tests/cellml_files/missing_units_for_conversion_tests.cellml +++ /dev/null @@ -1,81 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - time - - sv1 - - 1 - - - - x - 1 - - - - y - - - 1 - x - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 435bdcf8..acea7d54 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -15,11 +15,6 @@ def local_model(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ return shared.load_model('basic_ode') - @pytest.fixture - def model_missing_units(scope='function'): - """ Fixture to load a local copy of the basic_ode model that may get modified. """ - return shared.load_model('missing_units_for_conversion_tests') - @pytest.fixture def literals_model(scope='function'): """ Fixture to load a local copy of the basic_ode model that may get modified. """ @@ -873,38 +868,3 @@ def test_original_state(silly_names): assert str(silly_names.equations[1]) == 'Eq(_env_ode$sv1_orig_deriv_a, _1.0)' assert str(silly_names.equations[2]) == 'Eq(Derivative(_env_ode$sv1_converted_a, _environment$time), ' \ '0.001*_env_ode$sv1_orig_deriv_a)' - - # def test_missing_units(self, model_missing_units, literals_model): - # """ Tests the Model.add_output function that changes units. - # In this case the model needs to add the unit it wants to change to. - # e.g. - # var time: ms {pub: in}; - # var{sv11} sv1: mV {init: 2}; - # var{current} x: pA; - # var y: per_pA; - # - # ode(sv1, time) = 1{mV_per_ms}; - # x = 1{pA}; - # y = 1{dimensionless}/x; - # - # change x from pA to nA - # var time: ms {pub: in}; - # var{sv11} sv1: mV {init: 2}; - # var x: pA; - # var y: per_pA; - # var{current} x_converted: nA - # - # ode(sv1, time) = 1 :mV_per_ms; - # x = 1000 * x_converted: pA; - # y = 1{dimensionless}/x; - # x_converted = 0.001 * 1 : nA - # """ - # pA_unit = model_missing_units.get_units('pA') - # nA_unit = literals_model.get_units('nA') - # - # # check nA not in missing_units - # with pytest.raises(KeyError): - # model_missing_units.get_units('nA') - # - # model_missing_units.add_input('env_ode$x', nA_unit) - # From 7d9589d3bc35a12b1cd7331a6a6a1e0d9fdea77f Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 6 Jan 2020 13:10:27 +0000 Subject: [PATCH 42/47] pull repeated get initial state functions into locals --- tests/test_unit_conversion.py | 255 ++++++++++++---------------------- 1 file changed, 86 insertions(+), 169 deletions(-) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index acea7d54..e9507199 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -113,6 +113,74 @@ def test_bad_units(self, bad_units_model): # cellml file states b (per_ms) = power(a (ms), 1 (second)) bad_units_model.units.summarise_units(equation[1].rhs) + # original state for local_model + def _original_state_local_model(self, local_model): + assert len(local_model.variables()) == 3 + symbol_a = local_model.get_symbol_by_cmeta_id('sv11') + symbol_t = local_model.get_symbol_by_cmeta_id('time') + assert local_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert len(local_model.equations) == 1 + assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + state_symbols = local_model.get_state_symbols() + assert len(state_symbols) == 1 + assert symbol_a in state_symbols + return True + + # original state for literals_model + def _original_state_literals_model(self, literals_model): + assert len(literals_model.variables()) == 5 + symbol_a = literals_model.get_symbol_by_cmeta_id('sv11') + symbol_t = literals_model.get_symbol_by_cmeta_id('time') + symbol_x = literals_model.get_symbol_by_cmeta_id('current') + symbol_y = literals_model.get_symbol_by_name('env_ode$y') + assert symbol_x.name == 'env_ode$x' + assert literals_model.get_initial_value(symbol_a) == 2.0 + assert not literals_model.get_initial_value(symbol_t) + assert not literals_model.get_initial_value(symbol_x) + assert not literals_model.get_initial_value(symbol_y) + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_x.units == 'pA' + assert symbol_y.units == 'per_pA' + assert len(literals_model.equations) == 3 + assert str(literals_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(literals_model.equations[1]) == 'Eq(_env_ode$x, _1.0)' + assert str(literals_model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' + return True + + # original state for multiode_model + def _original_state_multiode_model(self, multiode_model): + assert len(multiode_model.variables()) == 5 + symbol_a = multiode_model.get_symbol_by_cmeta_id('sv11') + symbol_t = multiode_model.get_symbol_by_cmeta_id('time') + assert multiode_model.get_initial_value(symbol_a) == 2.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert len(multiode_model.equations) == 3 + assert str(multiode_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(multiode_model.equations[1]) == 'Eq(_env_ode$x, ' \ + '_3.0*Derivative(_env_ode$sv1, _environment$time))' + assert str(multiode_model.equations[2]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' + return True + + # original state + def _original_state_multiode_freevar(self, multiode_freevar_model): + assert len(multiode_freevar_model.variables()) == 4 + symbol_a = multiode_freevar_model.get_symbol_by_cmeta_id('sv11') + symbol_t = multiode_freevar_model.get_symbol_by_cmeta_id('time') + symbol_y = multiode_freevar_model.get_symbol_by_name('env_ode$y') + assert multiode_freevar_model.get_initial_value(symbol_a) == 2.0 + assert multiode_freevar_model.get_initial_value(symbol_y) == 3.0 + assert symbol_a.units == 'mV' + assert symbol_t.units == 'ms' + assert symbol_y.units == 'mV' + assert len(multiode_freevar_model.equations) == 2 + assert str(multiode_freevar_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' + assert str(multiode_freevar_model.equations[1]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' + return True + def test_add_input_state_variable(self, local_model): """ Tests the Model.convert_variable function that changes units. This particular test is when a state variable is being converted @@ -137,30 +205,15 @@ def test_add_input_state_variable(self, local_model): sv1_orig_deriv = 1{mV_per_ms} ode(sv1_converted, time) = 0.001 * sv1_orig_deriv """ - # original state - def test_original_state(local_model): - assert len(local_model.variables()) == 3 - symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - symbol_t = local_model.get_symbol_by_cmeta_id('time') - assert local_model.get_initial_value(symbol_a) == 2.0 - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert len(local_model.equations) == 1 - assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - state_symbols = local_model.get_state_symbols() - assert len(state_symbols) == 1 - assert symbol_a in state_symbols - return True - mV_unit = local_model.get_units('mV') volt_unit = local_model.get_units('volt') original_var = local_model.get_symbol_by_name('env_ode$sv1') - assert test_original_state(local_model) + assert self._original_state_local_model(local_model) # test no change in units newvar = local_model.convert_variable(original_var, mV_unit, DataDirectionFlow.INPUT) assert newvar == original_var - assert test_original_state(local_model) + assert self._original_state_local_model(local_model) # change mV to V newvar = local_model.convert_variable(original_var, volt_unit, DataDirectionFlow.INPUT) @@ -211,26 +264,14 @@ def test_add_input_free_variable(self, local_model): sv1_orig_deriv = 1{mV_per_ms} ode(sv1, time_converted) = 1000 * sv1_orig_deriv """ - # original state - def test_original_state(local_model): - assert len(local_model.variables()) == 3 - symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - symbol_t = local_model.get_symbol_by_cmeta_id('time') - assert local_model.get_initial_value(symbol_a) == 2.0 - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert len(local_model.equations) == 1 - assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - return True - ms_unit = local_model.get_units('ms') second_unit = local_model.get_units('second') original_var = local_model.get_symbol_by_name('environment$time') - assert test_original_state(local_model) + assert self._original_state_local_model(local_model) # test no change in units local_model.convert_variable(original_var, ms_unit, DataDirectionFlow.INPUT) - assert test_original_state(local_model) + assert self._original_state_local_model(local_model) # change ms to s local_model.convert_variable(original_var, second_unit, DataDirectionFlow.INPUT) @@ -281,36 +322,14 @@ def test_add_input_literal_variable(self, literals_model): x = 1000* x_converted; y = 1{dimensionless}/x; """ - # original state - def test_original_state(model): - assert len(model.variables()) == 5 - symbol_a = model.get_symbol_by_cmeta_id('sv11') - symbol_t = model.get_symbol_by_cmeta_id('time') - symbol_x = model.get_symbol_by_cmeta_id('current') - symbol_y = model.get_symbol_by_name('env_ode$y') - assert symbol_x.name == 'env_ode$x' - assert model.get_initial_value(symbol_a) == 2.0 - assert not model.get_initial_value(symbol_t) - assert not model.get_initial_value(symbol_x) - assert not model.get_initial_value(symbol_y) - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert symbol_x.units == 'pA' - assert symbol_y.units == 'per_pA' - assert len(model.equations) == 3 - assert str(model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(model.equations[1]) == 'Eq(_env_ode$x, _1.0)' - assert str(model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' - return True - pA_unit = literals_model.get_units('pA') nA_unit = literals_model.get_units('nA') original_var = literals_model.get_symbol_by_name('env_ode$x') - assert test_original_state(literals_model) + assert self._original_state_literals_model(literals_model) # test no change in units literals_model.convert_variable(original_var, pA_unit, DataDirectionFlow.INPUT) - assert test_original_state(literals_model) + assert self._original_state_literals_model(literals_model) # change pA to nA literals_model.convert_variable(original_var, nA_unit, DataDirectionFlow.INPUT) @@ -366,30 +385,14 @@ def test_add_input_free_variable_multiple(self, multiode_freevar_model): y_orig_deriv = 2{mV_per_ms} ode(y, time_converted) = 1000 * y_orig_deriv """ - # original state - def test_original_state(multiode_freevar_model): - assert len(multiode_freevar_model.variables()) == 4 - symbol_a = multiode_freevar_model.get_symbol_by_cmeta_id('sv11') - symbol_t = multiode_freevar_model.get_symbol_by_cmeta_id('time') - symbol_y = multiode_freevar_model.get_symbol_by_name('env_ode$y') - assert multiode_freevar_model.get_initial_value(symbol_a) == 2.0 - assert multiode_freevar_model.get_initial_value(symbol_y) == 3.0 - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert symbol_y.units == 'mV' - assert len(multiode_freevar_model.equations) == 2 - assert str(multiode_freevar_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(multiode_freevar_model.equations[1]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' - return True - ms_unit = multiode_freevar_model.get_units('ms') second_unit = multiode_freevar_model.get_units('second') original_var = multiode_freevar_model.get_symbol_by_name('environment$time') - assert test_original_state(multiode_freevar_model) + assert self._original_state_multiode_freevar(multiode_freevar_model) # test no change in units multiode_freevar_model.convert_variable(original_var, ms_unit, DataDirectionFlow.INPUT) - assert test_original_state(multiode_freevar_model) + assert self._original_state_multiode_freevar(multiode_freevar_model) # change ms to s multiode_freevar_model.convert_variable(original_var, second_unit, DataDirectionFlow.INPUT) @@ -452,29 +455,14 @@ def test_multiple_odes(self, multiode_model): ode(sv1_converted, time) = 0.001 * sv1_orig_deriv x = 3 * sv1_orig_deriv """ - # original state - def test_original_state(multiode_model): - assert len(multiode_model.variables()) == 5 - symbol_a = multiode_model.get_symbol_by_cmeta_id('sv11') - symbol_t = multiode_model.get_symbol_by_cmeta_id('time') - assert multiode_model.get_initial_value(symbol_a) == 2.0 - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert len(multiode_model.equations) == 3 - assert str(multiode_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(multiode_model.equations[1]) == 'Eq(_env_ode$x, ' \ - '_3.0*Derivative(_env_ode$sv1, _environment$time))' - assert str(multiode_model.equations[2]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' - return True - mV_unit = multiode_model.get_units('mV') volt_unit = multiode_model.get_units('volt') original_var = multiode_model.get_symbol_by_name('env_ode$sv1') - assert test_original_state(multiode_model) + assert self._original_state_multiode_model(multiode_model) # test no change in units multiode_model.convert_variable(original_var, mV_unit, DataDirectionFlow.INPUT) - assert test_original_state(multiode_model) + assert self._original_state_multiode_model(multiode_model) # change mV to V multiode_model.convert_variable(original_var, volt_unit, DataDirectionFlow.INPUT) @@ -521,29 +509,14 @@ def test_multiple_odes_1(self, multiode_model): ode(sv1, time_converted) = 1000 * sv1_orig_deriv x = 3 * sv1_orig_deriv """ - # original state - def test_original_state(multiode_model): - assert len(multiode_model.variables()) == 5 - symbol_a = multiode_model.get_symbol_by_cmeta_id('sv11') - symbol_t = multiode_model.get_symbol_by_cmeta_id('time') - assert multiode_model.get_initial_value(symbol_a) == 2.0 - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert len(multiode_model.equations) == 3 - assert str(multiode_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(multiode_model.equations[1]) == 'Eq(_env_ode$x, ' \ - '_3.0*Derivative(_env_ode$sv1, _environment$time))' - assert str(multiode_model.equations[2]) == 'Eq(Derivative(_env_ode$y, _environment$time), _2.0)' - return True - ms_unit = multiode_model.get_units('ms') second_unit = multiode_model.get_units('second') original_var = multiode_model.get_symbol_by_name('environment$time') - assert test_original_state(multiode_model) + assert self._original_state_multiode_model(multiode_model) # test no change in units multiode_model.convert_variable(original_var, ms_unit, DataDirectionFlow.INPUT) - assert test_original_state(multiode_model) + assert self._original_state_multiode_model(multiode_model) # change ms to s multiode_model.convert_variable(original_var, second_unit, DataDirectionFlow.INPUT) @@ -587,39 +560,14 @@ def test_add_output(self, literals_model): y = 1{dimensionless}/x; x_converted = 0.001 * x """ - # original state - def test_original_state(model): - assert len(model.variables()) == 5 - symbol_a = model.get_symbol_by_cmeta_id('sv11') - symbol_t = model.get_symbol_by_cmeta_id('time') - symbol_x = model.get_symbol_by_cmeta_id('current') - symbol_y = model.get_symbol_by_name('env_ode$y') - assert symbol_x.name == 'env_ode$x' - assert model.get_initial_value(symbol_a) == 2.0 - assert not model.get_initial_value(symbol_t) - assert not model.get_initial_value(symbol_x) - assert not model.get_initial_value(symbol_y) - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert symbol_x.units == 'pA' - assert symbol_y.units == 'per_pA' - assert len(model.equations) == 3 - assert str(model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - assert str(model.equations[1]) == 'Eq(_env_ode$x, _1.0)' - assert str(model.equations[2]) == 'Eq(_env_ode$y, _1.0/_env_ode$x)' - state_symbols = model.get_state_symbols() - assert len(state_symbols) == 1 - assert symbol_a in state_symbols - return True - pA_unit = literals_model.get_units('pA') nA_unit = literals_model.get_units('nA') original_var = literals_model.get_symbol_by_name('env_ode$x') - assert test_original_state(literals_model) + assert self._original_state_literals_model(literals_model) # test no change in units literals_model.convert_variable(original_var, pA_unit, DataDirectionFlow.OUTPUT) - assert test_original_state(literals_model) + assert self._original_state_literals_model(literals_model) # change pA to nA literals_model.convert_variable(original_var, nA_unit, DataDirectionFlow.OUTPUT) @@ -671,29 +619,14 @@ def test_add_output_state_variable(self, local_model): ode(sv1, time) = 1{mV_per_ms}; sv1_converted = sv1 / 1000 """ - # original state - def test_original_state(local_model): - assert len(local_model.variables()) == 3 - symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - symbol_t = local_model.get_symbol_by_cmeta_id('time') - assert local_model.get_initial_value(symbol_a) == 2.0 - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert len(local_model.equations) == 1 - assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - state_symbols = local_model.get_state_symbols() - assert len(state_symbols) == 1 - assert symbol_a in state_symbols - return True - mV_unit = local_model.get_units('mV') volt_unit = local_model.get_units('volt') original_var = local_model.get_symbol_by_name('env_ode$sv1') - assert test_original_state(local_model) + assert self._original_state_local_model(local_model) # test no change in units local_model.convert_variable(original_var, mV_unit, DataDirectionFlow.OUTPUT) - assert test_original_state(local_model) + assert self._original_state_local_model(local_model) # change mV to V local_model.convert_variable(original_var, volt_unit, DataDirectionFlow.OUTPUT) @@ -737,30 +670,14 @@ def test_add_output_free_variable(self, local_model): time_converted = 0.001 * time """ - # original state - def test_original_state(local_model): - assert len(local_model.variables()) == 3 - symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - symbol_t = local_model.get_symbol_by_cmeta_id('time') - assert local_model.get_initial_value(symbol_a) == 2.0 - assert symbol_a.units == 'mV' - assert symbol_t.units == 'ms' - assert len(local_model.equations) == 1 - assert str(local_model.equations[0]) == 'Eq(Derivative(_env_ode$sv1, _environment$time), _1.0)' - state_symbols = local_model.get_state_symbols() - assert len(state_symbols) == 1 - assert symbol_a in state_symbols - assert local_model.get_free_variable_symbol() == symbol_t - return True - ms_unit = local_model.get_units('ms') second_unit = local_model.get_units('second') original_var = local_model.get_symbol_by_name('environment$time') - assert test_original_state(local_model) + assert self._original_state_local_model(local_model) # test no change in units local_model.convert_variable(original_var, ms_unit, DataDirectionFlow.OUTPUT) - assert test_original_state(local_model) + assert self._original_state_local_model(local_model) # change ms to s local_model.convert_variable(original_var, second_unit, DataDirectionFlow.OUTPUT) From 4cea9dd47a40ef617f76cd0b6feb77a3716e8a3d Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 6 Jan 2020 15:28:28 +0000 Subject: [PATCH 43/47] add test for unchanged initial value and remove redundant test --- tests/test_unit_conversion.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index e9507199..a75c41cb 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -376,7 +376,7 @@ def test_add_input_free_variable_multiple(self, multiode_freevar_model): var{time} time_converted: s; var{sv11} sv1: mV {init: 2}; var sv1_orig_deriv mV_per_ms - var y : mV + var y : mV {init: 3} var y_orig_deriv mV_per_ms time = 1000 * time_converted; @@ -410,6 +410,7 @@ def test_add_input_free_variable_multiple(self, multiode_freevar_model): assert symbol_derv.units == 'mV / ms' symbol_orig_y = multiode_freevar_model.get_symbol_by_name('env_ode$y') assert symbol_orig_y.units == 'mV' + assert symbol_orig_y.initial_value == 3.0 symbol_derv_y = multiode_freevar_model.get_symbol_by_name('env_ode$y_orig_deriv') assert symbol_derv_y.units == 'mV / ms' assert len(multiode_freevar_model.equations) == 5 @@ -731,16 +732,7 @@ def test_convert_variable_invalid_arguments(self, local_model): with pytest.raises(DimensionalityError): local_model.convert_variable(variable, bad_unit, direction) - def test_noconversion_necessary(self, local_model): - """ Tests the Model.convert_variable() when no conversion is necessary. - """ - unit = local_model.get_units('ms') - variable = local_model.get_free_variable_symbol() - direction = DataDirectionFlow.INPUT - - assert local_model.convert_variable(variable, unit, direction) == variable - - def test_unique_names(self, silly_names): + def test_unique_names(self, silly_names): # original state def test_original_state(silly_names): assert len(silly_names.variables()) == 5 From 15f5861072a46d99e8b7d97128ffd3c4994837be Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 6 Jan 2020 15:31:05 +0000 Subject: [PATCH 44/47] fix indent --- tests/test_unit_conversion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index a75c41cb..9c6e9af0 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -732,7 +732,7 @@ def test_convert_variable_invalid_arguments(self, local_model): with pytest.raises(DimensionalityError): local_model.convert_variable(variable, bad_unit, direction) - def test_unique_names(self, silly_names): + def test_unique_names(self, silly_names): # original state def test_original_state(silly_names): assert len(silly_names.variables()) == 5 From b51f3f07d824210e81d91e9815750c3b454d6af1 Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 6 Jan 2020 17:25:44 +0000 Subject: [PATCH 45/47] tidy up logic of adding and replacing equations --- cellmlmanip/model.py | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 0713912a..82cd5cf0 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -856,34 +856,32 @@ def _convert_variable_instance(self, original_variable, cf, units, direction): initial_value=new_value, cmeta_id=original_variable.cmeta_id) original_variable.cmeta_id = '' - # if diurection is input; original var will be replaced by equation so do not need to store initial value + + # 4 add/remove/replace equations if direction == DataDirectionFlow.INPUT: + # if direction is input; original var will be replaced by equation so do not need to store initial value original_variable.initial_value = None - # 4. check whether we had an eqn for original - # if so, remove it and replace with new_var [new units] = rhs [old units] * cf [new units/old units] - original_had_eqn = False - for equation in self.equations: - if equation.is_Equality and equation.args[0] == original_variable: - if direction == DataDirectionFlow.OUTPUT: - expression = sympy.Eq(new_variable, original_variable * cf) - self.add_equation(expression) - else: + # find the equation for the original variable: orig_var = rhs + # remove equation from model + # add eqn for new variabale in terms of rhs of equation + # new_var [new units] = rhs [old units] * cf [new units/old units] + for equation in self.equations: + if equation.is_Equality and equation.args[0] == original_variable: expression = sympy.Eq(new_variable, equation.args[1] * cf) self.add_equation(expression) self.remove_equation(equation) - original_had_eqn = True - break + break - # 5. add an equation for original variable if there wasnt one - # oldvar [old units] = newvar [new units] / cf [new units/old units] - if direction == DataDirectionFlow.INPUT: + # add eqn for original variable in terms of new variable + # orig_var [old units] = new var [new units] / cf [new units/old units] expression = sympy.Eq(original_variable, new_variable / cf) self.add_equation(expression) else: - if not original_had_eqn: - expression = sympy.Eq(new_variable, original_variable * cf) - self.add_equation(expression) + # if direction is output add eqn for new variable in terms of original variable + # new_var [new units] = orig_var [old units] * cf [new units/old units] + expression = sympy.Eq(new_variable, original_variable * cf) + self.add_equation(expression) return new_variable From 4d95504bdd8f14b97f85867f3b0edee01cb9f545 Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Wed, 8 Jan 2020 14:11:05 +0000 Subject: [PATCH 46/47] Fix typos --- cellmlmanip/model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 82cd5cf0..52f527c9 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -731,7 +731,7 @@ def convert_variable(self, original_variable, units, direction): def _check_arguments_for_convert_variables(self, variable, units, direction): """ - Checks the arguments of the convert_variable functions. + Checks the arguments of the convert_variable function. :param variable: variable must be a VariableDummy object present in the model :param units: units must be a pint Unit object in this model :param direction: must be part of DataDirectionFlow enum @@ -741,7 +741,7 @@ def _check_arguments_for_convert_variables(self, variable, units, direction): # variable should be a VariableDummy assert isinstance(variable, VariableDummy) - # variable must b in model + # variable must be in model assert variable.name in self._name_to_symbol # units should be a pint Unit object in the registry for this model From bd8411fca0cbb19575021777822fe7bad4752296 Mon Sep 17 00:00:00 2001 From: Jonathan Cooper Date: Wed, 8 Jan 2020 14:11:37 +0000 Subject: [PATCH 47/47] New var doesn't get initial_value in OUTPUT case --- cellmlmanip/model.py | 4 ++-- tests/test_unit_conversion.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py index 52f527c9..e350ecf9 100644 --- a/cellmlmanip/model.py +++ b/cellmlmanip/model.py @@ -845,9 +845,9 @@ def _convert_variable_instance(self, original_variable, cf, units, direction): # 1. get unique name for new variable new_name = self._get_unique_name(original_variable.name + '_converted') - # 2. if original has initial_value calculate new initial value + # 2. if original has initial_value calculate new initial value (only needed for INPUT case) new_value = None - if original_variable.initial_value: + if direction == DataDirectionFlow.INPUT and original_variable.initial_value: new_value = original_variable.initial_value * cf # 3. copy cmeta_id from original and remove from original diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 9c6e9af0..126b6581 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -614,7 +614,7 @@ def test_add_output_state_variable(self, local_model): creates model var{time} time: ms {pub: in}; - var{sv11} sv1_converted: V {init: 0.002}; + var{sv11} sv1_converted: V; var sv1 mV {init: 2} ode(sv1, time) = 1{mV_per_ms}; @@ -633,7 +633,7 @@ def test_add_output_state_variable(self, local_model): local_model.convert_variable(original_var, volt_unit, DataDirectionFlow.OUTPUT) assert len(local_model.variables()) == 4 symbol_a = local_model.get_symbol_by_cmeta_id('sv11') - assert local_model.get_initial_value(symbol_a) == 0.002 + assert not local_model.get_initial_value(symbol_a) assert symbol_a.units == 'volt' assert symbol_a.name == 'env_ode$sv1_converted' symbol_t = local_model.get_symbol_by_cmeta_id('time')