Skip to content

Conversation

@nairita87
Copy link

@nairita87 nairita87 commented Jun 20, 2020

Add NCOM's formula for bottom drag coefficient calculation,
the formula is cb(i,j) = max(cbmin, (vonk/log(0.5*depth(i,j)/z0))**2 ))

@philipwjones
Copy link
Contributor

Hey all. Not a reviewer, but I did just refactor these impvmix routines for GPUs (see the ocean/gpuimpvmix branch in my philipwjones fork) and have a few comments. For both performance reasons and eliminating the need for all these separate routines, it's actually better to compute the bottom drag term as an (nEdges) array in a separate loop before computing the ABC coefficients.

Second, as a general rule, don't use real numbers as exponents if they are in fact integers. The algorithm for computing real exponentiation is more expensive than the integer form so you want the compiler to choose the right form. This is especially true if the exponent is 2 or 3 where the compiler will often replace with a simple multiply (or even better, for a simple square, do the replacement yourself and avoid the exponentiation).

For fixed constant real numbers, be sure to add the _RKIND suffix (eg 0.5_RKIND) to ensure correct precision - you may only get single precision otherwise. Finally, try to combine/precompute some of these factors (especially divisions). Although flops are cheap, they are not free and precomputing common factors almost always wins.

Copy link
Contributor

@pwolfram pwolfram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nairita87 ! I just have a few minor comments...

@sbrus89 sbrus89 force-pushed the ocean/coastal branch 2 times, most recently from 025c036 to acea49c Compare August 19, 2020 18:00
@proteanplanet proteanplanet added tides All developments of tides in MPAS Ocean and MPAS Sea Ice and removed Ocean labels Oct 9, 2020
@mark-petersen mark-petersen changed the base branch from ocean/coastal to ocean/develop October 16, 2020 20:16
@mark-petersen mark-petersen self-requested a review October 16, 2020 20:16
@mark-petersen mark-petersen changed the title MPAS-O PR for bottom drag coefficient calculation Add NCOM bottom drag formulation Oct 16, 2020
@mark-petersen mark-petersen self-requested a review March 5, 2021 12:27
@mark-petersen
Copy link
Contributor

Rebased to head of ocean/develop. Passes nightly test suite.

@nairita87
Copy link
Author

The PR has been verified against previous studies (Pringle et al 2021) and also with TPXO8 data.
Attached are plots to show such comparisons.
m2amp_ra4topo4

The following plot can be compared with Pringle et al 2021 (Figs 5 and 6)
standard_m2amp_ra4topo4

Below is the plot showing code agreement before and after rebasing
rebase

if (config_use_implicit_bottom_roughness) then
implicitCd = max(0.0025_RKIND, &
min(0.1_RKIND, &
0.16_RKIND/log(250.0_RKIND*(bottomDepth(cell1)+bottomDepth(cell2)))**2 ))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nairita87 please check this. I consolidated these two lines, putting min and max together, and removing the varmin variable.

Also, 0.5*0.5/0.001 = 250, so I just pre-computed the math.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mark-petersen , Mark, I verified, passes the test nicely
commit_c7583d8

@mark-petersen
Copy link
Contributor

mark-petersen commented Apr 28, 2021

The changes here were absorbed into #607, which is also on drag, so they could be tested together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants