Repository: ModellingWebLab/cellmlmanip

Issue:
Improve how initial values and parameters are handled
Basically, once parsing is done, initial values should only appear for state variables, and everything else should be represented as actual assignment equations. To support this, we can adapt how we store equations internally to the `Model` so we have some extra lookup maps.

* [x] Store a map from LHS to equation, so it's quick to look up the definition of a variable
    * Where the LHS is a derivative, also add the dependent variable in the map? Or just in the parser? Or in a separate map? I think a separate map, but open to suggestions.
* [x] `Model.add_equation` should maintain these maps (similarly `remove_equation`)
* [x] After the parser has processed all equations, it goes through all variables with initial_value
    * If they're in the 'defined by ODE' map, it leaves them alone
    * Otherwise it adds a defining equation with just a number on the RHS, and removes the initial_value
* [x] Update `get_derivative_symbols` to use the ODE map
* [x] Don't add excess metadata to graph (#180), but *do* ensure parameters are properly marked (so every equation where RHS is just a number should result in LHS marked as parameter)
* [x] Use the maps in units conversion logic, for quicker lookups of defining equations
* [x] And a related minor refactoring: remove the `get_initial_value` method, since you can just do `symbol.initial_value`

Possibly subsumes (some of) #135 and #120. Maybe #169 #130 #180?

I don't think we necessarily want to prioritise doing this before the next bits of weblab-fc work - unless we think it'll make doing that work easier!

Thoughts @skeating @MauriceHendrix ?

Discussion:

Patch:
diff --git a/cellmlmanip/model.py b/cellmlmanip/model.py
index feb8bf6..25a40d6 100644
--- a/cellmlmanip/model.py
+++ b/cellmlmanip/model.py
@@ -63,6 +63,12 @@ class Model(object):
         # An RDF graph containing further meta data
         self.rdf = rdflib.Graph()
 
+        # Map from VariableDummy to defining equation, where the variable is defined by a simple equation
+        self._var_definition_map = {}
+
+        # Map from VariableDummy to defining equation, where the variable is defined by an ODE
+        self._ode_definition_map = {}
+
     def add_unit(self, name, attributes=None, base_units=False):
         """
         Adds a unit of measurement to this model, with a given ``name`` and list of ``attributes``.
@@ -79,7 +85,7 @@ class Model(object):
         else:
             self.units.add_custom_unit(name, attributes)
 
-    def add_equation(self, equation):
+    def add_equation(self, equation, check_duplicates=True):
         """
         Adds an equation to this model.
 
@@ -89,11 +95,38 @@ class Model(object):
         :meth:`add_number()`, :meth:`add_variable()`, or :meth:`get_symbol_by_ontology_term()`.
 
         :param equation: A ``sympy.Eq`` object.
+        :param check_duplicates: whether to check that the equation's LHS is not already defined
         """
         assert isinstance(equation, sympy.Eq), 'The argument `equation` must be a sympy.Eq.'
         self.equations.append(equation)
+        lhs = equation.lhs
+        if lhs.is_Derivative:
+            state_var = lhs.free_symbols.pop()
+            if check_duplicates:
+                self._check_duplicate_definitions(state_var, equation)
+            self._ode_definition_map[state_var] = equation
+        elif isinstance(lhs, VariableDummy):
+            if check_duplicates:
+                self._check_duplicate_definitions(lhs, equation)
+            self._var_definition_map[lhs] = equation
+        else:
+            raise ValueError('Equation LHS should be a derivative or variable, not {}'.format(lhs))
         self._invalidate_cache()
 
+    def _check_duplicate_definitions(self, var, equation):
+        """
+        Assert that a variable doesn't have an existing definition.
+
+        :param var: the VariableDummy, either a state var or normal var
+        :param equation: the new definition being added
+        """
+        if var in self._ode_definition_map:
+            raise ValueError('The variable {} is defined twice ({} and {})'.format(
+                var, equation, self._ode_definition_map[var]))
+        if var in self._var_definition_map:
+            raise ValueError('The variable {} is defined twice ({} and {})'.format(
+                var, equation, self._var_definition_map[var]))
+
     def add_number(self, value, units):
         """
         Creates and returns a :class:`NumberDummy` to represent a number with units in sympy expressions.
