Refactor sea ice model #32
Conversation
|
I was thinking to convert also |
src/SeaIceThermodynamics/HeatBoundaryConditions/bottom_heat_boundary_conditions.jl
Outdated
Show resolved
Hide resolved
| ##### A bit of thermodynamics to start the day | ||
| ##### | ||
|
|
||
| abstract type AbstractSeaIceThermodynamics end |
There was a problem hiding this comment.
Why do we need an abstract type?
There was a problem hiding this comment.
Probably not necessary at the moment that we have only one thermodynamical model. I can remove it now and we can reintroduce it when we have more models
There was a problem hiding this comment.
also we only need an abstract type if we use it for dispatch somehow
src/SeaIceThermodynamics/SlabThermodynamics/SlabThermodynamics.jl
Outdated
Show resolved
Hide resolved
src/sea_ice_model.jl
Outdated
| ice_thickness :: IT | ||
| ice_concentration :: IC | ||
| # Thermodynamics | ||
| sea_ice_thermodynamics :: TD |
There was a problem hiding this comment.
pick ice or sea_ice. now we have ice_thickness but sea_ice_thermodynamics
There was a problem hiding this comment.
It may make sense to use sea_ice to refer to ice that is a mixture of brine and pure ice (eg with a salinity) and then to reserve ice for pure ice.
There was a problem hiding this comment.
@simone-silvestri did you address this?
|
|
||
| # TODO: Fix this after this PR | ||
| # include("EnthalpyMethodThermodynamics.jl") | ||
| include("SlabThermodynamics/SlabThermodynamics.jl") |
There was a problem hiding this comment.
I suggest using a flatter structure rather than another submodule here
src/SeaIceThermodynamics/SlabThermodynamics/slab_thermodynamics_tendencies.jl
Outdated
Show resolved
Hide resolved
|
Comments should be addressed and this should be ready to merge if it were not for these docs... @navidcy can you help me out a second? Seems like I am missing something. |
|
This shoud be ready to merge |
This PR is an attempt to refactor the ClimaSeaIce package following #31 to have a
SeaIceModelthat contains athermodynamicsanddynamicsfieldThis is necessary before starting to include a dynamics module.
cc @glwagner