ModellingWebLab / weblab-fc

Functional Curation backend for the Modelling Web Lab
Other
1 stars 0 forks source link

Allow model parameters (constants) to be changed after compilation #213

Open MichaelClerx opened 4 years ago

MichaelClerx commented 4 years ago

For fitting, it is essential that we can change model constants (parameters) after model compilation (and protocol association).

It looks to me like the model (generated from weblab_model.pyx, extends CvodeSolver) currently does not allow its parameters to be changed.

Protocol inputs end up in a 'parameters' list, which is referenced by the rhs:

cdef int _evaluate_rhs(...):
    ...
    # Mathematics
    {%- for eq in rhs_equations %}
    {%- if eq.parameter_index is none %}
    cdef double {{ eq.lhs }} = {{ eq.rhs }}
    {%- else %}
    cdef double {{ eq.lhs }} = parameters[{{ eq.parameter_index }}]
    {%- endif %}
    {%- endfor %}

so it points all constants towards the cvode model parameters array (a numpy array) (Although the syntax looks broken? parameters is not a global as far as I can see)

So this would allow us to change the parameters post compilation, but I don't see any code that then modifies the model. E.g., in Protocol.set_input:

    def set_input(self, name, value_expr):
        if isinstance(value_expr, V.AbstractValue):
            value = value_expr
        else:
            value = value_expr.evaluate(self.input_env)
        self.input_env.overwrite_definition(name, value)

This updates the input_env, but the input_env is not used by the compiled model (it is used in compiling the model)

We also don't have any tests that change inputs that correspond to model parameters (is that a thing?)

[michael@localhost test]$ grep "set_input" ./ -Ir
./test_python_reproducibility.py:                    proto.set_input(input, fc.language.values.Simple(1000))
./test_nested_protocols.py:    proto.set_input('block_levels', V.Array([0.0, 0.5]))
./test_compact_syntax_parser.py:def test_parsing_imports_with_set_input():
./test_while_loop.py:    proto.set_input('num_iters', E.N(10))
./test_while_loop.py:    proto.set_input('num_iters', E.N(10))
./test_imported_protocols.py:    proto.set_input('s2_intervals', V.Array(intervals))
./test_imported_protocols.py:    proto.set_input('s2_intervals', V.Array(intervals))

👆🏻 these are all fc variables

MichaelClerx commented 4 years ago

Or am I conflating two ideas here @jonc125 ?

jonc125 commented 4 years ago

You are conflating protocol inputs and model inputs. They operate completely differently. Model inputs are set from the protocol via the SetVariable modifier class, using the ModelWrapperEnvironment and the model's parameter_map and parameters attributes.

However, it is true there may be an issue, as the only variables marked as parameters in weblab-fc currently are probably those the protocol specifies as model inputs, which wouldn't necessarily include parameters the fitting process wants to alter! (The Chaste-based implementation was subtly different here, as all constant variables implicitly became parameters.) So we may need an extra hook around the set_model area to provide names of extra parameters from the fitting code?

MichaelClerx commented 4 years ago

Thanks

The Chaste-based implementation was subtly different here, as all constant variables implicitly became parameters.

We can do that again, if you like. The comments here and there already say that (but the code disagrees)

MichaelClerx commented 4 years ago

I think that might be desirable actually, so that we can ask the protocol-with-model for variable objects that have already been unit converted. Otherwise we'd have to tell the protocol before-hand, from a separate model parse...

jonc125 commented 4 years ago

Well, they'd only be unit-converted if you've told cellmlmanip (via the protocol?) what units you want them in surely? But that's a good point, the prior would be specifying parameters in units that may not match the model, so conversions will be needed, so we probably need a way for the fitting code to tell the protocol (before it manipulates the model) that it also needs a given set of variables to be treated as model inputs in specific units.