@@ -148,6 +181,25 @@ class Model(object):
 
         return var
 
+    def transform_constants(self):
+        """
+        Called by CellML parser to standardise handling of 'constants'.
+
+        Once this has been called, the only variables with an initial_value attribute will be state variables,
+        and the initial value will do what it implies - hold the value the state variable should take at t=0.
+
+        Non state variables with an initial value are actually just constants. For consistent processing later
+        on we add equations defining them, and remove the initial_value attribute.
+        """
+        for var in self._name_to_symbol.values():
+            if var in self._ode_definition_map:
+                assert var.initial_value is not None, 'State variable {} has no initial_value set'.format(var)
+            elif var.initial_value is not None:
+                value = var.initial_value
+                eq = sympy.Eq(var, sympy.Float(value))
+                self.add_equation(eq)
+                var.initial_value = None
+
     def connect_variables(self, source_name: str, target_name: str):
         """Given the source and target component and variable, create a connection by assigning
         the symbol from the source to the target. If units are not the same, it will add an equation
@@ -174,13 +226,28 @@ class Model(object):
             raise ValueError('Target already assigned to %s before assignment to %s' %
                              (target.assigned_to, source.assigned_to))
 
+        if target in self._var_definition_map:
+            raise ValueError('Multiple definitions for {} ({} and {})'.format(
+                target, source, self._var_definition_map[target]))
+
         # If source/target variable is in the same unit
         if source.units == target.units:
             # Direct substitution is possible
             target.assigned_to = source.assigned_to
-            # everywhere the target variable is used, replace with source variable
+            # everywhere the target variable is used, replace with source variable,
+            # updating the definition maps accordingly
             for index, equation in enumerate(self.equations):
-                self.equations[index] = equation.xreplace({target: source.assigned_to})
+                self.equations[index] = new_eq = equation.xreplace({target: source.assigned_to})
+                if equation.lhs.is_Derivative:
+                    state_var = equation.lhs.free_symbols.pop()
+                    assert state_var in self._ode_definition_map
+                    self._ode_definition_map[state_var] = new_eq
+                else:
+                    assert equation.lhs in self._var_definition_map
+                    self._var_definition_map[equation.lhs] = new_eq
+            if target in self._ode_definition_map:
+                self._ode_definition_map[source.assigned_to] = self._ode_definition_map[target]
+                del self._ode_definition_map[target]
 
         # Otherwise, this connection requires a conversion
         else:
@@ -191,7 +258,7 @@ class Model(object):
             factor_dummy = self.add_number(factor, target.units / source.units)
 
             # Add an equations making the connection with the required conversion
-            self.equations.append(sympy.Eq(target, source.assigned_to * factor_dummy))
+            self.add_equation(sympy.Eq(target, source.assigned_to * factor_dummy))
 
             logger.info('Connection req. unit conversion: %s', self.equations[-1])
 
@@ -287,7 +354,7 @@ class Model(object):
         """Returns a list of state variables found in the given model graph.
         The list is ordered by appearance in the cellml document.
         """
-        state_symbols = [v.args[0] for v in self.get_derivative_symbols()]
+        state_symbols = list(self._ode_definition_map.keys())
         return sorted(state_symbols, key=lambda state_var: state_var.order_added)
 
     def get_free_variable_symbol(self):
@@ -439,16 +506,12 @@ class Model(object):
 
         equation_count = 0
 
-        # Add a node for every variable in the model, and set additional variable meta data
+        # Add a node for every variable in the model, and set variable types
         for equation in self.equations:
             equation_count += 1
 
-            # Determine LHS.
-            lhs = equation.lhs
-            if not (lhs.is_Derivative or isinstance(lhs, VariableDummy)):
-                raise RuntimeError('DAEs are not supported. All equations must be of form `x = ...` or `dx/dt = ...')
-
             # Add the lhs symbol of the equation to the graph
+            lhs = equation.lhs
             graph.add_node(lhs, equation=equation)
 
             # Update variable meta data based on the variable's role in the model
@@ -460,8 +523,10 @@ class Model(object):
                 # Get the free symbol and update the variable information
                 free_symbol = lhs.variables[0]
                 free_symbol.type = 'free'
+            elif equation.rhs.is_number:
+                lhs.type = 'parameter'
             else:
-                lhs.type = None
+                lhs.type = 'computed'
 
         # Sanity check: none of the lhs have the same hash
         assert len(graph.nodes) == equation_count
@@ -484,11 +549,7 @@ class Model(object):
                     graph.add_node(rhs, equation=None, variable_type=rhs.type)
                     graph.add_edge(rhs, lhs)
                 else:
-                    # this variable is a parameter - add to graph and connect to lhs
-                    rhs.type = 'parameter'
-                    dummy = self.add_number(rhs.initial_value, rhs.units)
-                    graph.add_node(rhs, equation=sympy.Eq(rhs, dummy), variable_type='parameter')
-                    graph.add_edge(rhs, lhs)
+                    assert False, 'Unexpected symbol {} on RHS'.format(rhs)  # pragma: no cover
 
             # check that the free  and state variables are defined as a node
             if lhs.is_Derivative:
@@ -499,16 +560,11 @@ class Model(object):
                 if state_symbol not in graph.nodes:
                     graph.add_node(state_symbol, equation=None, variable_type=state_symbol.type)
 
-        # Add more meta-data to the graph
+        # Store variable type in the graph too
         for variable in graph.nodes:
             if not variable.is_Derivative:
-                for key in ['cmeta_id', 'name', 'units']:
-                    if getattr(variable, key):
-                        graph.nodes[variable][key] = getattr(variable, key)
-                if variable.type == 'state' and variable.initial_value is not None:
-                    graph.nodes[variable]['initial_value'] = sympy.Float(variable.initial_value)
-                if variable.type is not None:
-                    graph.nodes[variable]['variable_type'] = variable.type
+                assert variable.type is not None
+                graph.nodes[variable]['variable_type'] = variable.type
 
         # Cache graph and return
         self._graph = graph
@@ -583,23 +639,17 @@ class Model(object):
         """ Returns the evaluated value of the given symbol's RHS. """
         return float(self.graph.nodes[symbol]['equation'].rhs.evalf())
 
