-
Notifications
You must be signed in to change notification settings - Fork 73
Kd work and buoyancy flux diagnostics #853
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
Conversation
Looks like it is failing almost all the tests, so I'm marking this as draft and will re-examine ASAP. |
The issues that caused the checks to fail have been addressed. |
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.
These diagnostics have several problems with the choice of rescaled units and introduce an unnecessary dependency on the Boussinesq reference density when in non-Boussinesq mode. Please reconsider the scaling and expressions that are being used here and make sure that all of the diagnostics are passing the full range of dimensional consistency tests.
@Hallberg-NOAA I think this PR is now ready for re-review, but I'm unclear why it fails the last test (@marshallward any insight?). The updates are substantial, and the code now fully addresses issue #816 by also including the Kd diagnostics inside set_diffusivity and 2d and volume integrated versions of the diagnostic (it now adds ~80 new total diagnostics to MOM6). It still does not include every Kd that could be used in MOM6, but it should be straightforward for a user to add any new Kd's to this routine in the future. The PR was reorganized by moving most of the diagnostics into the new KdWork module. This reorganization avoids cluttering diabatic_driver. The code was tested for dimensional consistency in both Boussinesq and non-Boussinesq mode, but I'm open to feedback on choices for scaling. |
Nothing to do with your PR, the perf output parser is always encountering problems with system libraries. I'll see if it's fixable, but don't let it hold up the PR. (Maybe we should disable this test?) |
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.
Thank you for these updates. This is PR looks good to me now, apart from the failing testing for dimensional consistency testing with rescaled Z with the Bflux_dia_diff_dz diagnostic.
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.
Please fix the failing testing for dimensional consistency testing with rescaled Z with the Bflux_dia_diff_dz diagnostic, after which I expect that this will be ready to be merged in.
This issue is fixed now, I had a typo propagate into all of my 3d "dz" diagnostic work arrays. This example is definitely a great case study for the benefit of dimensional consistency tests, this caught a subtle but very important issue! |
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.
In my view, this PR is now ready to go.
- Adds the ability to diagnose the implied work and buoyancy flux due to given diffusivities inside of diabatic_ALE - Adds outputs of N2 after applying diffusion, the implied bouyancy flux, the integrated buoyancy flux, the salinity components of buoyancy flux and N2, the temperature components of buoyancy flux and N2, the ePBL contribution to the buoyancy flux, and double diffusion contribution to the buoyancy flux, - Only considers the contribution of KPP to the total diffusive flux, the diagnostics should not be used w/ KPP until the non-local term and its contribution are properly accounted for. - Only considers the contribution of convection to the total diffusive flux, since diagnose its implied work would require accounting for the convective contributions to diffusivity (it would be easily doable following this template in the future, but it is not done here). - More work is needed to apply these methods to diffusivities that are diagnosed and stored in set_diffusivity. - This code is not repeated for diabatic_ALE_legacy since the double diffusion component to diffusivity is applied separately, thus complicating the interpretation of these diagnostics in the legacy driver.
- Change buoyancy flux units to W/m3 (W/m2 for vertical integrated terms). - Correct issues in unit scaling for Boussinesq and non-Boussinesq code. - Compute N2 from specific volume for non-Boussinesq mode.
- Kd work diagnostic calculations migrated from MOM_diabatic_driver to MOM_diagnose_KdWork - Kd work diagnostics stored in new type which contains copies of all requested diffusivities to facilitate storing and moving data around so it is accessible at the conclusion of diabatic_driver. - All Kd arrays are allocatable that won't be allocated unless needed. - 2d Vertical and volume integral quantities added. - Many more Kd work diagnostics added including many from MOM_set_diffusivity, still not exhaustive.
…ing in layer mode
c28f3e0
to
810f27d
Compare
This is about half of the work needed for issue #816 .
This PR adds a new routine to compute KdWork (the buoyancy flux and its integral) based on N2 and Kd. When provided with N2 at the end of dt_therm in adiabatic_ALE it diagnoses the implied work done by Kd and can split this work into the various processes that contribute to Kd. It splits N2 into salinity and temperature to enhance the information provided by the diagnostic and for diffusivity that is different for heat and salt (e.g. double diffusion). It also allows the total N2 and the salinity and temperature contributions to N2 to be diagnosed and output, which is also useful since all other N2 diagnostics in the model are floored at 0.
It should not be expected to work with KPP, since KPP's nonlocal flux requires additional terms in the buoyancy flux. Therefore, a KPP based diagnostic is not attempted here.
I also did not add a diagnostic for the convection routine since it is not one I exercise, but it could easily be added later.
The diffusivities in set_viscosity can be diagnosed in a similar manner. This PR could be accepted independent of those diagnostics, or it could all be merged at once. I will work on them next.