Refactor sediment model

[hboisgon]

Issue addressed

Fixes #382

Explanation

For now this is a tentative to restructure the vertical part (ie soil erosion) of the sediment model.

General

The main features of the restructuration are:

• I think I would like to separate the current sediment.jl which contain everything either in two or three files: soil erosion, sediment transport (land and river) -> a new erosion.jl file has been created for the soil erosion part and the SoilLoss struct and a new sediment_flux.jl for OverlandFlowSediment and RiverSediment
• hydrometeo_forcing now contains all forcing for sediment whether they will be used by the land or river part. Not sure if this will be the final solution.
• new RiverGeometry and LandGeometry struct that contains cell properties such as width, slope, length, area etc to avoid defining and passing these parameters that can be common to many structs.

TODO left:

• BMI and @grid_loc
• Rename the initialise functions of the main structs (SoilLoss, LandSediment, RiverSediment)
• Update the test staticmaps

Vertical: SoilLoss

The main features of the restructuration are:

• Three main structs have been created AbstractRainfallErosionModel (with three models NoRainfallErosionModel, RainfallErosionEurosemModel, RainfallErosionAnswersModel), AbstractOverlandFlowModel (with two models NoOverlandFlowErosionModel, OverlandFlowAnswersModel) and SoilErosionModel which is in charge of summing erosion from the different sources (for now rainfall and overland flow but in the future we could add wind, mass wasting from landslide etc.) and differentiating for the different particle classes. Each struct is defined in its own script but the equations are in a common file (erosion_process.jl)
• very few parameters are common to each concept so for now I did not create a separate erosion_parameters (similar to vegetation_parameters in sbm)
• area is the only separate parameter of the main SoilLoss struct, timestep is passed directly to the update functions.
• The computation of the transport capacity has been moved to the land transport and is no longer part of the soil erosion (vertical concept)
• The calculations of the soil fractions of clay/silt/sand/small+large aggregates has been moved to HydroMT

TODO left:

• Computation of interception and canopygapfraction should re-use the interception model from the sbm refactor

Lateral.land: OverlandFlowSediment

The main features of the restructuration are:

• The main struct is composed of a transport_capacity model that can link to three different concepts with/without particle differentiation (TransportCapacityGoversModel, TransportCapacityYalinModel, TransportCapacityYalinDifferentiationModel); a sediment_flux model for the overland flow transport with two equations for with/without particle differentiation and a to_river to summarize the sediment reaching the river (with/without particle differentiation).
• The to_river model is only masking overland flow deposition for the river cells. It may not be needed but I started to like the concept that for vertical, lateral, you can call the different output in a generic way ie vertical.soil_erosion.clay, lateral.land.transport_capacity.clay, lateral.land.to_river.clay (actually with a an extra variables in between vertical.soil_erosion.variables.clay). I think this structure is also more straightforward than arranging around particles to make it easy to use different concepts or for dependencies on particles for certain processes (ie not an organization like vertical.clay.variables.soil_erosion or lateral.land.clay.variables.transport_capacity).

TODO left:

• vertical and lateral.land share two common forcing (q_land and waterlevel_land). I am not sure if there would be a smart way to avoid double definition / double reading in the sediment_config.toml of these variables part from passing the vertical.hydrometeo_forcing struct as an input to lateral.land.update).

Lateral.river: River Sediment

I thought I would be able to split more but because we need the upstream to compute the downstream, I ended up with less structures than planned and still quite a large structure for the mass balances (that still include then erosion and deposition). In the end this means the construction looks similar to lateral.land so this may not be for the worst. I also could not easily separate the reservoirs into an extra struct so I left it as is in master.
The main features of the restructuration are:

• The main struct is composed of a transport_capacity model that can link to five different concepts (TransportCapacityBagnoldModel, TransportCapacityEngelundModel, TransportCapacityYangModel, TransportCapacityMolinasModel, TransportCapacityKodatieModel); a potential_river_erosion model to get the maximum allowed direct bed and bank erosion (for example in the future to use delwaq concept this could be turned into none allowed); a sediment_flux model for the river flow transport (mass balance from upstream to downstream including deposition and erosion); and a concentrations to convert loads to concentrations and get the suspended vs bed part.

TODO left:

• a lot of particle parameters are now duplicated (density, mean diameter etc) and could be moved into separate Particle struct

Testing of this PR

• Because some parameter calculations have moved to hydromt, a new version of the staticmaps needs to be published for the test to pass
• The test results differ a little in v1 because pathfrac is no longer used with the answers concept (pathfrac is actually part of the usle_c parameter so it was counting double)

Checklist

• Updated tests or added new tests
• Branch is up to date with v1
• Tests & pre-commit hooks pass
• Updated documentation if needed
• Updated changelog.md if needed

Additional Notes (optional)

Add any additional notes or information that may be helpful.

[hboisgon]

Hi @verseve
I think I managed to rebase from v1 to master but this was a very painful process with a lot of conflicts that had nothing to do with the changes from this branch...

Anyhow, I think it is ready for review. For the leftover TODOS in the main message, I tried some of them myself but did not manage easily so it would be really great if you could have a look at these, this would be more efficient and I don't think for you it's too much:

• add BMI compatibility (@grid_loc)
• Rename the initialise functions of the main structs (SoilLoss, LandSediment, RiverSediment): I did not manage to get the proper types of the arguments when I tried
• Update the test staticmaps: also don't know how to do that or if I have access. The file can be found here: https://we.tl/t-6TIiVjAzhR
• Optional: adding interception calculation -> it's not used very often so it can be a post-v1 change

The two others in lateral.land and lateral.river are more questions or up for discussions with your review.

Thanks in advance!

[verseve]

Nice work!
I added a couple of comments. Besides these comments: the code coverage went down > 2%, would be good to check what is causing this and to get this back to ~90%.

[hboisgon]

I think I addressed all of your comments @vers-w ! A few are still open and if you are happy with my solutions then feel free to close them.
For the tests, a lot of the concepts in the sediment model were never tested and then with the refactor, a new concept meant a lot more lines of code so then the coverage issue showed up. I now added tests for all possible concepts in :)

[vers-w]

LGTM!