-    def get_initial_value(self, symbol):
-        """
-        Returns the initial value of the given symbol.
-
-        :param symbol: Sympy Dummy object of required symbol
-        :return: float of initial value
-        """
-        return symbol.initial_value
-
-    def find_symbols_and_derivatives(self, expression):
+    def find_symbols_and_derivatives(self, expressions):
         """ Returns a set containing all symbols and derivatives referenced in a list of expressions.
 
-        :param expression: a list of expressions to get symbols for.
+        Note that we can't just use ``.atoms(VariableDummy, sympy.Derivative)`` for this, because it
+        will return the state and free variables from inside derivatives, which is not what we want.
+
+        :param expressions: an iterable of expressions to get symbols for.
         :return: a set of symbols and derivative objects.
         """
         symbols = set()
-        for expr in expression:
+        for expr in expressions:
             if expr.is_Derivative or isinstance(expr, VariableDummy):
                 symbols.add(expr)
             else:
@@ -617,6 +667,13 @@ class Model(object):
         except ValueError:
             raise KeyError('Equation not found in model ' + str(equation))
 
+        # Update dependency maps
+        lhs = equation.lhs
+        if lhs.is_Derivative:
+            del self._ode_definition_map[lhs.free_symbols.pop()]
+        else:
+            del self._var_definition_map[lhs]
+
         # Invalidate cached equation graphs
         self._invalidate_cache()
 
@@ -711,11 +768,10 @@ class Model(object):
         # if free variable
         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] == original_variable:
