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

Stokes MOST modifications to unresolved shear #306

Merged

Conversation

gustavo-marques
Copy link
Collaborator

@gustavo-marques gustavo-marques commented Oct 2, 2024

  • Add a control of the Stokes enhancement to the turbulent scales contributing to the unresolved shear
    (Stokes_Vt = 0 for no enhancement; Stokes_Vt = Stokes_Xi for full enhancement);

  • Make a depth restriction to the turbulent velocity scale contribution to the unresolved shear, consistent with the contribution to diffusivity and viscosity (Vt_layer = 1 use full boundary layer depth; vt_layer = surf_layer_ext only includes the surface layer). Stokes MOST parameters are based on Vt_layer = 1;

* Add a control of the Stokes enhancement to the turbulent scales
contributing to the unresolved shear
(Stokes_Vt = 0 for no enhancement; Stokes_Vt = Stokes_Xi for
full enhancement);

* Make a depth restriction to the turbulent velocity scale
contribution to the unresolved shear, consistent with the
contribution to diffusivity and viscosity (Vt_layer = 1 use
full boundary layer depth; vt_layer = surf_layer_ext only
includes the surface layer). Stokes MOST parameters are
based on Vt_layer = 1;

* Replace divisions with reciprocal multiplications.
surfV = surfHv / hTot
surfUs = surfHus / hTot
surfVs = surfHvs / hTot
I_hTot = 1./hTot
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gustavo-marques While this is better for performance, it changes answers, soI reverted it in the previous PR. Are we okay with answer changes, or should we revert this again?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we are okay with answer changes. Thanks!

@gustavo-marques gustavo-marques self-assigned this Oct 11, 2024
@@ -1085,20 +1086,16 @@ subroutine KPP_compute_BLD(CS, G, GV, US, h, Temp, Salt, u, v, tv, uStar, buoyFl
!$OMP uS_SLD, vS_SLD, uS_SLC, vS_SLC, uSbar_SLD, vSbar_SLD, &
!$OMP StokesXI, StokesXI_1d, StokesVt_1d, kbl) &
!$OMP shared(G, GV, CS, US, uStar, h, dz, buoy_scale, buoyFlux, &
!$OMP Temp, Salt, waves, tv, GoRho, GoRho_Z_L2, u, v, lamult)
!$OMP Temp, Salt, waves, tv, GoRho, GoRho_Z_L2, u, v, lamult, Vt_layer)
do j = G%jsc, G%jec
do i = G%isc, G%iec ; if (G%mask2dT(i,j) > 0.0) then

do k=1,GV%ke
U_H(k) = 0.5 * (u(I,j,k)+u(I-1,j,k))
V_H(k) = 0.5 * (v(i,J,k)+v(i,J-1,k))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For multiple tests including SMS.TL319_t232.G1850MARBL_JRA and other, I get the following error:

dec1095.hsn.de.hpc.ucar.edu 2093: forrtl: severe (174): SIGSEGV, segmentation fault occurred
dec1095.hsn.de.hpc.ucar.edu 2093: Image              PC                Routine            Line        Source             
dec1095.hsn.de.hpc.ucar.edu 2093: libpthread-2.31.s  0000147407AC08C0  Unknown               Unknown  Unknown
dec1095.hsn.de.hpc.ucar.edu 2093: cesm.exe           0000000001683D0D  mom_cvmix_kpp_mp_        1095  MOM_CVMix_KPP.F90
dec1095.hsn.de.hpc.ucar.edu 2093: cesm.exe           0000000000D8BF79  mom_diabatic_driv        1326  MOM_diabatic_driver.F90
dec1095.hsn.de.hpc.ucar.edu 2093: cesm.exe           0000000000D7F76E  mom_diabatic_driv         395  MOM_diabatic_driver.F90
dec1095.hsn.de.hpc.ucar.edu 2093: cesm.exe           0000000000C66171  mom_mp_step_mom_t        1608  MOM.F90
dec1095.hsn.de.hpc.ucar.edu 2093: cesm.exe           0000000000C5A936  mom_mp_step_mom_          956  MOM.F90
dec1095.hsn.de.hpc.ucar.edu 2093: cesm.exe           0000000000C21689  mom_ocean_model_n         638  mom_ocean_model_nuopc.F90
dec1095.hsn.de.hpc.ucar.edu 2093: cesm.exe           0000000000BDC250  mom_cap_mod_mp_mo        1845  mom_cap.F90

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to evaluate this in conjunction with CVMix/CVMix-src#97
I apologize for missing this detail in the original description.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we need to update the CVMix submodule with this PR. When you do a git status, I believe you'll have something like:

    modified:   pkg/CVMix-src (new commits)

If so, can you do:

git add pkg/CVMix-src
git commit -m "update CVMix submodule"
git push

Note that this will not only modify CVMix but also change the CVMix remote from mom-ocean organization to CVMix organization, because I think the mom-ocean fork doesn't have those changes yet. Before submitting a MOM6 candidate PR to mom-ocean, we'll need to push these CVMix changes to the CVMix mom-ocean fork, I think. Comments, @mnlevy1981?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened mom-ocean/CVMix-src#1 to get these changes to mom-ocean/CVMix-src; I don't know if we can update dev/ncar before that PR is taken care of, because the git add pkg/CVMix-src will tell git modules to look for commit mom-ocean/CVMix-src@5e3d0ce and I don't know if having the open PR is sufficient for finding that external.

@gustavo-marques
Copy link
Collaborator Author

Once I reverted to the * hTot operations and added an if statement to guard the STOKES_MOST calculation, the results were b4b with the baseline. Here is the path to the pr_mom tests:

/glade/derecho/scratch/gmarques/cesm.tests/pr_mom/cime6.1.29-cesm_cice6_5_0_12-mi_240923_pr306_v4.intel/

@gustavo-marques gustavo-marques merged commit 6184273 into NCAR:dev/ncar Oct 17, 2024
10 checks passed
@mnlevy1981
Copy link
Collaborator

It looks like this is missing the update to the latest CVMix commit

@mnlevy1981 mnlevy1981 mentioned this pull request Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants