-
Notifications
You must be signed in to change notification settings - Fork 388
Add NCOM bottom drag formulation #606
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
Conversation
|
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. |
pwolfram
left a comment
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.
Thanks @nairita87 ! I just have a few minor comments...
025c036 to
acea49c
Compare
7e66df9 to
81dd341
Compare
81dd341 to
1565779
Compare
|
Rebased to head of |
… compilation bug)
1565779 to
2914fa7
Compare
| 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 )) |
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.
@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.
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.
@mark-petersen , Mark, I verified, passes the test nicely

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



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