Maximum recursion depth exceeded

[MichaelClerx]

<?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>

[MichaelClerx]

<?xml version="1.0" encoding="UTF-8"?>
<!-- CellML Test Suite. https://github.com/MichaelClerx/cellml-validation -->
<!-- CellML 1.0, 5.2.7: Dimensionless units can have a 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>

[MichaelClerx]

<?xml version="1.0" encoding="UTF-8"?>
<!-- CellML Test Suite. https://github.com/MichaelClerx/cellml-validation -->
<!-- CellML 1.0, 5.2.7: Dimensionless units can have an 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>

[MauriceHendrix]

Do I understand it correctly that those units (dimensionless and demensionless with some power / multiplier / offset) should all be equivalent?

If I try to implement that I run into failing tests. I'm not really sure if this makes any sense :

(chaste_codegen) C:\Users\uczmh2\Desktop\chaste_codegen\cellmlmanip>pytest tests\test_units.py
=================================================================================================================================================== test session starts ====================================================================================================================================================
platform win32 -- Python 3.9.0, pytest-7.1.0, pluggy-1.0.0
rootdir: C:\Users\uczmh2\Desktop\chaste_codegen\cellmlmanip
plugins: cov-3.0.0
collected 59 items

tests\test_units.py ..............................F....F.......................                                                                                                                                                                                                                                       [100%]

========================================================================================================================================================= FAILURES =========================================================================================================================================================
________________________________________________________________________________________________________________________________________ TestConvertingExpressions.test_mul_and_pow ________________________________________________________________________________________________________________________________________

self = <tests.test_units.TestConvertingExpressions object at 0x0000016F44A92AF0>, store = <cellmlmanip.units.UnitStore object at 0x0000016F44A7E610>, cm = <Unit('store4_cm')>, ms = <Unit('store4_ms')>

    def test_mul_and_pow(self, store, cm, ms):
        cmtr = Variable('cmtr', cm)
        y = Variable('y', ms)
        expr = cmtr / y  # Becomes cmtr * (1/y)

        # No conversion
        new_expr = store.convert_expression_recursively(expr, None)
        assert expr is new_expr

        # With conversion
        new_expr = store.convert_expression_recursively(expr, store.get_unit('metre') / store.get_unit('second'))
        assert str(new_expr) == '_10*_cmtr/_y'
        assert new_expr.args[2] is cmtr
        assert new_expr.args[0].args[0] is y

        # With conversion only for exponent
        _4 = Quantity('4', store.get_unit('second'))
        _2 = Quantity('2000', ms)
        expr = cmtr ** (_4 / _2)
        new_expr = store.convert_expression_recursively(expr, None)
>       assert str(new_expr) == '_cmtr**(_1000*_4/_2000)'
E       AssertionError: assert '_cmtr**(_4/_2000)' == '_cmtr**(_1000*_4/_2000)'
E         - _cmtr**(_1000*_4/_2000)
E         ?          ------
E         + _cmtr**(_4/_2000)

tests\test_units.py:618: AssertionError
_______________________________________________________________________________________________________________________________________ TestConvertingExpressions.test_exp_log_trig ________________________________________________________________________________________________________________________________________

self = <tests.test_units.TestConvertingExpressions object at 0x0000016F44ACC550>, store = <cellmlmanip.units.UnitStore object at 0x0000016F44A7E610>, dim = _dim, sec = _sec, ms = <Unit('store4_ms')>

    def test_exp_log_trig(self, store, dim, sec, ms):
        dimensionless = store.get_unit('dimensionless')
        z = Variable('z', ms)

        expr = sp.exp(dim)
        assert store.convert_expression_recursively(expr, dimensionless) is expr

        expr = sp.log(sec / z)
        new_expr = store.convert_expression_recursively(expr, None)
        assert isinstance(new_expr, sp.log)
>       assert str(new_expr) == 'log(_1000*_sec/_z)'
E       AssertionError: assert 'log(_sec/_z)' == 'log(_1000*_sec/_z)'
E         - log(_1000*_sec/_z)
E         ?     ------
E         + log(_sec/_z)

tests\test_units.py:692: AssertionError
================================================================================================================================================= short test summary info ==================================================================================================================================================
FAILED tests/test_units.py::TestConvertingExpressions::test_mul_and_pow - AssertionError: assert '_cmtr**(_4/_2000)' == '_cmtr**(_1000*_4/_2000)'
FAILED tests/test_units.py::TestConvertingExpressions::test_exp_log_trig - AssertionError: assert 'log(_sec/_z)' == 'log(_1000*_sec/_z)'
=============================================================================================================================================== 2 failed, 57 passed in 1.12s ===============================================================================================================================================

(chaste_codegen) C:\Users\uczmh2\Desktop\chaste_codegen\cellmlmanip>

[MichaelClerx]

dimensionless^x should match dimensionless

dimensionless*0.5 is convertible to dimensionless (x2)
(real-world examples arise if you have e.g. a conversion variable x = 1000 [mV per V] in your model somewhere)

dimensionless with some offset should trigger an error saying offsets not supported

[MauriceHendrix]

hhm offsets do not appear to be implemented at all, unless I'm missing something

[MichaelClerx]

Yeah we should have some error (or probably a warning) if anyone sets an offset that isn't zero

[MauriceHendrix]

Yeah we should have some error (or probably a warning) if anyone sets an offset that isn't zero

are you saying offsets aren't allowed in cellml? I thought they were there in https://www.cellml.org/specifications/cellml_1.1/#sec_units
I've had a go at implementing them (exctp for dimenionless) in #354

[MichaelClerx]

They are allowed in cellml but we chose not to implement them in cellmlmanip