-
Notifications
You must be signed in to change notification settings - Fork 62
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
diffusivities from internal tides ray tracing algo #677
diffusivities from internal tides ray tracing algo #677
Conversation
ed2353e
to
f6c9ef7
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev/gfdl #677 +/- ##
============================================
+ Coverage 36.88% 40.90% +4.01%
============================================
Files 271 42 -229
Lines 81692 5286 -76406
Branches 15269 1013 -14256
============================================
- Hits 30132 2162 -27970
+ Misses 45907 2939 -42968
+ Partials 5653 185 -5468 ☔ View full report in Codecov by Sentry. |
f6c9ef7
to
0a9ebe9
Compare
0a9ebe9
to
94dbf54
Compare
also reproduces over processor counts with N = 480 and N = 441 |
1ede7e1
to
5bb32cc
Compare
@Hallberg-NOAA PR passes unit scaling tests in Boussinesq .and. Non-Boussinesq |
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.
I have added a number of minor changes, mostly in the units of comments and in the dimensional rescaling factors, that I think need to be fixed with this latest revision to the code. None of these should be very complicated to address.
c88ab0e
to
aab5f48
Compare
- TKE_itidal_input values are set to zero where it is not applied to the model so that diagnostics are consistent - removed useless halo updates - diagnose energy loss terms as dE/dt instead of equivalent loss rate to properly close energy budget - tot_vel_btTide2 was already a velocity squared, no need to square again - Froude loss term was not properly initialized - add missing halo updates for internal tide speed - compute residual/critical slopes loss only where it has a meaning, allows to clean up noise in field - uh/vh in energy flux routines do not need to be inout - missing unit scaling for frequency
- put test for negative energt behind debug flag - do energy budget in debug mode - add flags to skip refraction, propagation for debugging - add option to input TKE only at first step for debugging - add checksums for unit scaling testing - rewrite terms in refraction to avoid substractions of velocities - rewrite gradient of coriolis in rotationally invariant form
- MOM_internal_tides gets new routine that converts TKE losses into diffusivities using the TKE_to_Kd array - MOM_set_diffusivities handles the diagnostics
d674e30
to
d4b04fe
Compare
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.
I have inspected the updated code, and I believe that all issues that were identified previously have been resolved with this code contribution. Well done, @raphaeldussin.
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/24768 ✔️ These 15 commits will be squash-merged. |
* (*)bugfixes for internal tides - TKE_itidal_input values are set to zero where it is not applied to the model so that diagnostics are consistent - removed useless halo updates - diagnose energy loss terms as dE/dt instead of equivalent loss rate to properly close energy budget - tot_vel_btTide2 was already a velocity squared, no need to square again - Froude loss term was not properly initialized - add missing halo updates for internal tide speed - compute residual/critical slopes loss only where it has a meaning, allows to clean up noise in field - uh/vh in energy flux routines do not need to be inout * Refactor debugging internal tides - put test for negative energt behind debug flag - do energy budget in debug mode - add flags to skip refraction, propagation for debugging - add option to input TKE only at first step for debugging - add checksums for unit scaling testing - rewrite terms in refraction to avoid substractions of velocities - rewrite gradient of coriolis in rotationally invariant form * Add internal tides diffusivities - MOM_internal_tides gets new routine that converts TKE losses into diffusivities using the TKE_to_Kd array - MOM_set_diffusivities handles the diagnostics * extend find_rho_bottom to get bbl thickness/index * new calculation of BBL, profiles in H units
For better readibility, the PR is split into 3 commits: the first deals with bugfixes and improvements relative to the internal tides energy (changes answers) while the second deals with refactoring of debugging infra. The third commit introduces the TKE loss to diffusivity part.
1st part:
2nd part:
last part: