-
Notifications
You must be signed in to change notification settings - Fork 388
Fixes CVMix surface layer averaging routine #608
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
Fixes CVMix surface layer averaging routine #608
Conversation
|
@mark-petersen and @qingli411 if you could take a look here, I'd appreciate it. I think this fix will be necessary for the 64 and 80 Layer meshes. However, I'm not sure how big the impact will be, I'm running a sensitivity test now and will post when it is complete. |
qingli411
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.
@vanroekel I think these changes look good to me. And I agree these changes are necessary for the stretching grid!
|
@vanroekel This is needed for the upcoming water cycle simulations, correct? And this must be a non-bfb change to all E3SM simulations? Any comments on your sensitivity tests on this PR? |
|
I realize I forgot to finish analysis of the control case. I'll post sensitivity results here once mpas-analysis runs. |
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.
We have a layerThicknessEdge variable in the Registry. Of course this is a little calculation, but layerThicknessEdge(iEdge) has a better memory access for an edge loop. I can change that.
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 for catching that. Not sure why I didn't think of that array! Thanks for making the change too
Currently when cvmix computes averages for the surface layer (needed for finding boundary layer depth) it simply does an arithmetic mean. This is reasonable for the 60 layer mesh (with near constant 10m resolution through 200m), but is not correct for stretched grids. Here the averaging is changed to a layerThickness weighted average.
|
testing in e3sm suggests this will be non-BFB but only slightly climate changing (at least for 6030 resolution). Here are the analysis |
91c8fc1 to
7a953d5
Compare
|
@vanroekel I made the change to use the I also changed the max vertical index from maxLevelCell to maxLevelEdgeTop, which makes more sense when you are computing on an edge. When on an edge below maxLevelEdgeTop but above maxLevelCell (edge neighboring water and land) the normalVelocity is zero and layerThicknessEdge is -1e34, so this should be the same because of the zero velocity. These changes seem trivial, but since they are non-bfb, I think we should either retest in E3SM to be sure, or remove them, and stick with your original commit. You can do whichever you prefer. |
mark-petersen
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.
@vanroekel confirmed that testing in E3SM looks good with the latest changes here.
…#3695) Update mpas-source: Fixes CVMix surface layer averaging This PR brings in a new mpas-source submdoule with changes only to the ocean core. Currently when cvmix computes averages for the surface layer (needed for finding boundary layer depth) it simply does an arithmetic mean. This is reasonable for the 60 layer mesh (with near constant 10m resolution through 200m), but is not correct for stretched grids. In this PR the averaging is changed to a layerThickness weighted average. See MPAS-Dev/MPAS-Model#608 [non-BFB]
Update mpas-source: Fixes CVMix surface layer averaging This PR brings in a new mpas-source submdoule with changes only to the ocean core. Currently when cvmix computes averages for the surface layer (needed for finding boundary layer depth) it simply does an arithmetic mean. This is reasonable for the 60 layer mesh (with near constant 10m resolution through 200m), but is not correct for stretched grids. In this PR the averaging is changed to a layerThickness weighted average. See MPAS-Dev/MPAS-Model#608 [non-BFB]
Currently when cvmix computes averages for the surface layer (needed for
finding boundary layer depth) it simply does an arithmetic mean. This
is reasonable for the 60 layer mesh (with near constant 10m resolution
through 200m), but is not correct for stretched
grids. Here the averaging is changed to a layerThickness weighted
average.