fixing errors deling with offsets in unit definitions

RELEASE.md

@@ -1,8 +1,10 @@
 # (unreleased)
 - 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
-- Added error message when trying to connect components that do not exist. 
+- Added error message when trying to connect components that do not exist.
 - Added an error for duplicate component names.
+- 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
 - 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

cellmlmanip/parser.py

@@ -253,6 +253,10 @@ 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 (not unit_element['offset'].strip().isnumeric() or
+                                             int(unit_element['offset']) != 0):
+                raise ValueError('Offsets in units are not supported!')
+
             # Collect/add this particular <unit> definition
             full_unit_expr.append(expr)
 

cellmlmanip/units.py

@@ -290,6 +290,10 @@ def convert(self, quantity, unit):
         """
         assert isinstance(quantity, self._registry.Quantity)
         assert isinstance(unit, self._registry.Unit)
+        #  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)
         return quantity.to(unit)
 
     def add_conversion_rule(self, from_unit, to_unit, rule):

tests/cellml_files/dimensionless_exp.cellml

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- CellML Test Suite. https://github.com/MichaelClerx/cellml-validation -->
+<!-- CellML 1.0, 5.2.7: The exponent of dimensionless doesn't matter -->
+<model name="unit_conversion_prefix"
+       xmlns="http://www.cellml.org/cellml/1.0#">
+  <units name="hyper_dimensionless">
+    <unit units="dimensionless" exponent="12" />
+  </units>
+  <component name="A">
+    <variable name="x" units="dimensionless" initial_value="3" public_interface="out" />
+  </component>
+  <component name="B">
+    <variable name="y" units="hyper_dimensionless" public_interface="in" />
+  </component>
+  <connection>
+    <map_components component_1="A" component_2="B" />
+    <map_variables variable_1="x" variable_2="y" />
+  </connection>
+</model>

tests/cellml_files/dimensionless_multiplier.cellml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- Testing dimensionless unit with multiplier -->
+<model name="unit_conversion_dimensionless_multiplier_1"
+       xmlns="http://www.cellml.org/cellml/1.0#">
+  <units name="halves">
+    <unit units="dimensionless" multiplier="0.5" />
+  </units>
+  <component name="A">
+    <variable name="x" units="dimensionless" initial_value="1" public_interface="out" />
+  </component>
+  <component name="B">
+    <variable name="y" units="halves" public_interface="in" />
+  </component>
+  <connection>
+    <map_components component_1="A" component_2="B" />
+    <map_variables variable_1="x" variable_2="y" />
+  </connection>
+</model>

tests/cellml_files/dimensionless_multiplier2.cellml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- Testing more dimensionless unit with multiplier -->
+<model name="unit_conversion_dimensionless_multiplier_1"
+       xmlns="http://www.cellml.org/cellml/1.0#">
+  <units name="half_meter">
+    <unit units="meter" multiplier="0.5" />
+  </units>
+</model>

tests/cellml_files/dimensionless_offset.cellml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- Testing dimensionless unit with offset -->
+<model name="unit_conversion_dimensionless_offset"
+       xmlns="http://www.cellml.org/cellml/1.0#">
+  <units name="biggers">
+    <unit units="dimensionless" offset="-1" />
+  </units>
+  <component name="A">
+    <variable name="x" units="dimensionless" initial_value="3" public_interface="out" />
+  </component>
+  <component name="B">
+    <variable name="y" units="biggers" public_interface="in" />
+  </component>
+  <connection>
+    <map_components component_1="A" component_2="B" />
+    <map_variables variable_1="x" variable_2="y" />
+  </connection>
+</model>

tests/cellml_files/test_offset.cellml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- cellmlmanip test: error should be raised if non-zero offset is used -->
+<model name="unit_conversion_dimensionless_offset"
+       xmlns="http://www.cellml.org/cellml/1.0#">
+  <units name="offsetmeter">
+    <unit units="meter" offset="10" />
+  </units>
+  <component name="A">
+    <variable name="x" units="offsetmeter" initial_value="3" public_interface="out" />
+  </component>
+</model>

tests/cellml_files/test_offset_0.cellml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- cellmlmaniptest: offset of 0 is ignored: does not affect parsing or unit equivalence when connections are made -->
+<model name="unit_conversion_dimensionless_offset"
+       xmlns="http://www.cellml.org/cellml/1.0#">
+  <units name="offsetmeter">
+    <unit units="meter" offset=" 0 " />
+  </units>
+  <component name="A">
+    <variable name="x" units="meter" initial_value="3" public_interface="out" />
+  </component>
+  <component name="B">
+    <variable name="y" units="offsetmeter" public_interface="in" />
+  </component>
+  <connection>
+    <map_components component_1="A" component_2="B" />
+    <map_variables variable_1="x" variable_2="y" />
+  </connection>
+</model>

tests/test_parser.py

@@ -332,6 +332,51 @@ def test_piecewise_booleans_error(self):
         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).magnitude
+        cf2 = model.units.convert(1 * hyper_dimensionless, dimensionless).magnitude
+        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).magnitude
+        cf2 = model.units.convert(1 * 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) 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')
+        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)
+
     def test_err_connect_to_non_existing_component(self):
         with pytest.raises(ValueError) as value_info:
             load_model('err_connect_to_non_existing_component.cellml')