Adjust sea ice model constants to align with ClimaSeaIce.jl#main#224
Adjust sea ice model constants to align with ClimaSeaIce.jl#main#224taimoorsohail wants to merge 8 commits into
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
This should be fixed in PR #164, can you review that one? Should be rather easy a part the change to the temperature interface computation which is reformulated a bit, the rest should be trivial (split freshwater fluxes in rain and snow and accumulate snow in the sea ice fluxes). |
|
Thanks @simone-silvestri -- I had a look and it looks good. However, I think there are some specific changes here that are not in #164 :
sea_ice_model(sea_ice::Simulation) = sea_ice_model(sea_ice.model)
sea_ice_model(model::SeaIceModel) = model
sea_ice_model(model) =
throw(ArgumentError("Expected a ClimaSeaIce.SeaIceModel or sea-ice Simulation, got $(typeof(model))"))(3) is more descriptive in case users accidentally swap the order of |
|
Ok, I ll merge that one then we can also add what implemented in this PR |
| atmosphere_ocean_fluxes = SimilarityTheoryFluxes(eltype(exchange_grid)), | ||
| atmosphere_sea_ice_fluxes = atmosphere_sea_ice_similarity_theory(eltype(exchange_grid)), | ||
| sea_ice_ocean_heat_flux = ThreeEquationHeatFlux(sea_ice), | ||
| sea_ice_ocean_heat_flux = ThreeEquationHeatFlux(eltype(exchange_grid)), |
There was a problem hiding this comment.
are you sure about this change? ThreeEquationHeatFlux is specialized for a ClimaSeaIce simulation to include the internal heat flux in the balance of fluxes required by the interfacial temperature. If we use eltype(exchange_grid) we loose the internal conductive flux
| heat_capacity(sea_ice) = sea_ice_model(sea_ice).phase_transitions.heat_capacity | ||
| reference_density(sea_ice) = sea_ice_model(sea_ice).phase_transitions.density |
There was a problem hiding this comment.
maybe instead of having a sea_ice_model guard we can specialize these functions for
heat_capacity(sea_ice::Simulation{<:SeaIceModel})which is the more "julia" way
|
|
||
| conductive_flux = sea_ice.model.ice_thermodynamics.internal_heat_flux | ||
| ice_temperature = sea_ice.model.ice_thermodynamics.top_surface_temperature | ||
| model = sea_ice_model(sea_ice) |
There was a problem hiding this comment.
same as above, we probably should just specialize ThreeEquationHeatFlux to dispatch on
ThreeEquationHeatFlux(sea_ice::Simulation{<:SeaIceModel}, ....)Co-authored-by: Simone Silvestri <silvestri.simone0@gmail.com>
|
Thanks @simone-silvestri -- I think you are right about the |
|
Actually @simone-silvestri I am closing this PR now. I think the only meaningful addition was the more verbose sea ice model error, and that can be added upstream to warn the user that the input order in |
|
Closed in favour of #230 |
Currently, NumericalEarth.jl#main and ClimaSeaIce.jl#main cannot be run together due to a mismatch in constant naming and function definitions in
src/EarthSystemModels/InterfaceComputations.I have attempted to change the relevant files so that
OceanSeaIceModel()again works with the #main branches of both projects. This was the minimal fix I could find to ensure the code ran -- not necessarily an exhaustive fix for existing bugs in the #main branches of either repo.