unifhy-org / unifhy

A Unified Framework for Hydrology
https://unifhy-org.github.io/unifhy
BSD 3-Clause "New" or "Revised" License
12 stars 5 forks source link

Create new Components to represent Nutrient cycle #14

Closed ThibHlln closed 3 months ago

ThibHlln commented 4 years ago

At the moment the framework only focusses on the water cycle, but we aim to include at least the nutrient cycle. Once one other cycle is included in the framework, and the two cycles are able to communicate, adding any additional cycle is likely to be trivial.

At this stage, I think the best way to achieve this is by subclassing the Model class into WaterModel (then NutrientModel, etc.). Each will take one component for each part of the terrestrial water cycle. But their inward/outward transfers will need to be specific to the cycle considered (i.e. compliant with the interface of the given cycle). This is likely to be done by renaming current SurfaceLayerComponent/SubSurfaceComponent/OpenWaterComponent to WaterSurfaceLayerComponent etc. and by creating additional ones for NutrientSurfaceLayerComponent etc. They should all be able to directly be subclasses of Component, as in currently the case for the water cycle components. The superclass Model should retain the Clock and the Coupler.

For now water and energy cycles are likely to be considered together, because they are very intertwined in JULES at the moment, and it would require substantial efforts to isolate them (if possible at all). But, by bringing an additional cycle in the framework, most of the challenges will already have been addressed and, separating water and energy cycles would only require to remove energy fluxes from the water cycle interface + create a new energy cycle + breakdown actual science components to comply with the new interfaces.

ThibHlln commented 3 years ago

One important decision to make here is whether we allow components of other cycles than water+energy to run on different temporal/spatial resolution of the water+energy cycle.

For now, we don't see a use case where different resolutions would be required for different cycles.

mattjbr123 commented 1 year ago

Starting development of separate Nutrient subclasses. I'm seeing this as the master-thread that collates together the various tasks and issues that will need to be completed to successfully add separate nutrient components to unifhy.

An initial, but temporary, solution was to integrate the nutrient components within the existing Surface, Subsurface and Openwater Components which are considered the 'water energy' Components. This means that the necessary transfers between Components are already implemented and the main remaining task is to separate these out into separate 'Nutrient' Components. A further task could then be to separate the nutrient Components out further into their own Model subclass, as @ThibHlln suggests above.

To ensure integrity of the code and minimise the chance of breaking stuff, I will follow the 'test-driven-development' idea which requires tests for the code to be developed before the code itself. The tests should fail, and subsequent development should focus on passing the test and no more. As comprehensive tests have already been designed for unifhy, I see the first step towards proper 'test-driven-development' as editing these tests to expect 3 additional Components - the NutrientSurface, NutrientSubsurface and NutrientOpenwater Components. This will cause the tests to fail, with subsequent development just enough to cause the tests to pass and no more. Once this has been achieved I can start following the 'test-driven-development' idea properly and implement much smaller changes to the tests and subsequent development.

To that end the first issue is to catalogue and then implement all the changes that need to be made to the tests to support the 3 additional components (#93), which involves generating dummy Components that mimick the new Nutrient Components, generating the dummy data for the test runs of these Components and calculating the values of the transfers between the dummy Components (#95). The new Nutrient Components will then be added to the core model code, along with any changes elsewhere in the core code needed to allow the new unit tests to pass. The changes made will be documented in the issues listed below, and the developer documentation of UniFHy will be updated to explain the process of adding new Components and transfers (#96) along with changes to the user and contributor sections of the documentation to recognise the addition of the new nutrient Components (#99).

Main pull request to merge changes to main branch: #97

rich-HJ commented 1 year ago

Great to see you have started on this. Let me know if there is anything I can do to help.

mattjbr123 commented 1 year ago

Will do! I've been quietly working a lot on it the last few weeks, mostly just deep-diving into the code and getting to grips with how it's structured and where the changes are most likely to be made.

ThibHlln commented 1 year ago

Happy to help as well if you have questions/need code reviews. :-)

mattjbr123 commented 1 year ago

Thanks Thibault! At the moment I think feedback on the approach I'm taking and stuff I'm planning to do (when I get to it) would be most useful. I intend to document what I intend to do here first before implementing anything. But extra code reviewers certainly can't hurt too!

mattjbr123 commented 9 months ago

Test updates (finally!) complete. Issue #93 closed. I will now update the framework to add in the nutrient components, to allow the tests to pass.

