cellml / libcellml

Repository for libCellML development.
https://libcellml.org
Apache License 2.0
17 stars 21 forks source link

Generator: initialise external variables and fix our compute methods #961

Closed agarny closed 3 years ago

agarny commented 3 years ago

Fixes #958.

agarny commented 3 years ago

@hsorby and @nickerso: I believe this is now ready to be reviewed.

hsorby commented 3 years ago

I'm interested to know why the external variables get printed in reverse numerical order. Everything else gets printed in numerical order (where possible). In the HH1952 model for instance in the compute_variables method we get

variables[17] = external_variable(voi, states, variables, 17)
variable[10] = external_variable(voi, states, variables, 10)

Why isn't the order the other way around?

agarny commented 3 years ago

I'm interested to know why the external variables get printed in reverse numerical order. Everything else gets printed in numerical order (where possible). In the HH1952 model for instance in the compute_variables method we get

variables[17] = external_variable(voi, states, variables, 17)
variable[10] = external_variable(voi, states, variables, 10)

Why isn't the order the other way around?

It's not in reverse numerical order. As you can tell, both v10 and v17 have been flagged as external variables, BUT... on top of that, v10 has been flagged as being dependent on v17, hence the order in which they are being computed.

agarny commented 3 years ago

Looks good to me. I wonder (but haven't looked yet) if we have a test similar to cellGeometryModelWithExternalVariables but setting variables from a different model as external variables?

I am not aware of such a test and, indeed, I believe it would only make sense if we were to actually run a model with external variables, which we don't (at the moment). At the moment, we "just" generate code for models with external variables. So, we don't address how those external variables are "computed".

nickerso commented 3 years ago

I am not aware of such a test and, indeed, I believe it would only make sense if we were to actually run a model with external variables, which we don't (at the moment). At the moment, we "just" generate code for models with external variables. So, we don't address how those external variables are "computed".

was more just to check if the analyser correctly determines the variables are not in the model being analysed so have no impact on the generated code. (Separate issue, just mentioning it here while thinking about this PR)