From b6c4a058c735649452b538a5418c41d4d577c1ff Mon Sep 17 00:00:00 2001 From: Maurice Hendrix Date: Tue, 27 Sep 2022 23:16:20 +0100 Subject: [PATCH 01/14] fixing errors deling with offsets in unit definitions --- cellmlmanip/parser.py | 10 ++++-- cellmlmanip/units.py | 24 ++++++++++++- tests/cellml_files/dimensionless_exp.cellml | 19 ++++++++++ .../dimensionless_multiplier.cellml | 19 ++++++++++ .../cellml_files/dimensionless_offset.cellml | 19 ++++++++++ tests/cellml_files/test_offset.cellml | 19 ++++++++++ tests/test_parser.py | 35 +++++++++++++++++++ 7 files changed, 141 insertions(+), 4 deletions(-) create mode 100644 tests/cellml_files/dimensionless_exp.cellml create mode 100644 tests/cellml_files/dimensionless_multiplier.cellml create mode 100644 tests/cellml_files/dimensionless_offset.cellml create mode 100644 tests/cellml_files/test_offset.cellml diff --git a/cellmlmanip/parser.py b/cellmlmanip/parser.py index 28a676c2..7422f91f 100644 --- a/cellmlmanip/parser.py +++ b/cellmlmanip/parser.py @@ -207,8 +207,8 @@ def _add_units(self, model): # unit is defined in terms of known units - ok to add to model if add_now: - definition = self._make_pint_unit_definition(unit_name, unit_elements) - self.model.units.add_unit(unit_name, definition) + definition, offset = self._make_pint_unit_definition(unit_name, unit_elements) + self.model.units.add_unit(unit_name, definition, offset) units_found.add(unit_name) iteration = 0 else: @@ -230,6 +230,7 @@ def _make_pint_unit_definition(self, units_name, unit_attributes): """ full_unit_expr = [] + offset = None # For each of the elements for this unit definition for unit_element in unit_attributes: @@ -253,11 +254,14 @@ def _make_pint_unit_definition(self, units_name, unit_attributes): if 'multiplier' in unit_element: expr = '(%s * %s)' % (unit_element['multiplier'], expr) + if 'offset' in unit_element: + offset = unit_element['offset'] + # Collect/add this particular definition full_unit_expr.append(expr) # Join together all the parts of the unit expression and return - return '*'.join(full_unit_expr) + return '*'.join(full_unit_expr), offset def _add_components(self, model): """ diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index b924ef34..959e2a49 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -140,7 +140,7 @@ def __init__(self, store=None): self.Unit = self._registry.Unit self.Quantity = self._registry.Quantity - def add_unit(self, name, expression): + def add_unit(self, name, expression, offset=None): """Adds a unit called ``name`` to the unit store, as defined by the string ``expression``. For example:: @@ -168,9 +168,15 @@ def add_unit(self, name, expression): # To test if this is a dimensionless unit, parse the string as a Quantity and check if it's dimensionless quantity = self._registry.parse_expression(expression) if quantity.units == self._registry.dimensionless: + # offsets are not supported for dimensionless + if offset is not None: + raise NotSupportedErrorOffsetsDimensionless(expression + '; offset: ' + offset) definition = UnitDefinition(qname, '', (), ScaleConverter(quantity.to(self._registry.dimensionless))) else: definition = qname + '=' + expression + # if we have an offset, add that to the definition now + if offset is not None: + definition += '; offset: ' + offset # Add to registry self._registry.define(definition) @@ -290,6 +296,10 @@ def convert(self, quantity, unit): """ assert isinstance(quantity, self._registry.Quantity) assert isinstance(unit, self._registry.Unit) + # Trying to convert dimensionless gives an error but we can convert to it + if quantity.units == self._registry.dimensionless: + quantity, unit = 1 * unit, quantity.units + return 1 / quantity.to(unit) return quantity.to(unit) def add_conversion_rule(self, from_unit, to_unit, rule): @@ -895,3 +905,15 @@ class UnitConversionError(UnitError): def __init__(self, expression, from_units, to_units): self.expression = expression self.message = 'Cannot convert units from {} to {}'.format(from_units, to_units) + + +class NotSupportedErrorOffsetsDimensionless(UnitError): + """Represents failure to convert between incompatible units. + + :param expression: the Sympy expression in which the error occurred + :param from_unit: the units the expression is in + :param to_unit: the units we tried to convert to + """ + def __init__(self, expression): + self.expression = expression + self.message = 'Offsets for dimensionless units are not supported' diff --git a/tests/cellml_files/dimensionless_exp.cellml b/tests/cellml_files/dimensionless_exp.cellml new file mode 100644 index 00000000..d1aa597a --- /dev/null +++ b/tests/cellml_files/dimensionless_exp.cellml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/cellml_files/dimensionless_multiplier.cellml b/tests/cellml_files/dimensionless_multiplier.cellml new file mode 100644 index 00000000..13d12c98 --- /dev/null +++ b/tests/cellml_files/dimensionless_multiplier.cellml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/cellml_files/dimensionless_offset.cellml b/tests/cellml_files/dimensionless_offset.cellml new file mode 100644 index 00000000..39a4d370 --- /dev/null +++ b/tests/cellml_files/dimensionless_offset.cellml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/cellml_files/test_offset.cellml b/tests/cellml_files/test_offset.cellml new file mode 100644 index 00000000..fbb813d3 --- /dev/null +++ b/tests/cellml_files/test_offset.cellml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/test_parser.py b/tests/test_parser.py index e3d41856..2e2baafb 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -5,6 +5,7 @@ import sympy from cellmlmanip import parser +from cellmlmanip.units import NotSupportedErrorOffsetsDimensionless from .shared import check_left_right_units_equal, load_model @@ -331,3 +332,37 @@ def test_piecewise_booleans_error(self): sympy.Eq(Ax, zero)] model_eqs = sorted(map(str, model.equations)) assert model_eqs == sorted(map(str, set(eqs1))) or model_eqs == sorted(map(str, set(eqs2))) + + def test_dimensionless_exp(self): + model = load_model('dimensionless_exp.cellml') + assert sorted(map(str, model.variables())) == ['A$x', 'B$y'] + dimensionless = model.units.get_unit('dimensionless') + hyper_dimensionless = model.units.get_unit('hyper_dimensionless') + cf1 = model.units.convert(1 * dimensionless, hyper_dimensionless) + cf2 = model.units.convert(1 * hyper_dimensionless, dimensionless) + assert cf1 == cf2 == 1 + + def test_dimensionless_multiplier(self): + model = load_model('dimensionless_multiplier.cellml') + assert sorted(map(str, model.variables())) == ['A$x', 'B$y'] + dimensionless = model.units.get_unit('dimensionless') + halves = model.units.get_unit('halves') + cf1 = model.units.convert(1 * dimensionless, halves) + cf2 = model.units.convert(1 * halves, dimensionless) + assert cf1 == 1 / cf2, str([cf1, cf2]) + + def test_dimensionless_offset(self): + with pytest.raises(NotSupportedErrorOffsetsDimensionless): + load_model('dimensionless_offset.cellml') + + def test_offset(self): + model = load_model('test_offset.cellml') + assert sorted(map(str, model.variables())) == ['A$x', 'B$y'] + meter = model.units.get_unit('meter') + offsetmeter = model.units.get_unit('offsetmeter') + to_offset = (1 * meter).to(offsetmeter) + to_meter = (1 * offsetmeter).to(meter) + assert to_offset.to(meter) == 1 * meter + assert to_meter.to(offsetmeter) == 1 * offsetmeter + assert to_offset.magnitude == -9.0 + assert to_meter.magnitude == 11.0 From cfabac852ff4d3c37189a6c6ad4b6bb7caa44cbd Mon Sep 17 00:00:00 2001 From: Maurice Hendrix Date: Tue, 27 Sep 2022 23:54:34 +0100 Subject: [PATCH 02/14] release notes --- RELEASE.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/RELEASE.md b/RELEASE.md index 76e0d7de..c8ebcf36 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -2,6 +2,9 @@ - Fixed a bug in the parser where equations in a piecewise containing a boolean caused parsing errors. see https://github.com/ModellingWebLab/cellmlmanip/issues/350 +- Fixes errors dealing with demenionless units which have an offset, multiplier or exponent and implements offset units more generally + see https://github.com/ModellingWebLab/cellmlmanip/issues/351 + # Release 0.3.4 - Updated how substitution of functions that were changed in the parser are handled during analysis for fixing singularities, in order to make sure it workes with sympy 1.10 - Dropped support for python 3.5 as it is end of life. From 27b489e428a73d239a681ec9e35458a9829593f2 Mon Sep 17 00:00:00 2001 From: Maurice Hendrix Date: Wed, 28 Sep 2022 10:41:43 +0100 Subject: [PATCH 03/14] added warnings for offset units --- RELEASE.md | 2 +- cellmlmanip/units.py | 11 +++++------ tests/test_parser.py | 14 +++++--------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index c8ebcf36..0dea72ec 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -2,7 +2,7 @@ - Fixed a bug in the parser where equations in a piecewise containing a boolean caused parsing errors. see https://github.com/ModellingWebLab/cellmlmanip/issues/350 -- Fixes errors dealing with demenionless units which have an offset, multiplier or exponent and implements offset units more generally +- Fixes errors dealing with dimensionless units which have an offset, multiplier or exponent and added a warbing for other offset units to indicate that these are not supported. see https://github.com/ModellingWebLab/cellmlmanip/issues/351 # Release 0.3.4 diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index 959e2a49..ba58ff81 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -168,15 +168,14 @@ def add_unit(self, name, expression, offset=None): # To test if this is a dimensionless unit, parse the string as a Quantity and check if it's dimensionless quantity = self._registry.parse_expression(expression) if quantity.units == self._registry.dimensionless: - # offsets are not supported for dimensionless + # offsets are not supported for dimensionless if offset is not None: - raise NotSupportedErrorOffsetsDimensionless(expression + '; offset: ' + offset) + raise OffsetNotSupportedError(expression + '; offset: ' + offset) definition = UnitDefinition(qname, '', (), ScaleConverter(quantity.to(self._registry.dimensionless))) else: definition = qname + '=' + expression - # if we have an offset, add that to the definition now - if offset is not None: - definition += '; offset: ' + offset + if offset is not None: # just warn and ignore + logger.warning('Offsets in unit definitions are not supported and are ignored!') # Add to registry self._registry.define(definition) @@ -907,7 +906,7 @@ def __init__(self, expression, from_units, to_units): self.message = 'Cannot convert units from {} to {}'.format(from_units, to_units) -class NotSupportedErrorOffsetsDimensionless(UnitError): +class OffsetNotSupportedError(UnitError): """Represents failure to convert between incompatible units. :param expression: the Sympy expression in which the error occurred diff --git a/tests/test_parser.py b/tests/test_parser.py index 2e2baafb..1f0dbca8 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -5,7 +5,7 @@ import sympy from cellmlmanip import parser -from cellmlmanip.units import NotSupportedErrorOffsetsDimensionless +from cellmlmanip.units import OffsetNotSupportedError from .shared import check_left_right_units_equal, load_model @@ -352,17 +352,13 @@ def test_dimensionless_multiplier(self): assert cf1 == 1 / cf2, str([cf1, cf2]) def test_dimensionless_offset(self): - with pytest.raises(NotSupportedErrorOffsetsDimensionless): + with pytest.raises(OffsetNotSupportedError): load_model('dimensionless_offset.cellml') - def test_offset(self): + def test_offset(self, caplog): model = load_model('test_offset.cellml') + assert 'Offsets in unit definitions are not supported and are ignored!' in caplog.text assert sorted(map(str, model.variables())) == ['A$x', 'B$y'] meter = model.units.get_unit('meter') offsetmeter = model.units.get_unit('offsetmeter') - to_offset = (1 * meter).to(offsetmeter) - to_meter = (1 * offsetmeter).to(meter) - assert to_offset.to(meter) == 1 * meter - assert to_meter.to(offsetmeter) == 1 * offsetmeter - assert to_offset.magnitude == -9.0 - assert to_meter.magnitude == 11.0 + assert model.units.is_equivalent(meter, offsetmeter) From b6ae4ecfda630171d4f6b458fe4f3022e2dd5cbc Mon Sep 17 00:00:00 2001 From: Maurice Hendrix Date: Wed, 28 Sep 2022 10:47:21 +0100 Subject: [PATCH 04/14] ignore offset of 0 without warning --- cellmlmanip/units.py | 2 +- tests/cellml_files/test_offset_0.cellml | 19 +++++++++++++++++++ tests/test_parser.py | 8 ++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 tests/cellml_files/test_offset_0.cellml diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index ba58ff81..2766edb9 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -174,7 +174,7 @@ def add_unit(self, name, expression, offset=None): definition = UnitDefinition(qname, '', (), ScaleConverter(quantity.to(self._registry.dimensionless))) else: definition = qname + '=' + expression - if offset is not None: # just warn and ignore + if offset is not None and offset.strip() != '0': # just warn and ignore logger.warning('Offsets in unit definitions are not supported and are ignored!') # Add to registry diff --git a/tests/cellml_files/test_offset_0.cellml b/tests/cellml_files/test_offset_0.cellml new file mode 100644 index 00000000..d1267e28 --- /dev/null +++ b/tests/cellml_files/test_offset_0.cellml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/test_parser.py b/tests/test_parser.py index 1f0dbca8..35966f8c 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -362,3 +362,11 @@ def test_offset(self, caplog): meter = model.units.get_unit('meter') offsetmeter = model.units.get_unit('offsetmeter') assert model.units.is_equivalent(meter, offsetmeter) + + def test_offset_0(self, caplog): + model = load_model('test_offset_0.cellml') + assert 'Offsets in unit definitions are not supported and are ignored!' not in caplog.text + assert sorted(map(str, model.variables())) == ['A$x', 'B$y'] + meter = model.units.get_unit('meter') + offsetmeter = model.units.get_unit('offsetmeter') + assert model.units.is_equivalent(meter, offsetmeter) From 8f291f032bc7941c5739f1f1aea82d765f2c9d8c Mon Sep 17 00:00:00 2001 From: Maurice Hendrix Date: Wed, 28 Sep 2022 11:10:26 +0100 Subject: [PATCH 05/14] ValueError nstead for non-0 offset units --- RELEASE.md | 2 +- cellmlmanip/parser.py | 11 +++++------ cellmlmanip/units.py | 19 +------------------ tests/test_parser.py | 11 +++-------- 4 files changed, 10 insertions(+), 33 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index 0dea72ec..8d715c2a 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -2,7 +2,7 @@ - Fixed a bug in the parser where equations in a piecewise containing a boolean caused parsing errors. see https://github.com/ModellingWebLab/cellmlmanip/issues/350 -- Fixes errors dealing with dimensionless units which have an offset, multiplier or exponent and added a warbing for other offset units to indicate that these are not supported. +- Fixes errors dealing with dimensionless units which have a multiplier or exponent and added an error for offset units (where the offset is not 0) as those are not supported. see https://github.com/ModellingWebLab/cellmlmanip/issues/351 # Release 0.3.4 diff --git a/cellmlmanip/parser.py b/cellmlmanip/parser.py index 7422f91f..bc07781a 100644 --- a/cellmlmanip/parser.py +++ b/cellmlmanip/parser.py @@ -207,8 +207,8 @@ def _add_units(self, model): # unit is defined in terms of known units - ok to add to model if add_now: - definition, offset = self._make_pint_unit_definition(unit_name, unit_elements) - self.model.units.add_unit(unit_name, definition, offset) + definition = self._make_pint_unit_definition(unit_name, unit_elements) + self.model.units.add_unit(unit_name, definition) units_found.add(unit_name) iteration = 0 else: @@ -230,7 +230,6 @@ def _make_pint_unit_definition(self, units_name, unit_attributes): """ full_unit_expr = [] - offset = None # For each of the elements for this unit definition for unit_element in unit_attributes: @@ -254,14 +253,14 @@ def _make_pint_unit_definition(self, units_name, unit_attributes): if 'multiplier' in unit_element: expr = '(%s * %s)' % (unit_element['multiplier'], expr) - if 'offset' in unit_element: - offset = unit_element['offset'] + if 'offset' in unit_element and unit_element['offset'].strip() != '0': + raise ValueError('Offsets in units are not supported!') # Collect/add this particular definition full_unit_expr.append(expr) # Join together all the parts of the unit expression and return - return '*'.join(full_unit_expr), offset + return '*'.join(full_unit_expr) def _add_components(self, model): """ diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index 2766edb9..24560807 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -140,7 +140,7 @@ def __init__(self, store=None): self.Unit = self._registry.Unit self.Quantity = self._registry.Quantity - def add_unit(self, name, expression, offset=None): + def add_unit(self, name, expression): """Adds a unit called ``name`` to the unit store, as defined by the string ``expression``. For example:: @@ -168,14 +168,9 @@ def add_unit(self, name, expression, offset=None): # To test if this is a dimensionless unit, parse the string as a Quantity and check if it's dimensionless quantity = self._registry.parse_expression(expression) if quantity.units == self._registry.dimensionless: - # offsets are not supported for dimensionless - if offset is not None: - raise OffsetNotSupportedError(expression + '; offset: ' + offset) definition = UnitDefinition(qname, '', (), ScaleConverter(quantity.to(self._registry.dimensionless))) else: definition = qname + '=' + expression - if offset is not None and offset.strip() != '0': # just warn and ignore - logger.warning('Offsets in unit definitions are not supported and are ignored!') # Add to registry self._registry.define(definition) @@ -904,15 +899,3 @@ class UnitConversionError(UnitError): def __init__(self, expression, from_units, to_units): self.expression = expression self.message = 'Cannot convert units from {} to {}'.format(from_units, to_units) - - -class OffsetNotSupportedError(UnitError): - """Represents failure to convert between incompatible units. - - :param expression: the Sympy expression in which the error occurred - :param from_unit: the units the expression is in - :param to_unit: the units we tried to convert to - """ - def __init__(self, expression): - self.expression = expression - self.message = 'Offsets for dimensionless units are not supported' diff --git a/tests/test_parser.py b/tests/test_parser.py index 35966f8c..f70379db 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -5,7 +5,6 @@ import sympy from cellmlmanip import parser -from cellmlmanip.units import OffsetNotSupportedError from .shared import check_left_right_units_equal, load_model @@ -352,16 +351,12 @@ def test_dimensionless_multiplier(self): assert cf1 == 1 / cf2, str([cf1, cf2]) def test_dimensionless_offset(self): - with pytest.raises(OffsetNotSupportedError): + with pytest.raises(ValueError): load_model('dimensionless_offset.cellml') def test_offset(self, caplog): - model = load_model('test_offset.cellml') - assert 'Offsets in unit definitions are not supported and are ignored!' in caplog.text - assert sorted(map(str, model.variables())) == ['A$x', 'B$y'] - meter = model.units.get_unit('meter') - offsetmeter = model.units.get_unit('offsetmeter') - assert model.units.is_equivalent(meter, offsetmeter) + with pytest.raises(ValueError): + load_model('test_offset.cellml') def test_offset_0(self, caplog): model = load_model('test_offset_0.cellml') From 6fa9ee85b58153ee7945e7795f9f931d62cc8079 Mon Sep 17 00:00:00 2001 From: Michael Clerx Date: Wed, 28 Sep 2022 14:11:37 +0100 Subject: [PATCH 06/14] Update RELEASE.md --- RELEASE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE.md b/RELEASE.md index 8d715c2a..1c7b89e1 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -2,7 +2,7 @@ - Fixed a bug in the parser where equations in a piecewise containing a boolean caused parsing errors. see https://github.com/ModellingWebLab/cellmlmanip/issues/350 -- Fixes errors dealing with dimensionless units which have a multiplier or exponent and added an error for offset units (where the offset is not 0) as those are not supported. +- Fixed errors dealing with dimensionless units which have a multiplier or exponent, and added an error for offset units (where the offset is not 0) as those are not supported. see https://github.com/ModellingWebLab/cellmlmanip/issues/351 # Release 0.3.4 From 34e9d264b9a418d9695e7017b8044baff7a8ae2d Mon Sep 17 00:00:00 2001 From: Dr Maurice Hendrix <52317399+MauriceHendrix@users.noreply.github.com> Date: Wed, 28 Sep 2022 15:49:24 +0100 Subject: [PATCH 07/14] Update dimensionless_offset.cellml --- tests/cellml_files/dimensionless_offset.cellml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/cellml_files/dimensionless_offset.cellml b/tests/cellml_files/dimensionless_offset.cellml index 39a4d370..c484219d 100644 --- a/tests/cellml_files/dimensionless_offset.cellml +++ b/tests/cellml_files/dimensionless_offset.cellml @@ -1,6 +1,5 @@ - - + @@ -16,4 +15,4 @@ - \ No newline at end of file + From b7337c588ceeff826a4df30e6da71f08ec55615e Mon Sep 17 00:00:00 2001 From: Dr Maurice Hendrix <52317399+MauriceHendrix@users.noreply.github.com> Date: Wed, 28 Sep 2022 15:49:40 +0100 Subject: [PATCH 08/14] Update test_offset.cellml --- tests/cellml_files/test_offset.cellml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/cellml_files/test_offset.cellml b/tests/cellml_files/test_offset.cellml index fbb813d3..e37cc04d 100644 --- a/tests/cellml_files/test_offset.cellml +++ b/tests/cellml_files/test_offset.cellml @@ -1,6 +1,5 @@ - - + @@ -16,4 +15,4 @@ - \ No newline at end of file + From eedf60aa9713df82bd4b6f8eff695482f23a5d66 Mon Sep 17 00:00:00 2001 From: Dr Maurice Hendrix <52317399+MauriceHendrix@users.noreply.github.com> Date: Wed, 28 Sep 2022 15:49:56 +0100 Subject: [PATCH 09/14] Update test_offset_0.cellml --- tests/cellml_files/test_offset_0.cellml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/cellml_files/test_offset_0.cellml b/tests/cellml_files/test_offset_0.cellml index d1267e28..6da61ea1 100644 --- a/tests/cellml_files/test_offset_0.cellml +++ b/tests/cellml_files/test_offset_0.cellml @@ -1,6 +1,5 @@ - - + @@ -16,4 +15,4 @@ - \ No newline at end of file + From 89d7ecd75823018831648be3de7c5c7c04a92b44 Mon Sep 17 00:00:00 2001 From: Dr Maurice Hendrix <52317399+MauriceHendrix@users.noreply.github.com> Date: Wed, 28 Sep 2022 15:51:01 +0100 Subject: [PATCH 10/14] Update tests/cellml_files/test_offset.cellml Co-authored-by: Michael Clerx --- tests/cellml_files/test_offset.cellml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/cellml_files/test_offset.cellml b/tests/cellml_files/test_offset.cellml index e37cc04d..0f43cab7 100644 --- a/tests/cellml_files/test_offset.cellml +++ b/tests/cellml_files/test_offset.cellml @@ -6,13 +6,6 @@ - + - - - - - - - From 6ac80c907244e4f634806acf67dc8285ae2ddb2a Mon Sep 17 00:00:00 2001 From: Dr Maurice Hendrix <52317399+MauriceHendrix@users.noreply.github.com> Date: Wed, 28 Sep 2022 15:51:29 +0100 Subject: [PATCH 11/14] Update test_offset.cellml --- tests/cellml_files/test_offset.cellml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cellml_files/test_offset.cellml b/tests/cellml_files/test_offset.cellml index 0f43cab7..0f888780 100644 --- a/tests/cellml_files/test_offset.cellml +++ b/tests/cellml_files/test_offset.cellml @@ -1,5 +1,5 @@ - + From 854d9489bd5dcb8fd80615c0a555bde73259764d Mon Sep 17 00:00:00 2001 From: Maurice Hendrix Date: Wed, 28 Sep 2022 16:15:20 +0100 Subject: [PATCH 12/14] fixing dimensionless conversion accprding to michael's comments --- cellmlmanip/parser.py | 3 +- cellmlmanip/units.py | 2 +- .../dimensionless_multiplier.cellml | 3 +- .../dimensionless_multiplier2.cellml | 8 ++++++ tests/cellml_files/test_offset_0.cellml | 2 +- tests/test_parser.py | 28 +++++++++++++++---- 6 files changed, 35 insertions(+), 11 deletions(-) create mode 100644 tests/cellml_files/dimensionless_multiplier2.cellml diff --git a/cellmlmanip/parser.py b/cellmlmanip/parser.py index bc07781a..410fbbd6 100644 --- a/cellmlmanip/parser.py +++ b/cellmlmanip/parser.py @@ -253,7 +253,8 @@ def _make_pint_unit_definition(self, units_name, unit_attributes): if 'multiplier' in unit_element: expr = '(%s * %s)' % (unit_element['multiplier'], expr) - if 'offset' in unit_element and unit_element['offset'].strip() != '0': + if 'offset' in unit_element and (not unit_element['offset'].strip().isnumeric() or + int(unit_element['offset']) != 0): raise ValueError('Offsets in units are not supported!') # Collect/add this particular definition diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index 24560807..810bdb7e 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -293,7 +293,7 @@ def convert(self, quantity, unit): # Trying to convert dimensionless gives an error but we can convert to it if quantity.units == self._registry.dimensionless: quantity, unit = 1 * unit, quantity.units - return 1 / quantity.to(unit) + return quantity.magnitude / quantity.to(unit) return quantity.to(unit) def add_conversion_rule(self, from_unit, to_unit, rule): diff --git a/tests/cellml_files/dimensionless_multiplier.cellml b/tests/cellml_files/dimensionless_multiplier.cellml index 13d12c98..99831e93 100644 --- a/tests/cellml_files/dimensionless_multiplier.cellml +++ b/tests/cellml_files/dimensionless_multiplier.cellml @@ -1,6 +1,5 @@ - - + diff --git a/tests/cellml_files/dimensionless_multiplier2.cellml b/tests/cellml_files/dimensionless_multiplier2.cellml new file mode 100644 index 00000000..1078f309 --- /dev/null +++ b/tests/cellml_files/dimensionless_multiplier2.cellml @@ -0,0 +1,8 @@ + + + + + + + \ No newline at end of file diff --git a/tests/cellml_files/test_offset_0.cellml b/tests/cellml_files/test_offset_0.cellml index 6da61ea1..86856dbe 100644 --- a/tests/cellml_files/test_offset_0.cellml +++ b/tests/cellml_files/test_offset_0.cellml @@ -1,5 +1,5 @@ - + diff --git a/tests/test_parser.py b/tests/test_parser.py index f70379db..41970895 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -337,8 +337,8 @@ def test_dimensionless_exp(self): assert sorted(map(str, model.variables())) == ['A$x', 'B$y'] dimensionless = model.units.get_unit('dimensionless') hyper_dimensionless = model.units.get_unit('hyper_dimensionless') - cf1 = model.units.convert(1 * dimensionless, hyper_dimensionless) - cf2 = model.units.convert(1 * hyper_dimensionless, dimensionless) + cf1 = model.units.convert(1 * dimensionless, hyper_dimensionless).magnitude + cf2 = model.units.convert(1 * hyper_dimensionless, dimensionless).magnitude assert cf1 == cf2 == 1 def test_dimensionless_multiplier(self): @@ -346,17 +346,33 @@ def test_dimensionless_multiplier(self): assert sorted(map(str, model.variables())) == ['A$x', 'B$y'] dimensionless = model.units.get_unit('dimensionless') halves = model.units.get_unit('halves') - cf1 = model.units.convert(1 * dimensionless, halves) - cf2 = model.units.convert(1 * halves, dimensionless) - assert cf1 == 1 / cf2, str([cf1, cf2]) + cf1 = model.units.convert(1 * dimensionless, halves).magnitude + cf2 = model.units.convert(1 * halves, dimensionless).magnitude + assert cf1 == 2 + assert cf2 == 0.5 + + cf1 = model.units.convert(2 * dimensionless, halves).magnitude + cf2 = model.units.convert(2 * halves, dimensionless).magnitude + assert cf1 == 2 + assert cf2 == 0.5 + + def test_dimensionless_multiplier2(self): + model = load_model('dimensionless_multiplier2.cellml') + meter = model.units.get_unit('meter') + half_meter = model.units.get_unit('half_meter') + cf1 = model.units.convert(2 * meter, half_meter).magnitude + cf2 = model.units.convert(3 * half_meter, meter).magnitude + assert cf1 == 4.0 + assert cf2 == 1.5 def test_dimensionless_offset(self): with pytest.raises(ValueError): load_model('dimensionless_offset.cellml') def test_offset(self, caplog): - with pytest.raises(ValueError): + with pytest.raises(ValueError) as value_info: load_model('test_offset.cellml') + assert 'Offsets in units are not supported!' in str(value_info) def test_offset_0(self, caplog): model = load_model('test_offset_0.cellml') From 95a5108156e2ce4778b9661a5c42d273626ea1cf Mon Sep 17 00:00:00 2001 From: Maurice Hendrix Date: Wed, 28 Sep 2022 16:20:20 +0100 Subject: [PATCH 13/14] remove unneeded extra test --- tests/test_parser.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/test_parser.py b/tests/test_parser.py index 41970895..d1bc0c26 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -351,11 +351,6 @@ def test_dimensionless_multiplier(self): assert cf1 == 2 assert cf2 == 0.5 - cf1 = model.units.convert(2 * dimensionless, halves).magnitude - cf2 = model.units.convert(2 * halves, dimensionless).magnitude - assert cf1 == 2 - assert cf2 == 0.5 - def test_dimensionless_multiplier2(self): model = load_model('dimensionless_multiplier2.cellml') meter = model.units.get_unit('meter') From 6d1d7e219133bcba750d60cacff6fc2462a9f133 Mon Sep 17 00:00:00 2001 From: Dr Maurice Hendrix <52317399+MauriceHendrix@users.noreply.github.com> Date: Wed, 28 Sep 2022 16:53:52 +0100 Subject: [PATCH 14/14] Update cellmlmanip/units.py Co-authored-by: Michael Clerx --- cellmlmanip/units.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index 810bdb7e..a706c8a4 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -290,7 +290,7 @@ def convert(self, quantity, unit): """ assert isinstance(quantity, self._registry.Quantity) assert isinstance(unit, self._registry.Unit) - # Trying to convert dimensionless gives an error but we can convert to it + # Trying to convert FROM dimensionless gives an error but we can convert TO it if quantity.units == self._registry.dimensionless: quantity, unit = 1 * unit, quantity.units return quantity.magnitude / quantity.to(unit)