Resolving constants milestone issues and constants bundling.

This is a cross between a meta-issue and a discussion to tidy up a backlog of constants questions that we've grouped in the "Constants system" milestone.

This issue is also a summary of the inaugural monthly "GitHub issues" developer meetings to bundle existing issues and clean them up 🎉 🥂

Where we are now

• We have now adopted the constants system presented in discussion #162
• There's been some discussion of passing multiple constants classes to a model: CoreConstants and ModelSpecificConstants and whether it might be easier to merge core constants into each model constants to provide a simpler namespace (see #342).

Outstanding issues

We have five outstanding constants issues:

• #129 is about creating core constants but contains still relevant discussions about units. We have a separate milestone bundling units issues so I think we can simply close #129.
• #277 is about how we ensure that configured constants are passed down correctly rather than falling back to defaults. I think this is largely outdated now that we are using the constants system, so I think we can close #277.
• #348 is still active and just needs a PR to redefine constants values through the models.

The remaining two issues are linked and this PR suggests a resolution for both:

• #342 is the discussion about simplifying model signatures to only need one bundled constants class. Implementations for this were also discussed in #341.
• #358 is an issue that @vgro had with sharing constants and functions between parallel model development: if a function using constants is defined in one model then currently importing and re-using that function in another model requires also importing the constants and juggling multiple parallel constants classes.

Sharing constants and functionality

The proposed solution to #358 that we can define model "meta" constants classes and use class inheritance to allow the clean use of different constants classes with shared functions. So:

from virtual_rainforest.core.constants import ConstantsDataclass

class ModelMeta(ConstantsDataclass):
    """A meta constants class created in e.g. SimpleModel."""
    b = 2
    c = 3

class ActualModelSimple(ModelMeta):
    """The actual simple model constants from SimpleModel.

    This is derived from the meta class and only adds a constant specific to this model
    """
    d = 5

class ActualModelComplex(ModelMeta):
    """A more complex model constants from a parallel model.

    This adds two new constants and overwrites an existing constant with a model specific version
    """
    b = 4
    d = 5
    e = 6


def shared_function(constants: ModelMeta):
    """A function created in simple model that we also want to use in the complex model."""    
    return constants.b 

These can then be used seamlessly in both models - all of the statements below are mypy friendly because the typing of ModelMeta supports the meta class itself and any derived model specific subclasses.

In [7]: meta = ModelMeta()
In [8]: simple = ActualModelSimple()
In [10]: cplex = ActualModelComplex()
In [11]: shared_function(meta) # Although you'd never really do this in practice!
Out[11]: 2
In [12]: shared_function(simple)
Out[12]: 2
In [13]: shared_function(cplex)
Out[13]: 4

Merging core constants into models - we could but no

That solution also opens up this resolution for #342 - which seems really clean and avoids the whole issue with properties that was mentioned on this topic in #341.

from virtual_rainforest.core.constants import ConstantsDataclass

class CoreConstants(ConstantsDataclass):
    a = 1

class ModelMeta(CoreConstants):
    b = 2
    c = 3

class ActualModelSimple(ModelMeta):
    d = 5

And hence:

In [15]: const = ActualModelSimple()
In [16]: const.a
Out[16]: 1

However, the dev meeting concluded:

• That seems super clean and simple but we then have the core constants multiply defined independently in each model constants class - they shouldn't differ but why would we even open up the possibility?
• This does mean that models will typically always have two constants classes, but this does make a clean separation of a singly defined set of core constants and then model specific things.
• As an aside, at the moment, models using the core constants are independently creating fresh instances from Config, but we should simply create one instance at startup and share it through the model signature somehow. We could add core_constants to the ABC signature.

So - we should simply close #342 and live with having two constants classes.