added diagnostic for internal heat in 3D

[gmacgilchrist]

Previous diagnostic for heat tendency due to geothermal heating was only a 2D field, with no information on the model layer into which the heat is added, preventing closure of cell-wise heat budget. Created a new diagnostic in MOM_geothermal.F90, called internal_heat_tend_3d that keeps track of the change in the 3d heat field heat before and after geothermal heating is applied.

[marshallward]

Hi @gmacgilchrist, I added some code style comments, including changes needed to pass the test suite.

The diagnostic itself looks OK but will have a look with one of the other guys.

[ashao]

@gmacgilchrist, I had a couple of comments, but nothing major. Not officially 'reviewing' this PR, but looked at it out of curiosity.

I have a more general inquiry for @Hallberg-NOAA and @adcroft about the approach to geothermal heating done here. I understand that it makes sense to distribute the heat over a finite depth range to avoid just dumping heat into a 'vanished' layer. Why does this routine do the calculation of redistributing layer thickness in isopycnal mode. Couldn't this be simplified to dump the heat into the first non-vanished layer, and then let MOM_entrain_diffusive deal with sorting the densities?

[gmacgilchrist]

Latest commit (still just for heat, no thickness or temperature diagnostics included yet) does the calculation within the main for-loop, and puts everything behind conditional statements. Not sure if it is done in the best or most efficient way, so I would welcome feedback and criticism.

Most of what I've done is based on code in MOM_diabatic_driver.F90. In that module, calculation of the tendency diagnostics are carried out in their own subroutine. Might this be something worth considering here, or is that overkill?

[marshallward]

Hi Graeme, just chatted with Alistair and it looks like I gave you a bit of bad advice regarding the need to conditionally allocate the variables.

This declaration is OK (ignoring the character limit):

  real, dimension(SZI_(G),SZJ_(G),SZK_(G)) :: temp_old  ! Temperature of each layer before any heat is added, for diagnostics [degC]
  real, dimension(SZI_(G),SZJ_(G),SZK_(G)) :: h_old     ! Thickness of each layer before any heat is added, for diagnostics [m or kg m-2]
  real, dimension(SZI_(G),SZJ_(G),SZK_(G)) :: work_3d   ! Scratch variable used to calculate change in heat due to geothermal

as long as all of the operations on temp_old, h_old and work_3d are inside conditional blocks:

if (CS%id_internal_heat_tend_3d > 0) then
  ! ...
endif

This is sufficient to ensure that the memory will never be touched or added to the stack when the diagnostic is not used.

Apologies for the confusion here.

If you fix this, address the missing indices in T_old, and remove the line with trailing whitespace, it should be OK to merge.

[gmacgilchrist]

Thanks @marshallward, this makes sense.

Can I just confirm that the procedure for deallocating the diagnostic arrays is done correctly? I just included it into the conditional statement after the diagnostic has been posted.

^ actually forget that... I won't need to allocate or deallocate given what you've just said.

[gmacgilchrist]

OK, here's another crack, including diagnostics for temperature and for thickness, and making the allocation implicit when values are assigned.

I think that the logic is good, but would appreciate someone double checking this.

I was also not sure what to do about naming the diagnostics. Is there a general convention that people stick to? I tried to stick to the conventions of the tendency diagnostics already available (although these are somewhat scattered). This makes them rather verbose though (e.g. internal_heat_heat_tendency)...

[marshallward]

This all looks great, thanks @gmacgilchrist. I will merge this now. (Edit: After testing in the CI.)

[marshallward]

GitLab regression test: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/8708

[marshallward]

Regression tests have passed, merging now.