-                        new_derivatives.append(self._convert_free_variable_deriv(equation, new_variable, cf))
+            for ode in [eq for v, eq in sorted(self._ode_definition_map.items(),
+                                               key=lambda v_eq: v_eq[0].order_added)]:
+                if ode.args[0].args[1].args[0] == original_variable:
+                    new_derivatives.append(self._convert_free_variable_deriv(ode, new_variable, cf))
 
         # replace any instances of derivative of rhs of other eqns with new derivative variable
         for new_derivative in new_derivatives:
@@ -751,13 +807,13 @@ class Model(object):
         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 equation in self.equations.copy():
             for argument in equation.rhs.atoms(sympy.Derivative):
                 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)
+                    self.add_equation(new_eqn)
                     break
 
     def _create_new_deriv_variable_and_equation(self, eqn, derivative_variable):
@@ -775,14 +831,14 @@ class Model(object):
 
         # 2. create new equation and remove original
         expression = sympy.Eq(new_deriv_variable, eqn.args[1])
-        self.add_equation(expression)
         self.remove_equation(eqn)
+        self.add_equation(expression)
 
         return new_deriv_variable
 
     def _convert_free_variable_deriv(self, eqn, new_variable, cf):
         """
-        Create relevant variables/equations when converting a free variable  within a 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]
@@ -807,13 +863,7 @@ class Model(object):
         :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)
-        eqn = None
-        for equation in self.equations:
-            if equation.args[0].is_Derivative:
-                if equation.args[0].args[0] == original_variable:
-                    eqn = equation
-                    break
+        eqn = self._ode_definition_map[original_variable]
 
         # get free variable symbol
         # units [x]
@@ -858,21 +908,20 @@ class Model(object):
             # if direction is input; original var will be replaced by equation so do not need to store initial value
             original_variable.initial_value = None
 
-            # find the equation for the original variable: orig_var = rhs
+            # find the equation for the original variable (if any): orig_var = rhs
             # remove equation from model
-            # add eqn for new variabale in terms of rhs of equation
+            # add eqn for new variable 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)
-                    break
+            original_equation = self._var_definition_map.get(original_variable)
+            if original_equation is not None:
+                new_equation = sympy.Eq(new_variable, original_equation.args[1] * cf)
+                self.remove_equation(original_equation)
+                self.add_equation(new_equation)
 
             # 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)
+            self.add_equation(expression, check_duplicates=False)
         else:
             # 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]
@@ -882,14 +931,12 @@ class Model(object):
         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')
-                break
+        """Function to create a unique name within the model.
+        :param str name: Suggested unique name
+        :return str: guaranteed unique name
+        """
+        if name in self._name_to_symbol:
+            name = self._get_unique_name(name + '_a')
         return name
 
 
diff --git a/cellmlmanip/parser.py b/cellmlmanip/parser.py
index 336b3c2..c723c2a 100644
--- a/cellmlmanip/parser.py
+++ b/cellmlmanip/parser.py
@@ -101,6 +101,9 @@ class Parser(object):
         self._add_relationships(model_xml)
         self._add_connection(model_xml)
 
+        # Canonicalise representation
+        self.model.transform_constants()
+
         return self.model
 
     @staticmethod
@@ -342,10 +345,6 @@ class Parser(object):
         # those variables we know are source variables
         connections_to_process = deque(connect_from_to)
 
-        # For testing: shuffle the order of connections TODO: REMOVE!
-        from random import shuffle
-        shuffle(connections_to_process)
-
         # keep processing the list of connections until we've done them all
         unchanged_loop_count = 0
         while connections_to_process:


Test patch:
diff --git a/tests/test_model_functions.py b/tests/test_model_functions.py
index 6c1a42d..5042524 100644
--- a/tests/test_model_functions.py
+++ b/tests/test_model_functions.py
@@ -50,14 +50,9 @@ class TestModelFunctions():
 
     def test_graph_for_dae(self):
         """ Checks if writing a DAE in a model raises an exceptions. """
-
-        # Parsing should be OK
         path = os.path.join(os.path.dirname(__file__), 'cellml_files', '4.algebraic_ode_model.cellml')
-        model = parser.Parser(path).parse()
-
-        # But equation graph will raise error (if accessed)
-        with pytest.raises(RuntimeError, match='DAEs are not supported'):
-            model.graph
+        with pytest.raises(ValueError, match='Equation LHS should be a derivative or variable'):
+            parser.Parser(path).parse()
 
     #######################################################################
     # this section contains tests for each get_XXX function on Model
@@ -115,13 +110,6 @@ class TestModelFunctions():
         free_variable_symbol = aslanidi_model.get_free_variable_symbol()
         assert free_variable_symbol.name == 'environment$time'
 
-    # also tested in test_aslanidi
-    def test_get_initial_value(self, aslanidi_model):
-        """ Tests Model.get_initial_value() works correctly. """
-
-        membrane_voltage = aslanidi_model.get_symbol_by_ontology_term(shared.OXMETA, "membrane_voltage")
-        assert(aslanidi_model.get_initial_value(membrane_voltage) == -80.0)
-
     def test_get_derivative_symbols(self, basic_model):
         """ Tests Model.get_derivative_symbols() works correctly. """
 
@@ -352,7 +340,7 @@ class TestModelFunctions():
         assert len(model.equations) == 1
         # so we are adding
         # newvar2 = newvar + newvar1
-        # but need to also add newvar1 = 2; newvar = 2 in order or the graph to resolve correctly
+        # but need to also add newvar1 = 2; newvar = 2 in order for the graph to resolve correctly
         model.add_variable(name='newvar', units='mV')
         symbol = model.get_symbol_by_name('newvar')
         model.add_variable(name='newvar1', units='mV')
@@ -372,6 +360,26 @@ class TestModelFunctions():
         assert eqn[2].lhs == symbol2
         assert eqn[2].rhs == sp.Add(symbol, symbol1)
 
+        # Now check some error cases
+        with pytest.raises(ValueError, match='The variable newvar is defined twice'):
+            # Redefining normal var
+            model.add_equation(sp.Eq(symbol, 3.0))
+
+        sv1 = model.get_symbol_by_cmeta_id('sv11')
+        with pytest.raises(ValueError, match='The variable env_ode\\$sv1 is defined twice'):
+            # Redefining state var
+            model.add_equation(sp.Eq(sv1, 2.0))
+
+        sv1_def = model._ode_definition_map[sv1]
+        with pytest.raises(ValueError, match='The variable env_ode\\$sv1 is defined twice'):
+            # Redefining with ODE
+            model.add_equation(sp.Eq(sv1_def.lhs, 1.0))
+
+        # But if we don't check for duplicates these are 'OK'
+        model.add_equation(sp.Eq(symbol, 3.0), check_duplicates=False)
+        model.add_equation(sp.Eq(sv1, 2.0), check_duplicates=False)
+        model.add_equation(sp.Eq(sv1_def.lhs, 1.0), check_duplicates=False)
+
     def test_remove_equation(self, local_hh_model):
         """ Tests the Model.remove_equation method. """
 
@@ -559,6 +567,12 @@ class TestModelFunctions():
         assert new_target.assigned_to == source
         assert len(local_hh_model.equations) == num_eqns
 
+        # Can't connect a variable already defined by an equation to another source
+        newvar2 = local_hh_model.add_variable(name='newvar2', units='dimensionless', public_interface='in')
+        local_hh_model.add_equation(sp.Eq(newvar2, 1.0))
+        with pytest.raises(ValueError, match='Multiple definitions for newvar2'):
+            local_hh_model.connect_variables('sodium_channel$E_Na', 'newvar2')
+
     def test_connect_variable2(self, local_hh_model):
         """ Tests Model.connect_variables() function. """
         num_eqns = len(local_hh_model.equations)
diff --git a/tests/test_parser.py b/tests/test_parser.py
index 6866179..548837a 100644
--- a/tests/test_parser.py
+++ b/tests/test_parser.py
@@ -44,8 +44,8 @@ class TestParser(object):
 
     def test_equations_count(self, simple_ode_model):
         """Tests correct number of equations read and then created where necessary by parser. """
-        # Include 19 equations from model + 2 equations added by parser for unit conversion
-        assert len(simple_ode_model.equations) == 21  # NOTE: determined by eye!
+        # Include 19 equations from model + 2 equations added by parser for unit conversion + 1 constant
+        assert len(simple_ode_model.equations) == 22  # NOTE: determined by eye!
 
     def test_rdf(self, simple_ode_model):
         """ Tests correct number of rdf statements parsed. """
diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py
index 95f0770..e5c84be 100644
--- a/tests/test_unit_conversion.py
+++ b/tests/test_unit_conversion.py
@@ -123,7 +123,7 @@ class TestUnitConversion:
         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.initial_value == 2.0
         assert symbol_a.units == 'mV'
         assert symbol_t.units == 'ms'
         assert len(local_model.equations) == 1
@@ -141,10 +141,10 @@ class TestUnitConversion:
         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.initial_value == 2.0
+        assert symbol_t.initial_value is None
+        assert symbol_x.initial_value is None
+        assert symbol_y.initial_value is None
         assert symbol_a.units == 'mV'
         assert symbol_t.units == 'ms'
         assert symbol_x.units == 'pA'
@@ -160,7 +160,7 @@ class TestUnitConversion:
         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.initial_value == 2.0
         assert symbol_a.units == 'mV'
         assert symbol_t.units == 'ms'
         assert len(multiode_model.equations) == 3
@@ -176,8 +176,8 @@ class TestUnitConversion:
         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.initial_value == 2.0
+        assert symbol_y.initial_value == 3.0
         assert symbol_a.units == 'mV'
         assert symbol_t.units == 'ms'
         assert symbol_y.units == 'mV'
@@ -225,17 +225,17 @@ class TestUnitConversion:
         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.initial_value == 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 not local_model.get_initial_value(symbol_orig)
+        assert symbol_orig.initial_value is None
         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 symbol_derv.initial_value is None
         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)'
@@ -282,7 +282,7 @@ class TestUnitConversion:
         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
+        assert symbol_a.initial_value == 2.0
         assert symbol_a.units == 'mV'
         assert symbol_a.name == 'env_ode$sv1'
         symbol_t = local_model.get_symbol_by_cmeta_id('time')
@@ -345,11 +345,11 @@ class TestUnitConversion:
         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.initial_value == 2.0
+        assert symbol_t.initial_value is None
+        assert symbol_x.initial_value is None
+        assert symbol_y.initial_value is None
+        assert symbol_x_orig.initial_value is None
         assert symbol_a.units == 'mV'
         assert symbol_t.units == 'ms'
         assert symbol_x.units == 'nA'
@@ -403,7 +403,7 @@ class TestUnitConversion:
         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
+        assert symbol_a.initial_value == 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')
@@ -592,11 +592,11 @@ class TestUnitConversion:
         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.initial_value == 2.0
+        assert symbol_t.initial_value is None
+        assert symbol_x.initial_value is None
+        assert symbol_y.initial_value is None
+        assert symbol_x_orig.initial_value is None
         assert symbol_a.units == 'mV'
         assert symbol_t.units == 'ms'
         assert symbol_x.units == 'nA'
@@ -646,14 +646,14 @@ class TestUnitConversion:
         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 not local_model.get_initial_value(symbol_a)
+        assert symbol_a.initial_value is None
         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 symbol_orig.initial_value == 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)'
@@ -697,7 +697,7 @@ class TestUnitConversion:
         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
+        assert symbol_a.initial_value == 2.0
         assert symbol_a.units == 'mV'
         assert symbol_a.name == 'env_ode$sv1'
         symbol_t = local_model.get_symbol_by_cmeta_id('time')
@@ -761,7 +761,7 @@ class TestUnitConversion:
             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.initial_value == 2.0
             assert symbol_a.units == 'mV'
             assert symbol_t.units == 'ms'
             assert silly_names.get_symbol_by_name('env_ode$sv1_converted')
@@ -782,7 +782,7 @@ class TestUnitConversion:
         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.initial_value == 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')
@@ -794,7 +794,7 @@ class TestUnitConversion:
         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 symbol_derv.initial_value is None
         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)'