feat: Improve model building from schema.

[jetuk]

Combine the loading of parameters and nodes into a single while loop. The intention is that each are attempted to be loaded, falling back if a node or parameter they depend on can not be found. They are then tried again once the load has progressed.

The change allows for the removal of the set_constraints method on Node. The functionality in this method is added into the add_to_model method instead. This means developers need to be a little careful with writing that method.

The main problem is when a storage node depends on a parameter for maximum volume. This requires that parameter to be available (and evaluated) prior to the node in order for the initial volume to work correctly.

Some basic developer documentation on how to write new nodes is added to help with the above.

[jetuk]

I've written this, but having some doubts if it is the correct answer.

My worry is that by removing set_constraints it's a little be up to the node implementation whether a parameter is evaluated before or after the node. The main issue comes because of the need to load max_volume ahead of the node it is used on in order for it to set the initial volume correctly.

I wonder if instead a solution is to keep the separation of adding the node and setting the constraints, but then implement #62 and only allow max_volume be a constant that is evaluated first and never changes. Obviously, this places a stronger restriction by not allowing max_volume to change during a simulation (but I think that is a fairly rare / niche requirement).

Any thoughts @Batch21 @s-simoncelli @laurenpetch ?

[jetuk]

If you want a quick pointer. Read the added "developer guide" for the book, and then perhaps look at the changes to InputNode in pywr-schema/src/nodes/core.rs, but the impacts are more important for the storage nodes (piecewise, virtual, etc).

[s-simoncelli]

I agree that one may introduce errors depending on how they implement the add_to_model method. I quite like the idea of having the separation between the node core properties and their constraints.

This is just my personal view, but I don't see a strong need of having a variable max_volume parameter. If it's a constant parameter (as defined in #62) you still have the advantage of being able to load it from an external file or to define it separately from the node definition. The only behaviour, where a variable value may useful, would be when there is a drawdown scheme in place and we need to maintain a specific target level and change the max volume we can store. But this kind of behaviour can be achieved by a custom parameter/node.

What are other user-cases for which we need it to make it variable?

[jetuk]

There are two cases:

1. The PiecewiseStorage is made of several core Storage nodes whose maximum volume can vary along,
2. Someone modelling a new investment that increases the size of reservoir part way through a simulation.

(1) is more of a problem and the original case for allowing max_volume to be a parameter.

I think there's something in #62 which is part way between a constant and the fully dynamic parameter we have now. I'll add something to that issue.

[Batch21]

I keep changing my mind on this but agree that it was nicer to have the separation that add_to_model and set_constraints gave. Also, having considered it further, behaviour being defined by the order of operations in the method feels a little bit too opaque.

The 'semi-constant' parameter solution looks promising. I guess this property could apply to most parameters, such as profiles, that don't dependent on other components for calculating values. This probably applies to most of the parameters you might want to use for curves in a PiecewiseStorage?

[jetuk]

Closing in favour of #194.