Fixes surface buoyancy forcing calculation #690
Conversation
current calculation does not appropriately account for seaIceFreshwater and IcebergFreshwaterfluxes
|
I've started a G-case to test this and will report back when it's finished. |
| - fracAbsorbed * (rainFlux(iCell) + evaporationFlux(iCell)) * activeTracers(indexTempFlux,1,iCell)/rho_sw & | ||
| - fracAbsorbedRunoff * surfaceThicknessFluxRunoff(iCell)* min(activeTracers(indexTempFlux,1,iCell),0.0_RKIND)/rho_sw | ||
|
|
||
| if(config_cvmix_use_kpp_buoyancyForcing_fix) then |
There was a problem hiding this comment.
@philipwjones @mattdturner or @amametjanov:
We put this if flag here for backwards compatibility. It is within a cell loop.
Is this still a performance sin, or do the compilers handle this now for flags that never change during the run?
There was a problem hiding this comment.
It's a sin my son and will require at least 3 Hail Marys. Particularly in this case, where the loop is complex enough that the compiler can't break it up, it's not optimal. I don't know that it'll be a huge performance hit on CPU architectures but will probably need to be broken out for the GPU version.
There was a problem hiding this comment.
this will hopefully be a relatively short term hack. we just need to remember to remove it once we are satisfied with the new (correct) code.
There was a problem hiding this comment.
agreed. Once we test this in WC and are happy with results, I'll make a PR to remove the if statement. I can't imagine this is in for more than a week.
There was a problem hiding this comment.
Perfect, thanks for the perspective. I made an issue to help us remember: #692
…(remove from package)
src/core_ocean/Registry.xml
Outdated
| @@ -394,6 +394,10 @@ | |||
| description="If true, use the Community Vertical MIXing routines to compute vertical diffusivity and viscosity" | |||
| possible_values="True or False" | |||
| /> | |||
| <nml_option name="config_cvmix_use_kpp_buoyancyForcing_fix" type="logical" default_value=".false." units="NA" | |||
There was a problem hiding this comment.
@vanroekel do you want this default true in E3SM? If so, I'll make it true here as well.
Fixes surface buoyancy forcing calculation #690 Currently the KPP buoyancy flux is assuming the seaIceFreshWaterFlux and iceBergFreshwaterFlux enter the ocean at 0C, but the surface_bulk_forcing code assumes it enters at the freezing temperature. This PR unifies the treatment.
There was a problem hiding this comment.
Reviewed code, passes stand-alone tests. Testing in E3SM now, but @vanroekel has tested as well.
This PR brings in a new mpas-source submodule with changes only to the ocean core, plus scripts updates corresponding to Registry changes in the ocean. It includes three changes to improve ocean heat uptake: * Fixes limiting on redi k33 (MPAS-Dev/MPAS-Model/pull/684); * Fixes surface buoyancy forcing calculation (MPAS-Dev/MPAS-Model/pull/690); and * Revert GM buoyancy gradient calculation to v1 form (MPAS-Dev/MPAS-Model/pull/687). [NML] [non-BFB]
Update mpas-source: GM/Redi and surface buoyancy fixes This PR brings in a new mpas-source submodule with changes only to the ocean core, plus scripts updates corresponding to Registry changes in the ocean. It includes three changes to improve ocean heat uptake: * Fixes limiting on redi k33 (MPAS-Dev/MPAS-Model/pull/684); * Fixes surface buoyancy forcing calculation (MPAS-Dev/MPAS-Model/pull/690); and * Revert GM buoyancy gradient calculation to v1 form (MPAS-Dev/MPAS-Model/pull/687). [NML] [non-BFB]
…/develop Fixes surface buoyancy forcing calculation MPAS-Dev#690 Currently the KPP buoyancy flux is assuming the seaIceFreshWaterFlux and iceBergFreshwaterFlux enter the ocean at 0C, but the surface_bulk_forcing code assumes it enters at the freezing temperature. This PR unifies the treatment.
Currently the KPP buoyancy flux is assuming the
seaIceFreshWaterFluxandiceBergFreshwaterFluxenter the ocean at 0C, but the surface_bulk_forcing code assumes it enters at the freezing temperature. This PR unifies the treatment.