Add coupler_push! and coupler_pull! to Sea Breeze
#83
Conversation
|
Will add more docs in #75 |
| T_sfc_ocean = coupler_get(coupler, :T_sfc_ocean, atmos) | ||
| T_sfc_land = coupler_get(coupler, :T_sfc_land, atmos) | ||
| atmos.integrator.p.T_sfc .= T_sfc_land .+ T_sfc_ocean |
There was a problem hiding this comment.
@LenkaNovak this is a good example of why we would want a coupler_get rather than coupler_get! - because the atmos boundary is shared by land and ocean we can't just use .= directly as in coupler_get!.
| @@ -38,7 +38,7 @@ end | |||
|
|
|||
| simA = SimulationA(ones(spaceA)) | |||
| simB = SimulationB(zeros(spaceB)) | |||
| coupler = CouplerState() | |||
| coupler = CouplerState(1.0) | |||
There was a problem hiding this comment.
Coupled timestep storage: As the coupled timestepping info becomes more well defined, this may need to be refined (e.g. a CoupledTimestepper struct that could pair with the CouplerState struct as a full Coupler). This will likely be determined by how accumulation is ultimately handled.
This is one reason that having a separation here might be useful! You shouldn't really need time information when testing these CouplerState put and get methods. On the other hand, push and pull are more tied into the time coordination (since they would be part of a coupled step method). I still think we can punt on this.
There was a problem hiding this comment.
I agree with the separation between put/get and push/pull. But I would expect Δt_coupled to be in the CoupledSimulation to mimic the model simulations. There seems to be quite a bit of cross-over between the CoupledSimulation and CoupledState. Any thoughts on combining these?
There was a problem hiding this comment.
If we do this, it would be a separate PR anyway, so I'm happy to go ahead with merging this one. Thanks for pushing this through! 🚀
There was a problem hiding this comment.
I agree with the separation between put/get and push/pull. But I would expect
Δt_coupledto be in theCoupledSimulationto mimic the model simulations. There seems to be quite a bit of cross-over between theCoupledSimulationandCoupledState. Any thoughts on combining these?
I think there is a separation - CoupledSimulation should fit the Simulations interface and deals with packing up component simulations and the coupler (currently just a CouplerState) and deals with how to execute things. More on the time stepper side as a whole.
CouplerState deals with the actual coupled fields and passing them around. This is the part that will deal with MPI and how things get distributed.
jb-mackay
force-pushed
the
bm/seabreeze_pushpull
branch
from
b7e8f5b
to
c8b86ce
Compare
|
bors r+ |
Purpose and Content
Adds
coupler_push!andcoupler_pull!methods for each component in the sea breeze demo. These methods wrap how each component interacts with the coupler (sending & receiving viacoupler_putandcoupler_get).This PR also moves the coupled time-step into the
CouplerStatestruct so that it is accessible in the push/pull calls.Benefits and Risks
Push/pull: A major benefit is the conciseness of the main driver loop - which now consists of only
coupler_push!,step!andcoupler_pull!calls. This means we can standardize the simulation run loop and develop a set of general coupled timesteppers that take some inputted component order. This may not be a high priority as custom coupled steppers and schemes will be useful.Coupled timestep storage: As the coupled timestepping info becomes more well defined, this may need to be refined (e.g. a CoupledTimestepper struct that could pair with the
CouplerStatestruct as a fullCoupler). This will likely be determined by how accumulation is ultimately handled.Linked Issues
Part of #44
PR Checklist