mattjbr123 commented 9 months ago

unifhy.model

nutrientsubsurface: NutrientSubSurfaceComponent object The Component responsible for the subsurface compartment of any nutrient modelling.

nutrientopenwater: NutrientOpenWaterComponent object The Component responsible for the open water compartment of any nutrient modelling.

to the docstring of the [\_\_init\_\_ ](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L27) function
- [x] [\_\_init\_\_](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L27) needs to accept the three extra nutrient components as arguments
- [x] [\_\_init\_\_](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L27): Needs to be a _process_component_type call for the three nutrient components, with the above comment (for the doc generation)
- [x] [_check_components_plugging](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L125): Add three nutrients components to the loop
- [x] [identifier.setter](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L98): Add lines for the nutrient components
- [x] [\_\_str\_\_](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L141): Add the nutrient component info as for the other components
- [x] [from_config](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L154):  add import_module statements for the nutrient components as for the other components
- [x] And add them to the return statement of the method similarly
- [x] [to_config](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L175): Add the nutrient components in the return statement as the other components
- [x] Add

nutrientsurfacelayer: tests.components.nutrientsurfacelayer.dummy nutrientsubsurface: tests.components.nutrientsubsurface.dummy nutrientopenwater: tests.components.nutrientopenwater.dummy


to the docstring of the [from_yaml](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L231) method
- [x] [initialise_transfers_from_dump](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L275): Add nutrient components to Compass, Clock and Exchanger calls, as they are for the other components
- [x] [spin_up](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L351): Add get_spin_up_timedomain calls, add main_??_td lines/assignments and in two places add self.nutrientlayername.timedomain lines/assignments 
- [x] [_initialise](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L488): Add self.nutrientlayername.initialse_(...) lines like for the other modules
- [x] [_run](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L494): Add the nutrient components to the Compass, Clock and Exchanger sections as for the other components
- [x] Add run_nutrientlayernames to the [loop over clock](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L537), then add dump lines in the [if dumping:](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L540) section, then add run_nutrientlayername blocks for the three nutrient components, like the other components 
- [x] [_finalise](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L575): Add self.nutrientlayername.finalise\_() lines like for the other components
- [x] [resume](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L583): Add the nutrient components to the [loop](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L627) to initialise component states and records, and to the remaining time domain calculation [loop](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/model.py#L739). Change [`if data_or_null == 3`](https://github.com/unifhy-org/unifhy/blob/c827d4b035623dc7b2aac9475c71022670f91739/unifhy/model.py#L638) to 6.

### unifhy.component
- [x] In the \_\_init\_\_.py file add the Nutrient Components to the [imports from .component](https://github.com/unifhy-org/unifhy/blob/53c26de39a49d1aaf14e79b83103c8ed495102d7/unifhy/__init__.py#L10)
- [x] In the Component Class, change the documentation of the [category getter](https://github.com/unifhy-org/unifhy/blob/5aa2e61b7abf4d44ffc50c097c63f53b2a53c128/unifhy/component.py#L966) to include nutrients, i.e. "part of the water*/nutrients* cycle"
- [x] Add NutrientSurfaceLayerComponent, NutrientSubSurfaceComponent and NutrientOpenWaterComponent Classes, following the structure of the pre-existing Components, and moving the nutrients entries in their _input and _output_info dictionaries to their respective nutrients Components (i.e. from SurfaceLayerComponent to NutrientSurfaceLayerComponent, SubSurfaceComponent to NutrientSubSurfaceComponent, OpenWaterComponent to NutrientOpenWaterComponent)
- [x] In the [\_\_str\_\_ method of the Metaclass for the Component class
](https://github.com/unifhy-org/unifhy/blob/b49fef0d8ae258e45ca7f8b930dc99bb4b8491fb/unifhy/component.py#L129), add any transfers that should now be appearing in the docstring, from any modifications to the SurfaceLayer dummy Component

### Elsewhere
- [x] Calls anywhere to unifhy.Model() need to accept nutrient components as an additional three arguments
mattjbr123 commented 9 months ago

Now to check and commit after a break...

mattjbr123 commented 6 months ago

Documentation also needs updating: #99

mattjbr123 commented 5 months ago

This is a generic list of changes for developers to follow when adding further Components to the framework:

unifhy.model

unifhy.component

XxxxxYyyyy is the name of the class of the Component in the unifhy/component.py and xxxxxyyyyy is the name given to the component by the _category attribute of the class.

Elsewhere