Skip to content

Conversation

@vanroekel
Copy link
Contributor

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.

@vanroekel
Copy link
Contributor Author

@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.

Copy link

@qingli411 qingli411 left a 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!

@mark-petersen mark-petersen self-assigned this Jul 8, 2020
@mark-petersen
Copy link
Contributor

@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?

@vanroekel
Copy link
Contributor Author

I realize I forgot to finish analysis of the control case. I'll post sensitivity results here once mpas-analysis runs.

Copy link
Contributor

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.

Copy link
Contributor Author

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.
@vanroekel
Copy link
Contributor Author

testing in e3sm suggests this will be non-BFB but only slightly climate changing (at least for 6030 resolution). Here are the analysis

Baseline (control) 64 layer case -- https://portal.nersc.gov/project/e3sm/lvroekel/20200618_64LayerNewRedi_GMPAS-IAF.T62_oECv3_anvil_21_30/

with the fix -- https://portal.nersc.gov/project/e3sm/lvroekel/20200618_64LayerNewRediKPPfix_GMPAS-IAF.T62_oECv3_anvil_10_30/

@mark-petersen mark-petersen force-pushed the ocean/fixCVMixInterpolation branch from 91c8fc1 to 7a953d5 Compare July 10, 2020 15:04
@mark-petersen
Copy link
Contributor

mark-petersen commented Jul 10, 2020

@vanroekel I made the change to use the layerThicknessEdge array, commit edc25c6. I tested before/after and was concerned that it was not bfb between the two. I suspect that layerThicknessEdge is computed at the end of the step, so there is a time offset that doesn't matter much.

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.

Copy link
Contributor

@mark-petersen mark-petersen left a 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.

@mark-petersen mark-petersen merged commit 82eb734 into MPAS-Dev:ocean/develop Jul 14, 2020
jonbob added a commit to E3SM-Project/E3SM that referenced this pull request Jul 14, 2020
…#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]
jonbob added a commit to E3SM-Project/E3SM that referenced this pull request Jul 15, 2020
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]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants