Update combine_surfaces!
#316
Conversation
LenkaNovak
force-pushed
the
ln/combine-surfaces-update
branch
from
f557fed
to
0d58835
Compare
docs/src/interfacer.md
Outdated
| update_field!(sim::SurfaceModelSimulation, ::Val{:area_fraction}, field::Fields.Field) = ... | ||
| update_field!(sim::SurfaceModelSimulation, ::Val{:air_temperature}, field::Fields.Field) = ... | ||
| ``` | ||
| - these adapter function, to be defined in the component models' init files (preferably in their own repositories), allow the coupler to operate without having to assume particular data structures of the underlying component models. This allows easy swapping of model components, as well as a stable source code with coupler-specific unit tests. |
There was a problem hiding this comment.
| - these adapter function, to be defined in the component models' init files (preferably in their own repositories), allow the coupler to operate without having to assume particular data structures of the underlying component models. This allows easy swapping of model components, as well as a stable source code with coupler-specific unit tests. | |
| - these adapter functions, to be defined in the component models' init files (preferably in their own repositories), allow the coupler to operate without having to assume particular data structures of the underlying component models. This allows easy swapping of model components, as well as a stable source code with coupler-specific unit tests. |
| @@ -162,6 +163,7 @@ function bucket_init( | |||
| space, | |||
| dt::FT, | |||
| saveat::FT, | |||
| land_fraction, | |||
There was a problem hiding this comment.
The named field in the struct was generic area_fraction (as for the other SurfaceModelSimulation types). Is this discrepancy here desired/intentional?
There was a problem hiding this comment.
Ahh, this was left from an old version. Well spotted!
There was a problem hiding this comment.
Good point - I think it would be nice to consistently use area_fraction :)
src/Interfacer.jl
Outdated
| """ | ||
| Interfacer | ||
|
|
||
| ... | ||
| """ |
There was a problem hiding this comment.
There are some missing docstring warnings in the docs
src/Regridder.jl
Outdated
|
|
||
| Sums Field objects in `fields` weighted by the respective area fractions, and updates | ||
| these values in `combined_field`. | ||
| NamedTuples `fields` and `fractions` must have matching field names. |
There was a problem hiding this comment.
Can we give an example here of the call with matching field names?
| @@ -162,6 +163,7 @@ function bucket_init( | |||
| space, | |||
| dt::FT, | |||
| saveat::FT, | |||
| land_fraction, | |||
There was a problem hiding this comment.
Good point - I think it would be nice to consistently use area_fraction :)
LenkaNovak
force-pushed
the
ln/combine-surfaces-update
branch
from
9e9960b
to
21d46e3
Compare
update amip experiment slabplanet runs amip runs component model test passes fix combine_surfaces_from_sol! import viz fix add more unit tests docs and interface bug fixes conservation fix `combine_surfaces!` docstring fix revs rev2 doc comment
LenkaNovak
force-pushed
the
ln/combine-surfaces-update
branch
from
78a5f56
to
08850b4
Compare
|
bors r+ |
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Purpose
We need to generalize
combine_surfaces!andupdate_area_fractionsbefore introducing the new surface fluxes outlined in #296 .Closes #317 (also see for more details)