Skip to content
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

cvmix tidal mixing default value is incorrect #875

Open
mark-petersen opened this issue May 18, 2021 · 3 comments
Open

cvmix tidal mixing default value is incorrect #875

mark-petersen opened this issue May 18, 2021 · 3 comments
Labels

Comments

@mark-petersen
Copy link
Contributor

From Mehmet Ilicak, by email:
I was coding CVmix into the NorESM model and I realized a default value in tidal mixing scheme is wrong.

Here is the github issue link: CVMix/CVMix-src#91

Before I opened that issue, I was searching google just in case if I was making a mistake, and I saw this link about MPAS;
https://oceans11.lanl.gov/mpas_data/mpas_ocean/doxygen/release_3.0/mpas__ocn__vmix__cvmix_8_f_source.html

I am not sure how much it is up to date, or if you guys are using CVmix at all or not, but it looks like MPAS is using default values;

!      ! initialize tidal mixing
  688  ! (at present, tidal mixing can only use CVMix default parameter settings)
  689  !
  690  if (config_use_cvmix_tidal_mixing) then
  691  call cvmix_init_tidal(cvmix_tidal_params,'Simmons')
  692  endif
@mark-petersen
Copy link
Contributor Author

@vanroekel @caozd999 @sbrus89
This line:
https://github.com/MPAS-Dev/MPAS-Model/blob/ocean/develop/src/core_ocean/shared/mpas_ocn_vmix_cvmix.F#L975

call cvmix_init_tidal(cvmix_tidal_params,'Simmons')

was originally written by Todd in 2013. From the comments at CVMix/CVMix-src#91, it looks like the cvmix subroutine uses an incorrect value of 3, and should be 1/3. Until cvmix is corrected, the fix in MPAS-Ocean would be to add the flag

call cvmix_init_tidal(cvmix_tidal_params,'Simmons', &
       local_mixing_frac=0.333333_RKIND)

CESM-POP uses a similar argument here:
https://github.com/ESCOMP/POP2-CESM/blob/cesm_pop_2_1_20210419/source/tidal_mixing.F90#L1150

@vanroekel
Copy link
Contributor

Thanks for noting this @mark-petersen. Indeed that should be changed. But I wasn't aware anyone was actually using that function? The mpas cvmix code only has the tidal init hooks not the actual diffusivity calculation. I'm actually working on them under water cycle but haven't finalized the code, so many thanks for bringing this to my attention! I hope to make a PR in a week or two and will fold this in.

@sbrus89 or @caozd999 are either of you working with (or implementing) the CVMix tidal scheme? I don't want to duplicate work.

@sbrus89
Copy link

sbrus89 commented May 18, 2021

@vanroekel - I haven't been working with the CVMix tidal scheme, so you're not duplicating anything on my end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants