Closed kmdeck closed 3 years ago
Merging #23 (b2ce4f3) into main (5f2e920) will increase coverage by
22.21%
. The diff coverage is93.18%
.:exclamation: Current head b2ce4f3 differs from pull request most recent head cf62b39. Consider uploading reports for the commit cf62b39 to get more accurate results
@@ Coverage Diff @@
## main #23 +/- ##
===========================================
+ Coverage 51.67% 73.88% +22.21%
===========================================
Files 8 8
Lines 269 337 +68
===========================================
+ Hits 139 249 +110
+ Misses 130 88 -42
Impacted Files | Coverage Δ | |
---|---|---|
src/SoilModel/right_hand_side.jl | 53.73% <72.72%> (+25.70%) |
:arrow_up: |
src/SoilModel/boundary_conditions.jl | 96.25% <95.94%> (+62.91%) |
:arrow_up: |
src/Domains/domain.jl | 100.00% <100.00%> (ø) |
|
src/SoilModel/SoilHeatParameterizations.jl | 75.86% <0.00%> (-0.81%) |
:arrow_down: |
src/SoilModel/models.jl | 100.00% <0.00%> (+33.33%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 5f2e920...cf62b39. Read the comment docs.
The domains in ClimaCore have boundary names associated with them. For example, IntervalDomain has :top and :bottom.
In my RHS, I loop over the boundary faces and compute the boundary fluxes to apply at each boundary. Do I need to consistently label my boundary faces (for example, in my bc object SoilDomainBC(top = ....., bottom = ....) with their names in IntervalDomain? @simonbyrne
Right now I am requiring that somewhat implicitly:
faces = model.domain.x3boundary
bcs = getproperty.(Ref(model.boundary_conditions), faces)
fluxes = (;
zip(
faces,
return_fluxes.(Ref(Y), bcs, faces, Ref(model), Ref(cspace), t),
)...,
)
this yields fluxes = (top = (water =..., energy =...), bottom = (water = ..., energy = ...))
.
Inside the RHS methods, all boundary conditions are converted to flux boundary conditions using a single function interface/call, so that we dont need to write different RHS methods for each boundary condition possibility. This PR sets this up for two new types of boundary conditions -
Dirichlet
andFreeDrainage
(water only), and extends what we had before.The flux formula depends on the face we are on, and we need to evaluate it for each boundary face. Inside the RHS, we broadcast over faces (face names taken from those used in ClimaCore for that domain), and populate a named tuple with the fluxes at each face, which is then accessible everywhere else in the RHS. Ideally this will cleanly extend to domains with different #s of faces, periodicities, etc.
Do we need to ensure consistency between the Domain face identifiers (e.g. labeled :top and :bottom in ClimaCore for columns) and the attribute fields of SoilDomainBC?
when reviewing, only domains.jl, boundary_conditions.jl and right_hand_side.jl need to be looked at.