ImperialCollegeLondon / virtual_ecosystem

This repository is the home for the codebase for the Virtual Ecosystem project.
https://virtual-ecosystem.readthedocs.io
BSD 3-Clause "New" or "Revised" License
9 stars 1 forks source link

Defining the CoreComponents, LayerStructure and ModelTiming classes #374

Closed davidorme closed 9 months ago

davidorme commented 9 months ago

Description

Following the discussion in #369, we've aligned on a solution that tries to strike a balance between the original completely identical BaseModel subclass signatures and something that retains some of the model to model differences. See https://github.com/ImperialCollegeLondon/virtual_rainforest/issues/369#issuecomment-1908099087 for a sketch of this solution.

That requires a dataclass of core details that can be created once and then passed to all model initialisations. Models will then share a genuine single copy of those details - not individual identical copies - and model signatures can avoid the more opaque Config objects while still inheriting core components.

This PR implements that by creating CoreComponents which includes nested classes for LayerStructure and ModelTiming. Those borrow heavily from existing code for generating layer_roles and the extract_timing_details function. Once this is accepted, those functions will disappear and an instance of CoreComponents can be used in their place.

Issue #369 is not completed by this PR, but it would be good to agree on this structure and I can then adopt this structure in a second PR onto this, so we can break up the review a little bit!

Edited to add: That second WIP PR (#375) is now in progress - I wanted to see if implementing the changes here threw up anything unexpected. They did not.

Type of change

Key checklist

Further checks

codecov-commenter commented 9 months ago

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (e9365f0) 93.30% compared to head (42bd787) 93.18%.

Files Patch % Lines
virtual_rainforest/core/core_components.py 94.21% 7 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #374 +/- ## =========================================== - Coverage 93.30% 93.18% -0.13% =========================================== Files 59 60 +1 Lines 2866 2918 +52 =========================================== + Hits 2674 2719 +45 - Misses 192 199 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

davidorme commented 9 months ago

Something odd going on with sphinx (what a surprise) that means it can't seem to find references for imported objects like InitVar and np abbreviations.

davidorme commented 9 months ago

I've merged #375 into this branch, ready to merge the whole thing down into develop.