-
Notifications
You must be signed in to change notification settings - Fork 230
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
User/bgr/pe mld diag #1215
User/bgr/pe mld diag #1215
Conversation
- New MLD is computed as the depth a given energy would homogenize the water column - Three values are coded for output, 25 J/m2, 2500 J/m2, and 250000 J/m2.
- Fixing whitespace - Fixing parenthesis in A*B/C to (A*B)/C - Moving /grav to *igrav (and computing threshold outside of i,j loop).
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #1215 +/- ##
============================================
+ Coverage 46.08% 46.17% +0.08%
============================================
Files 214 224 +10
Lines 69399 70690 +1291
============================================
+ Hits 31984 32642 +658
- Misses 37415 38048 +633
Continue to review full report at Codecov.
|
I'm working on the iteration, potentially will offer an improvement soon, and I also notice that Travis is failing so will need to figure out why (might be just signaling the new diagnostics). But I am wondering, is the empty line at the beginning of MOM_diabatic_aux.F90 intentional? My whitespace macro keeps removing it so I have to manually reinsert it. It is missing again in my last commit so wanted to check before I reinsert it again. |
Yes delete the blank line. |
The new diagnostics should no longer signal an error. In fact, this is passing the regression tests. Most of the reported fails are dimensional. My guess is many of those hard-coded constants need dimensions applied to them. Could also be that the diagnostics were improperly scaled? |
Thanks, good to know. Will double check if that -1.e8 is triggering this or if it is in the scaling of the diagnostics (my hunch). |
…ser/bgr/PE_MLD_Diag
…ator instead of a secant method.
The latest commit is a complete rewrite of the original commit, where the iterator used to calculate the fraction of a layer that could be mixed with a given energy has been replaced. This should be faster and more accurate. |
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 am very happy with this PR, especially now that it is using Newton's method and is finding the diagnostic mixed layer depth very quickly. It is a valuable diagnostic addition.
There are couple of places with implied array copies from non-stride-1 array syntax arguments that might be addressed for efficiency (e.g., line 792 of MOM_diabatic_aux.F90), and there is a curious empty logical test at lines 3451-3452 of MOM_diabatic_driver.F90 that probably should be deleted, but these are minor things that could be cleaned up later.
Thanks for the review. I can delete the lines at 3451, it is leftover code that should have been removed. |
Adding new mixed layer depth diagnostics.