Skip to content
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

Open
wants to merge 15 commits into
base: dev/gfdl
Choose a base branch
from

Conversation

raphaeldussin
Copy link

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:

(*)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

- missing unit scaling for frequency

2nd part:

Refactor debugging internal tides

- put test for negative energy 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

last part:

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

@raphaeldussin raphaeldussin force-pushed the int_tides_fixes branch 2 times, most recently from ed2353e to f6c9ef7 Compare July 2, 2024 19:07
Copy link

codecov bot commented Jul 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.90%. Comparing base (9351353) to head (f6c9ef7).

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.
📢 Have feedback on the report? Share it here.

@raphaeldussin
Copy link
Author

Energy budget is closing:

image

@raphaeldussin
Copy link
Author

also reproduces over processor counts with N = 480 and N = 441

@raphaeldussin raphaeldussin marked this pull request as draft July 23, 2024 20:30
@raphaeldussin raphaeldussin force-pushed the int_tides_fixes branch 3 times, most recently from 1ede7e1 to 5bb32cc Compare July 30, 2024 02:48
@raphaeldussin raphaeldussin marked this pull request as ready for review July 30, 2024 03:57
@raphaeldussin
Copy link
Author

@Hallberg-NOAA PR passes unit scaling tests in Boussinesq .and. Non-Boussinesq

Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a 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.

@raphaeldussin raphaeldussin marked this pull request as draft August 1, 2024 22:09
@raphaeldussin raphaeldussin marked this pull request as ready for review August 19, 2024 13:08
Raphael Dussin added 14 commits September 6, 2024 17:26
- 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
Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants