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.

Sign up for GitHub

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

Jump to bottom

add distinct passive tracer background vert diffusivity #6263

Merged
merged 3 commits into from
May 10, 2024
Merged

add distinct passive tracer background vert diffusivity #6263

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48bff80
add distinct passive tracer background vert diffusivity
maltrud Feb 26, 2024
5c6fdd5
added explicit k-loop for calculating vertDiffPassive
maltrud May 8, 2024
b25883b
Make bld files consistent with mpaso Registry using automated scripts
jonbob May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions components/mpas-ocean/bld/build-namelist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,8 @@ add_default($nl, 'config_use_cvmix');
add_default($nl, 'config_cvmix_prandtl_number');
add_default($nl, 'config_cvmix_background_scheme');
add_default($nl, 'config_cvmix_background_diffusion');
add_default($nl, 'config_cvmix_background_diffusion_passive_enable');
add_default($nl, 'config_cvmix_background_diffusion_passive');
add_default($nl, 'config_cvmix_background_viscosity');
add_default($nl, 'config_cvmix_BryanLewis_bl1');
add_default($nl, 'config_cvmix_BryanLewis_bl2');
Expand Down
2 changes: 2 additions & 0 deletions components/mpas-ocean/bld/namelist_files/namelist_defaults_mpaso.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@
<config_cvmix_prandtl_number>1.0</config_cvmix_prandtl_number>
<config_cvmix_background_scheme>'constant'</config_cvmix_background_scheme>
<config_cvmix_background_diffusion>0.0</config_cvmix_background_diffusion>
<config_cvmix_background_diffusion_passive_enable>.false.</config_cvmix_background_diffusion_passive_enable>
<config_cvmix_background_diffusion_passive>0.0</config_cvmix_background_diffusion_passive>
<config_cvmix_background_viscosity>1.0e-4</config_cvmix_background_viscosity>
<config_cvmix_BryanLewis_bl1>8.0e-5</config_cvmix_BryanLewis_bl1>
<config_cvmix_BryanLewis_bl2>1.05E-4</config_cvmix_BryanLewis_bl2>
Expand Down
16 changes: 16 additions & 0 deletions components/mpas-ocean/bld/namelist_files/namelist_definition_mpaso.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,22 @@ Valid values: Any positive real value.
Default: Defined in namelist_defaults.xml
</entry>

<entry id="config_cvmix_background_diffusion_passive_enable" type="logical"
category="cvmix" group="cvmix">
Enable different background vertical diffusion value for passive tracer quantities

Valid values: .true. or .false.
Default: Defined in namelist_defaults.xml
</entry>

<entry id="config_cvmix_background_diffusion_passive" type="real"
category="cvmix" group="cvmix">
Background vertical diffusion applied to passive tracer quantities if enabled

Valid values: Any positive real value.
Default: Defined in namelist_defaults.xml
</entry>

<entry id="config_cvmix_background_viscosity" type="real"
category="cvmix" group="cvmix">
Background vertical viscosity applied to horizontal velocity
Expand Down
12 changes: 12 additions & 0 deletions components/mpas-ocean/src/Registry.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,14 @@
description="Background vertical diffusion applied to tracer quantities"
possible_values="Any positive real value."
/>
<nml_option name="config_cvmix_background_diffusion_passive" type="real" default_value="1.0e-5" units="m^2 s^-1"
description="Background vertical diffusion applied to passive tracer quantities"
possible_values="Any positive real value."
/>
<nml_option name="config_cvmix_background_diffusion_passive_enable" type="logical" default_value=".false."
description="flag to enable using a different background vertical diffusion for passive tracers"
possible_values=".true. or .false."
/>
<nml_option name="config_cvmix_background_viscosity" type="real" default_value="1.0e-4" units="m^2 s^-1"
description="Background vertical viscosity applied to horizontal velocity"
possible_values="Any positive real value."
Expand Down Expand Up @@ -3085,6 +3093,10 @@
description="vertical diffusion defined at the cell center (horizontally) and top (vertically)"
packages="forwardMode;analysisMode"
/>
<var name="vertDiffPassiveTopOfCell" type="real" dimensions="nVertLevelsP1 nCells Time" units="m^2 s^-1"
description="vertical diffusion defined at the cell center (horizontally) and top (vertically) for passive tracers"
packages="forwardMode;analysisMode"
/>
<var name="bulkRichardsonNumber" type="real" dimensions="nVertLevels nCells Time" units="1"
description="CVMix/KPP: bulk Richardson number"
missing_value="FILLVAL" missing_value_mask="cellMask"
Expand Down
6 changes: 6 additions & 0 deletions components/mpas-ocean/src/shared/mpas_ocn_diagnostics_variables.F
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ module ocn_diagnostics_variables
type (field2DReal), pointer :: wettingVelocityField

