Adds the spin-up step

[dalonsoa]

Description

Adds the possibility to run spin-up steps before the actual simulation.

Details of PR TBC

Fixes #581

Type of change

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist

• Make sure you've run the pre-commit checks: $ pre-commit run -a
• All tests pass: $ poetry run pytest

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works
• Relevant documentation reviewed and updated

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 27.08333% with 35 lines in your changes missing coverage. Please review.

Project coverage is 94.07%. Comparing base (1de171f) to head (e98c87d).

Files with missing lines	Patch %	Lines
virtual_ecosystem/main.py	26.08%	34 Missing ⚠️
virtual_ecosystem/core/variables.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##           687_fix_static     #714      +/-   ##
==================================================
- Coverage           94.73%   94.07%   -0.66%     
==================================================
  Files                  73       73              
  Lines                4941     4983      +42     
==================================================
+ Hits                 4681     4688       +7     
- Misses                260      295      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dalonsoa]

Maybe is not an issue in real life, but in its current implementation, if a variable is chosen to be carried over from the previous step, it might trigger errors in either de variable system or the the static configuration check.

For example, the following config file runs fine the spin-up but, for the actual simulation it fails with the error below (which was to be expected). I am not sure if this can never happen in a real scenario but I feel this is going to cause trouble.

Input file:

[core]
[[core.spin_up]]
models = ["hydrology"]
timing = {start_date = "2013-01-01", update_interval = "1 month", run_length = "2 years"}
variables = ["air_temperature", "relative_humidity"]
[hydrology]
[abiotic_simple]
[animal]
[litter]
[soil]

Error

ValueError: Variable air_temperature initialised by abiotic_simple already in registry as initialised by data.

[jacobcook1995]

@dalonsoa this looks like a sensible approach to me. I unfortunately don't have much to add beyond that, but I am sure others will have more to say

[vgro]

Maybe is not an issue in real life, but in its current implementation, if a variable is chosen to be carried over from the previous step, it might trigger errors in either de variable system or the the static configuration check.

For example, the following config file runs fine the spin-up but, for the actual simulation it fails with the error below (which was to be expected). I am not sure if this can never happen in a real scenario but I feel this is going to cause trouble.

Input file:

[core]
[[core.spin_up]]
models = ["hydrology"]
timing = {start_date = "2013-01-01", update_interval = "1 month", run_length = "2 years"}
variables = ["air_temperature", "relative_humidity"]
[hydrology]
[abiotic_simple]
[animal]
[litter]
[soil]

Error

ValueError: Variable air_temperature initialised by abiotic_simple already in registry as initialised by data.

What exactly are the 'variables' in this setup?

[alexdewar]

I fear I've lost track of VE development a bit too much to be able to provide too much in the way of useful comments, but it does seem like a sensible approach in general.

Bigger picture, I am wondering if there might be a way to avoid having RUN_VARIABLES_REGISTRY as a global variable in general. If, for example, it's only ever used in the body of a main loop, you could just declare it as a variable in that loop and pass it into the functions that need it, then you'd get a clean slate on every iteration anyway. But I suspect it's probably not that simple 😉

[dalonsoa]

What exactly are the 'variables' in this setup?

They are the variables from that step that will be used in the following step. If we look at the issue this is trying to solve, there you said:

Run models with all frozen apart from Hydrology for 10 years, save soil moisture.

So soil_moisture will be one of these variables.

[vgro]

What exactly are the 'variables' in this setup?

They are the variables from that step that will be used in the following step. If we look at the issue this is trying to solve, there you said:

Run models with all frozen apart from Hydrology for 10 years, save soil moisture.

So soil_moisture will be one of these variables.

Ok, that makes sense. Does the same error come up when you use variables that are more exclusive to the hydrology model, such as soil moisture? air_temperature for example is a bit of a tricky one because it is initialized by the hydrology model but updated by the abiotic model. I can't quite figure out in this spinup functionality what is going on in terms of static etc, is it equivalent to static with one update?

[dalonsoa]

Same problem. soil_moisture is declared to be initialised by the hydrology model, so when it is there already, in the data object, as it has been loaded from the previous step, the variables system complains.

The models that are declared static - all the models except those we are spinning up in the given step - follow the standard rules for static models (first update if no variables to be initialised are in the data object or frozen if all variables are there). For the actual simulation, none of the models are static - unless they were declared as such in the config file.

The bottom line is that we cannot provide some variables as input that were supposed to be initialised by a model. The variables system prevents it. But even if we modify the variables system to allow that, the input value will be overwritten when the model is initialised in the real simulation (where presumably, it won't be static) with whatever it calculates.

So the only way forward I see is setting up some mechanism within the model - or the data object - that prevents creating a new variable if it is already there. But that might lead to inconsistent results if other variables depend on the calculated one, so I'm not sure of the best way forward. It might require a significant refactoring of the model system.

[davidorme]

The models that are declared static - all the models except those we are spinning up in the given step - follow the standard rules for static models (first update if no variables to be initialised are in the data object or frozen if all variables are there). For the actual simulation, none of the models are static - unless they were declared as such in the config file.

And we have another problem there: because we use a simple boolean flag and the data payload to determine the static mode, we cannot at the moment spin the model up in fully-frozen mode and then run the model in update-once mode.

[davidorme]

@vgro and @jacobcook1995 How much of this can we achieve using manual spinup runs using the existing infrastructure?

So:

• define a specific spin up config using the static system and run that
• define a "real" run that points to the spun-up variables output for the required variables

My worry here is that anything we try and develop is going to be brittle until we have a better idea of the actual use cases. If we can develop that understanding more, we'll be able to guide this process. If we can't do this manually now, then I think we think about how to fix those blocks for manual spinup before we go about trying to do it programatically?

[dalonsoa]

@davidorme check Teams. I've pointed this same thing there and suggested a meeting.

[davidorme]

check Teams.

Too many channels 😄