-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dynamical Sea Ice Model #69
Conversation
01a3dce
to
6681193
Compare
@kmdeck this is ready to be merged as a checkpoint for the dynamic sea ice. It is now in a separate directory, so it shouldn't conflict with the Bucket PR, and we can pick it up again after AMIP. If you're ok with it, I'll wait for you to merge #57 and then rebase on that. As for documentation, the sea ice model is documented in the |
6681193
to
a97869f
Compare
Manifest.toml
Outdated
@@ -0,0 +1,929 @@ | |||
# This file is machine-generated - editing it directly is not advised | |||
|
|||
julia_version = "1.7.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did the Manifest for ClimaCoupler change with this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh! Good catch! I think the formatting did this.
global_sol_u_slab[i] = Fields.Field(global_Y_slab, global_h_space) | ||
end | ||
end | ||
if ClimaComms.iamroot(comms_ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems hardcoded to a slab+atmos. Will this work for this example? If not, should it be removed from the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point - btw, if we wanted to use it (say in the Bucket example), we'd need to combine all of the surface fields into one, like we do in the coupling loop in the coupler_driver.jl
. But since we don't need MPI for this example imminently, and since we'll be changing the interface for this in the bucket example (and then broadcast it to the other examples), I think deleting this here makes sense. :)
end | ||
|
||
|
||
function semster_zero_layer_model(T_ml, T_sfc, h_ice, F_atm, ∂F_atm∂T_sfc, p, Δt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
semtner
Y = u | ||
FT = eltype(dY) | ||
|
||
if p.Ya.prescribed_sic_data !== nothing # Prognostic eqn for T_sfc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if only the Dynamic Sea Ice model should be added - i.e. remove code for prescribed_sic = true from this PR. Moving forwards, the prescribed SIC+dynamic T and the dynamic sea ice (h, T_sfc prognostic) should be different types of SeaIceModels, and we would use dispatch - we wouldnt want to have these if/else statements about prescribed_sic, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this plan. I'm tempted to leave it in for now though until we revamp the interface, as it is useful to have the prescribed_sic option as a check in case the Dynamic one is doing something funky.
space = nothing, | ||
mask = nothing, | ||
prescribed_sic_data = nothing, | ||
ocean_params = (; ρ = FT(1e3), c = FT(4e3), h = FT(1)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should these be enforced to be consistent with the slab_ocean params being used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I made it a non-keyword argument, so there is no accidental imposition of the default ocean parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Lenka - not sure of the best way to proceed, it's up to you. We can merge this as is (except for small changes noted in comments), as a copy of the sea ice model, but this code will have to be changed a lot to be used in practice (globals, unused files, etc). This is also not being tested, so it may break at some point (due to other packages changing). It might be better to bring it up to date with moist_mpi_earth changes (but still in it's own directory, so it can have its own versions of conservation checker, etc). Then we can add it to buildkite. and it will be easier to refactor, etc later for interface changes.
I almost wonder if it better to keep this as a branch. Then we could start a new branch off of ClimaCoupler next week (or when slab models are abstracted), and jsut add in the dynamic ice model as a model option with its own methods then. And that should work if we have done the abstractions well. |
As discussed on Slack, we will go ahead with the merge, because we be won't be working on the dynamical sea ice for a while (except for perhaps making it an abstract type), so it would be good to checkpoint it now (as a working model integration PR), and then iterate with smaller PRs (with a new interface or enhancements to the physical parameterization) when we have time. |
fix coupler_loop_rough spatial BCs, LinearSolver deps update, runs, rough spatial BCs working dry conservation moist conservation, SF.jl, idealised radiation(t) update for new ClimaAtmos inteface changes depend on ClimaAtmos checkpoint wip - mpi race condition fix mpi working + conserving fixes for single processor surface flux clean up readme add deps deps deps for cluster scaling scaling tests finished format rm residual output init Co-authored-by: Akshay Sridhar <akshaysridhar@users.noreply.github.com> Ben Mackay <jbmackay@caltech.edu> jiahe23 <jiahe23@users.noreply.github.com> Zhaoyi Shen <szy21@users.noreply.github.com> runs sst + land-sea - conservation pending Co-authored-by: Akshay Sridhar <akshaysridhar@users.noreply.github.com> stable conservative 60d sst option write vtk from Float32 prescribed sea ice + ssts running - pending tests ice runs for >15d, qualitatively behaves space fix format space fix old bug fixes copy over rad utils conserves with radiation TOA balance maintained (but still needs time log) TOA fluxes conservation check callback sea ice conservation check passes format dynamic sea ice clean up - wip note init from PR#50 mv dir mv dir dynamic sea ice conservation - up to 27d, 1% err clean dynamical sea ice runs format rev fix remove unused mpi hooks
a97869f
to
4dc9942
Compare
bors r+ |
Purpose and Content
moist_mpi_earth
directory, which will contain the prescribed sea ice and bucket model (PR#51 that addresses SDI Land implement bucket model in Land #41 ).To do:
separate example from the bucket / prescribed (
moist_mpi_earth
) directoryenergy conservation test - , neglecting the initial equilibration over the first 2 timesteps, up to 0.1% error after 27d
physical test - h_ice (m)
h_ice.mp4
Future PRs:
Benefits and Risks
Linked Issues
(Provide references to any link issues. Use closes #issuenum to automatically close an open issue)
PR Checklist