real (kind=RKIND), dimension(:,:), pointer :: vertDiffTopOfCell
real (kind=RKIND), dimension(:,:), pointer :: vertDiffPassiveTopOfCell
real (kind=RKIND), dimension(:,:,:), pointer :: vertNonLocalFlux
integer, dimension(:,:), pointer :: rediLimiterCount
real (kind=RKIND), dimension(:,:,:), pointer :: activeTracerSurfaceFluxTendency
Expand Down Expand Up @@ -494,6 +495,8 @@ subroutine ocn_diagnostics_variables_init(domain, jenkinsOn, hollandJenkinsOn, e
vertViscTopOfCell)
call mpas_pool_get_array(diagnosticsPool, 'vertDiffTopOfCell', &
vertDiffTopOfCell)
call mpas_pool_get_array(diagnosticsPool, 'vertDiffPassiveTopOfCell', &
vertDiffPassiveTopOfCell)
call mpas_pool_get_array(diagnosticsPool, 'vertAleTransportTop', &
vertAleTransportTop)
call mpas_pool_get_array(diagnosticsPool, 'vertTransportVelocityTop', &
Expand Down Expand Up @@ -752,6 +755,7 @@ subroutine ocn_diagnostics_variables_init(domain, jenkinsOn, hollandJenkinsOn, e
!$acc barotropicForcing, &
!$acc vertViscTopOfCell, &
!$acc vertDiffTopOfCell, &
!$acc vertDiffPassiveTopOfCell, &
!$acc GMStreamFuncZonal, &
!$acc relativeVorticity, &
!$acc kineticEnergyCell, &
Expand Down Expand Up @@ -999,6 +1003,7 @@ subroutine ocn_diagnostics_variables_destroy(err) !{{{
!$acc barotropicForcing, &
!$acc vertViscTopOfCell, &
!$acc vertDiffTopOfCell, &
!$acc vertDiffPassiveTopOfCell, &
!$acc GMStreamFuncZonal, &
!$acc relativeVorticity, &
!$acc kineticEnergyCell, &
Expand Down Expand Up @@ -1204,6 +1209,7 @@ subroutine ocn_diagnostics_variables_destroy(err) !{{{
barotropicForcing, &
vertViscTopOfCell, &
vertDiffTopOfCell, &
vertDiffPassiveTopOfCell, &
GMStreamFuncZonal, &
relativeVorticity, &
kineticEnergyCell, &
Expand Down
19 changes: 18 additions & 1 deletion components/mpas-ocean/src/shared/mpas_ocn_vmix.F
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1583,9 +1583,26 @@ subroutine ocn_vmix_implicit(dt, meshPool, statePool, forcingPool, scratchPool,
!$acc enter data copyin(tracerGroupSurfaceFlux)
#endif

call ocn_tracer_vmix_tend_implicit(dt, vertDiffTopOfCell, layerThickness, tracersGroup, &
if (trim(groupItr % memberName) /= 'activeTracers' .and. &
config_cvmix_background_diffusion_passive_enable) then
vertDiffPassiveTopOfCell = vertDiffTopOfCell - &
config_cvmix_background_diffusion + config_cvmix_background_diffusion_passive
Copy link
Contributor

Choose a reason for hiding this comment

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

@maltrud this probably doesn't impact the simulation but this does create a nonzero diffusivity at the surface and bottom of the ocean, which I don't think we want in the code. The change should only be from k=2:maxLevelCell

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, this should be in explicit loops over cells as well as k=2:maxLevelCell. Implicit indexing hides everything, and does not include the acc and openmp pragmas. The loop should look like, e.g., line 1553 in mpas_ocn_vmix.F, with all the lengthy pragmas and ifdefs around it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vanroekel and @mark-petersen thanks for finding this. I'll clean it up--array syntax has become second nature to me so going back to loops often doesn't occur to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does this look correct to you? it compiled ok and I'll make sure it runs.

#ifdef MPAS_OPENACC
!$acc parallel loop gang vector collapse(3) present(vertDiffPassiveTopOfCell, vertDiffTopOfCell)
#else
!$omp parallel
!$omp do schedule(runtime) private(k, iTracer)
#endif
do iCell = 1, nCellsOwned
do k = 2, maxLevelCell(iCell)
vertDiffPassiveTopOfCell(k,iCell) = vertDiffTopOfCell(k,iCell) - &
config_cvmix_background_diffusion + config_cvmix_background_diffusion_passive
end do
end do
#ifndef MPAS_OPENACC
!$omp end do
!$omp end parallel
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should the $acc directive also have maxLevelCell?

also a general question: in the loop @mark-petersen references as an example (thanks for that!) the k loop is 1:nVertLevels, not 1:maxLevelCell(iCell). I realize this is a bit different (we don't want a k=1 value for diffusivity) but why not loop over 1:maxLevelCell?

Copy link
Contributor

Choose a reason for hiding this comment

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

@maltrud you won't be able to collapse these loops with maxLevelCell as a loop limit. For maximum parallelism, you can either use fixed loop (2:nVertLevels) and add a conditional inside (if k < maxLevelCell) or use a mask. Otherwise, you can have just one level of parallelism over cells and leave the loop limit variable. And yes, you'll need maxLevelCell in the present() list. You also need to change collapse to either (2) in the first case or remove it if you only parallelize the cell loop. Finally, you should verify that the arrays in the present() list have all been copied to the device at some point (all mesh related variables like maxLevel are always on device and vertDiffTopOfCell I think has been copied earlier, but don't know if you added VertDiffPassive...)

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 your help, @philipwjones. I think I'll opt for limits of 2:nVertLevels and adding a conditional, though I assume that would affect vectorization. would using 'merge' help for vectorization? eg:

vertDiffPassiveTopOfCell(k,iCell) = merge( vertDiffTopOfCell(k,iCell) -....,0.0_RKIND,k<maxLevelCell(k,iCell))

how can I tell if something is present? I see $acc update host for vertDiffTopOfCell in ocn_vmix_coefs, then $acc update device after the coefficients are calculated. do I need to do something similar for vertDiffPassiveTopOfCell? I did put in $acc update host for vertDiffPassiveTopOfCell but I suspect that isn't correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

@maltrud it may affect CPU/vector performance but I suspect this loop has a fairly small cost anyway. The merge probably won't be any better - this is a simple enough loop the compiler should figure it out anyway and sometimes the intrinsic is actually worse. I didn't look closely at the data transfers when I wrote that last, but what you have looks correct - this is computed on the device so you just need to update the host which you've done. And you added it to the create/destroy calls in diagnostic_vars so should be fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

For multiple passive tracers this line of code is redundant. It could be placed above the tracer loop, but it is way up there, so for a few wasted flops this more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mark-petersen yes I debated this one in my head a bit, and also decided clarity might might be worth some redundancy. I'll make a comment in the code but leave the logic as-is if that's OK with you.

#ifdef MPAS_OPENACC
!$acc update host (vertDiffPassiveTopOfCell)
#endif
!maltrud debug
do k = 1, maxLevelCell(1)
write(*,*)' DEBUG01: ',k,vertDiffPassiveTopOfCell(k,1)
enddo
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing you meant to remove these debug lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vanroekel , yeah, thanks for catching that. will remove.


call ocn_tracer_vmix_tend_implicit(dt, vertDiffPassiveTopOfCell, layerThickness, tracersGroup, &
vertNonLocalFlux, tracerGroupSurfaceFlux, &
config_cvmix_kpp_nonlocal_with_implicit_mix, err)
else
call ocn_tracer_vmix_tend_implicit(dt, vertDiffTopOfCell, layerThickness, tracersGroup, &
vertNonLocalFlux, tracerGroupSurfaceFlux, &
config_cvmix_kpp_nonlocal_with_implicit_mix, err)
end if
#ifdef MPAS_OPENACC
!$acc exit data delete(tracerGroupSurfaceFlux)
#endif
Expand Down