CVMix / CVMix-src

CVMix source code (including protex documentation) as well as build system and examples / tests
Other
30 stars 30 forks source link

Prevent contribution of OBL to interior diffusivity when lenhanced_di… #64

Closed breichl closed 8 years ago

breichl commented 8 years ago

…ff set to false.

Located a potential bug when "lenhanced_diff" is set to false. In this case, the diffusivity/viscosity at the interface below the boundary layer should only be due to the interior scheme and the boundary layer contribution should be 0. However, what appears to be a bug caused the shape function to be evaluated for the first interface in which sigma>1, which caused the OBL scheme to contribute to diffusivity/viscosity at this first interior interface. This gave strange results causing the boundary layer to "jump" periodically as these artificial diffusivities mixed interior layers into the boundary layer. If "enhanced_diff" is enabled, this value would be correctly replaced before the diffusivity is output, but if it is disabled, this value makes it into the output diffusivity profile.

See my proposed fix, where I set the shape function equal to 0 when sigma exceeds or equals 1. The proposed fix zeros out the shape function for case sigma>=1, preventing this artificial diffusivity.

vanroekel commented 8 years ago

I checked out this fix and plugged it into MPAS, running the free convection test. The boundary layer depths from a few experiments are below

bldconvectnew

Here the green line is the current CVMix source, which has enhanced_diff off. The dashed green is current CVMix with matchBoth, which enables enhanced_diff. The solid yellow is brandon's fix with enhanced_diff on, and the dashed yellow is the fix without enhanced diff (both of these tests use simpleShapes). The solid blue is LES and the dashed blue is an analytic result. The fix greatly reduces the stair case structure seen in the baseline experiment.

mnlevy1981 commented 8 years ago

I'd like to close this pull request in favor of accepting #68 - thoughts?

breichl commented 8 years ago

I am satisfied with 68 as a replacement to 64. Please feel free to close this request. -Brandon

On Sun, Jul 10, 2016 at 11:18 AM, Michael Levy notifications@github.com wrote:

I'd like to close this pull request in favor of accepting #68 https://github.com/CVMix/CVMix-src/pull/68 - thoughts?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/CVMix/CVMix-src/pull/64#issuecomment-231594240, or mute the thread https://github.com/notifications/unsubscribe/AJQH2PZWvQUo7OQCPEuAqlh1kgC71kUUks5qUQ1jgaJpZM4INBfw .

vanroekel commented 8 years ago

I'm o kay with this change as well.

Luke

On Sun, Jul 10, 2016 at 9:18 AM, Michael Levy notifications@github.com wrote:

I'd like to close this pull request in favor of accepting #68 https://github.com/CVMix/CVMix-src/pull/68 - thoughts?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/CVMix/CVMix-src/pull/64#issuecomment-231594240, or mute the thread https://github.com/notifications/unsubscribe/ANB5UcSh1jCVuBa6JzFnjHfIKkxDx_Aaks5qUQ1jgaJpZM4INBfw .

mnlevy1981 commented 8 years ago

merged #68, closing this request. Fix is available starting in v0.84-beta