+Fix dimensional rescaling with HARMONICS_SAL#503
Merged
marshallward merged 2 commits intoOct 30, 2023
Conversation
Hallberg-NOAA
added
bug
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #503 +/- ##
=========================================
Coverage 37.52% 37.53%
=========================================
Files 270 270
Lines 79059 79064 +5
Branches 14629 14630 +1
=========================================
+ Hits 29670 29673 +3
- Misses 43923 43925 +2
Partials 5466 5466
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Hallberg-NOAA
force-pushed
the
spherical_harm_scaling_fix
branch
from
0295d4e to
7a65f46
Compare
Member
|
@herrwang Do you mind reviewing these changes? |
herrwang0
approved these changes
Oct 27, 2023
Corrected dimensional rescaling bugs in the spherical harmonics SAL code. An issue with horizontal length scaling was corrected by using G%Rad_Earth_L in place of G%Rad_Earth in spherical_harmonics_init. There are new optional tmp_scale arguments to calc_SAL and spherical_harmonics_forward to allow the rescaling to be undone before calling the reproducing sums. This commit also modifies the call to the reproducing sums in spherical_harmonics_forward so that all real or imaginary components are calculated with a single call, which reduces the cost of the SAL calculation reproducing sums from about 6.7 times the cost with non-reproducing sums to just 5.5 times as much in testing with the tides_025 test case. There is also code added to avoid NaNs arising from a square root operating on a negative argument from a 32-bit integer roll-over when a very large number of harmonics components (more than 1024 x 1024) are unadvisedly being used. While this commit corrects the dimensional scaling when HARMONICS_SAL is true, all answers are bitwise identical when no rescaling is used or when the spherical harmonics SAL is not used. There are new optional arguments to two publicly visible interfaces.
Hallberg-NOAA
force-pushed
the
spherical_harm_scaling_fix
branch
from
7a65f46 to
b589181
Compare
Member
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/21155 ✔️ |
This was referenced Nov 13, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Corrected dimensional rescaling bugs in the spherical harmonics SAL code. An issue with horizontal length scaling was corrected by using G%Rad_Earth_L in place of G%Rad_Earth in spherical_harmonics_init. There are new optional tmp_scale arguments to calc_SAL and spherical_harmonics_forward to allow the rescaling to be undone before calling the reproducing sums.
This commit also modifies the call to the reproducing sums in spherical_harmonics_forward so that all real or imaginary components are calculated with a single call, which reduces the cost of the SAL calculation reproducing sums from about 6.7 times the cost with non-reproducing sums to just 5.5 times as much in testing with the tides_025 test case.
There is also code added to avoid NaNs arising from a square root operating on a negative argument from a 32-bit integer roll-over when a very large number of harmonics components (more than 1024 x 1024) are unadvisedly being used.
While this commit corrects the dimensional scaling when HARMONICS_SAL is true, all answers are bitwise identical when no rescaling is used or when the spherical harmonics SAL is not used. There are new optional arguments to two publicly visible interfaces.