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

Add SAL updates and topographic wave drag schemes for barotropic tides #6310

Merged
merged 19 commits into from
May 20, 2024

Conversation

sbrus89
Copy link
Contributor

@sbrus89 sbrus89 commented Mar 27, 2024

This PR incorporates updates to the global tidal forcing in MPAS-Ocean. It combines two contributions developed under the ICoM project:

  1. @dengwirda's modifications to a) include surface pressure in the self attraction and loading (SAL) calculations, b) use a depth rather than distance-based criteria in the SAL coastal smoothing factor, and c) update the harmonic analysis AM to account for initial SSH offsets. See also: Self-attraction-and-loading and harmonic analysis for SSH != 0 E3SM-Ocean-Discussion/E3SM#52.
  2. @knbarton's refactoring of the existing Jayne and St. Laurent topographic wave drag (TWD) parameterization and the addition of the Zaron and Egbert and Local Generation Formula TWD schemes.

This PR has been tested using a new compass case, which performs a 125-day tidal run (with a 90 day analysis period) on the VR45to5 mesh using the Zaron and Egbert topographic wave drag option. This results in a 3.3 cm M2 deep water RMSE as compared to the TPXO9 tidal database, which is the most accurate MPAS-Ocean tidal result to date. Here is a link to the compass PR with testing results: MPAS-Dev/compass#802

[NML]
[BFB] - mpas-ocean standalone

dengwirda and others added 16 commits March 26, 2024 12:35
- Include pressure forcing term in SAL computation to account for
  under isc and atm pressure cases.

- Update SAL coastal smoothing factor to mask shallow cells where
  wet-dry effects may occur out of SAL computation.

- Compute harmonic analysis AM wrt. SSH-SSH_init to account for
  initially non-zero SSH distributions.
- Clean up descriptions, comments and use real-valued switches.
- Change sshSmoothed to btrPressure in SAL routines, since we're
  now working with surface pressures too.
@sbrus89 sbrus89 added the MPAS-Ocean standalone Issues and features for standalone MPAS-Ocean code that dont impact E3SM. label Mar 27, 2024
@sbrus89 sbrus89 added mpas-ocean BFB PR leaves answers BFB labels Mar 27, 2024
Comment on lines -672 to +693
! Set up coastal ssh smoothing
allocate(sshSmoothed(nCellsAll))

if (config_self_attraction_loading_smoothing_width > 2.0_RKIND) then
trans_width = config_self_attraction_loading_smoothing_width*1000.0_RKIND
trans_start = 0.0_RKIND
do iCell = 1,nCellsAll
d = distanceToCoast(iCell)
coastalSmoothingFactor(iCell) = 0.5_RKIND*(tanh((d - trans_start - 0.5_RKIND * trans_width) / (0.2_RKIND *trans_width)) + 1.0_RKIND)
enddo
else
coastalSmoothingFactor(:) = 1.0_RKIND
endif
! Set up coastal smoothing
allocate(btrPressure(nCellsAll))

! Set-up smoothing mask for shallow regions where SAL should not
! be computed across e.g. wet-dry cells
depth_tol = config_self_attraction_loading_depth_cutoff
do iCell = 1,nCellsAll
depth = ssh(iCell) + bottomDepth(iCell)
coastalSmoothingFactor(iCell) = 0.5_RKIND*(tanh( &
(depth - 0.5_RKIND*depth_tol) / (0.2_RKIND*depth_tol)) + 1.0_RKIND)
enddo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the switch from distance-based to depth-based SAL smoothing

Comment on lines -174 to +181
sshSmoothed(iCell) = coastalSmoothingFactor(iCell)*ssh(iCell)
! compute SAL wrt. the full barotropic bottom pressure
btrPressure(iCell) = coastalSmoothingFactor(iCell) * ( &
ssh(iCell) + rho0gInv * surfacePressure(iCell) )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where the surface pressure is added to the SAL calculation

Comment on lines +369 to +374
allocate(sshDiff(nCellsSolve))

do iCell = 1,nCellsSolve
sshDiff(iCell) = ssh(iCell) - sshInit(iCell)
end do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the change that makes the harmonic analysis work on the ssh anomaly.

Comment on lines 1978 to 1981
<var name="temp_twd"/>
<var name="normalCoeff"/>
<var name="tangentialCoeff"/>
<var name="topographicWaveDrag"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe these need to be in the restart stream, since they are calculated in an init routine

Comment on lines 258 to 269
topographicWaveDrag = topographicWaveDragCoeff &
* topoDragTurnOff &
* (sqrt((topo_buoyancy_N1B(iEdge)**2 - omegaM2**2) * &
(topo_buoyancy_N1V(iEdge)**2 - omegaM2**2))) &
/ (4*pii*omegaM2)
normalCoeff = (lonGradEdge(iEdge)**2) * &
(cos(angleEdge(iEdge))**2) + (latGradEdge(iEdge)**2)*(sin(angleEdge(iEdge))**2) - &
2*latGradEdge(iEdge)*lonGradEdge(iEdge)*sin(angleEdge(iEdge))*(cos(angleEdge(iEdge)))
tangentialCoeff = sin(angleEdge(iEdge)) * &
cos(angleEdge(iEdge)) * &
(lonGradEdge(iEdge)**2 - latGradEdge(iEdge)**2) + &
latGradEdge(iEdge)*lonGradEdge(iEdge)*(cos(angleEdge(iEdge))**2 - sin(angleEdge(iEdge))**2)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These variables should be indexed, i.e., topographicWaveDrag(iEdge) = , normalCoeff(iEdge) =, tangentialCoeff(iEdge) =

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 not sure what fortran will do in this case, but I feel like testing for LGF should be revisited...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure it'll set the entire array to a single value, so this definitely needs to be retested.

topographicWaveDrag(iEdge) = 0.0_RKIND
if (kmax > 0) then
k = maxLevelEdgeTop(iEdge)
topoDragTurnOff = (TANH((layerThickEdgeMean(k,iEdge)-topoDragCutoffDepth)/topoDragCutoffWidth)+1)/2
Copy link
Contributor Author

@sbrus89 sbrus89 Mar 28, 2024

Choose a reason for hiding this comment

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

I think I'm going to pull out the application of the topoDragTurnOff factor since it's common to all three drag schemes

! topographic_wave_drag = 1/rinv
if (k>0) then
temp_twd(iEdge) = topographicWaveDrag(iEdge) * normalVelocity(k,iEdge)
tend(k,iEdge) = tend(k,iEdge) - topographicWaveDrag(iEdge) * normalVelocity(k,iEdge)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should just set tend(k,iEdge) = tend(k,iEdge) - temp_twd(iEdge)

topoDragTurnOff = (TANH((layerThickEdgeMean(k,iEdge)-topoDragCutoffDepth)/topoDragCutoffWidth)+1)/2
topographicWaveDrag(iEdge) = topographicWaveDragCoeff*topoDragTurnOff*50.0_RKIND * &
(bed_slope_edges(iEdge)**2) * &
(5.24e-3_RKIND*1300.0_RKIND * &
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These should be defined as constant variables

<nml_option name="config_topographic_wave_drag_coeff" type="real" default_value="5.0e-4"
<nml_option name="config_topographic_wave_drag_scheme" type="character" default_value="JSL" units="unitless"
description="hich of the following three wave drag schemes to use: JSL: Jayne and St. Laurent; ZAE: Zaron and Egbert; or LGF: Local generation formula"
possible_values="JSL or ZAE or LGF"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the implementation/tuning for LGF well established? I know that JSL and especially ZAE are working well, but am less confident re: LGF.
Should ZAE be the default if it's typically the best performing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having ZAE as the default sounds good to me. I'm going to do a full tidal run with all three and I'll report back on the errors. I believe that LGF needs to be re-evaluated given the indexing issue I found above.

@jonbob
Copy link
Contributor

jonbob commented May 2, 2024

@sbrus89 -- no problem. I'm just trying to keep PRs moving, so I also updated the bld files here

@jonbob jonbob added the NML label May 2, 2024
@dengwirda
Copy link
Contributor

@jonbob, @sbrus89 and all, another option could be to just stick with the ZAE and JSL schemes --- I think they're both robust and well tested. If you're keen to merge LGF with the fixes to the array indexing as well though, I'd suggest to work through some tuning + benchmarking first.

@sbrus89
Copy link
Contributor Author

sbrus89 commented May 6, 2024

@dengwirda, I confirmed that with the LGF changes described above (e.g. MPAS-Dev/compass@ab0da31), the model achieves a 3.36 cm deep M2 RMSE with a wave drag coefficient of 2.5. That wave drag value is much more reasonable and is solidly in the rage used by ADCRIC (See figure 6 here). While I haven't done a full set of tuning runs to find an optimal value, this result gives me a lot more confidence that LGF is working correctly.
M2_plot

@dengwirda
Copy link
Contributor

dengwirda commented May 7, 2024

Looks good @sbrus89! Those array indexing issues must have been where the issues with LGF had been lurking in the past.
Thanks for chasing all of this down.

@sbrus89
Copy link
Contributor Author

sbrus89 commented May 7, 2024

@jonbob, this one is ready now when convenient.

@sbrus89
Copy link
Contributor Author

sbrus89 commented May 7, 2024

@dengwirda - no problem, I'm glad we got it sorted out. Thank you for the SAL/HA changes in this PR. I appreciate you taking the time to review as well!

jonbob added a commit that referenced this pull request May 16, 2024
Add SAL updates and topographic wave drag schemes for barotropic tides

This PR incorporates updates to the global tidal forcing in MPAS-Ocean.
It combines two contributions developed under the ICoM project:
* @dengwirda's modifications to
  a) include surface pressure in the self attraction and loading (SAL)
     calculations
  b) use a depth rather than distance-based criteria in the SAL coastal
     smoothing factor, and
  c) update the harmonic analysis AM to account for initial SSH offsets.
* @knbarton's refactoring of the existing Jayne and St. Laurent
  topographic wave drag (TWD) parameterization and the addition of the
  Zaron and Egbert and Local Generation Formula TWD schemes.

[NML]
[BFB] - mpas-ocean standalone
@jonbob
Copy link
Contributor

jonbob commented May 16, 2024

Passes:

  • ERS_Ld5.T62_oQU120.CMPASO-NYF.chrysalis_intel
  • ERP_Ld3.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel.allactive-pioroot1

with expected NML DIFFs. Merged to next

@jonbob jonbob merged commit b25c065 into E3SM-Project:master May 20, 2024
13 checks passed
@jonbob
Copy link
Contributor

jonbob commented May 20, 2024

merged to master and expected NML DIFFs blessed

@sbrus89
Copy link
Contributor Author

sbrus89 commented May 20, 2024

Thanks very much, @jonbob!

@xylar
Copy link
Contributor

xylar commented May 29, 2024

A reminder that it is always a good idea to check the compass pr suite as part of testing changes that can affect standalone MPAS-Ocean. I'm seeing segfaults in standalone MPAS-Ocean forward runs starting with this branch.

@sbrus89
Copy link
Contributor Author

sbrus89 commented May 29, 2024

@xylar, I looked into this and wanted to note that the issue is in the namelist settings for the global_ocean cases which currently use topographic wave drag, but don't have the input stream data now required following this PR. So, the fix to the segfaults in the pr suite should be on the compass side and not in E3SM.

@xylar
Copy link
Contributor

xylar commented May 29, 2024

@sbrus89, as we discussed, that's great and quite a relief!

@xylar
Copy link
Contributor

xylar commented May 31, 2024

I'm afraid I think this needs to be fixed or reverted ASAP. I am seeing major changes in MPAS-Ocean init mode for global_ocean test cases. In particular, I am seeing zero layer thickness in the Icos240 test case for all layers >= 5.

Here is the deepest layer before this change:
before
And after:
after

@@ -893,7 +893,8 @@ subroutine ocn_init_setup_global_ocean_create_model_topo(domain, iErr)!{{{
end if
if (isOcean) then
! Enforce minimum depth
bottomDepth(iCell) = max(bottomDepthObserved(iCell), refBottomDepth(minimum_levels))
bottomDepth(iCell) = max(bottomDepthObserved(iCell), config_global_ocean_minimum_depth)
bottomDepth(iCell) = min(bottomDepth(iCell), refBottomDepth(minimum_levels))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should have been a max, not a min.

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 fixing this @xylar! Sorry I overlooked this change and didn't catch it earlier.

xylar added a commit to xylar/E3SM that referenced this pull request May 31, 2024
A bug was introduced in E3SM-Project#6310 that made the ocean shallower
rather than deeper than the minimum allowed depth.  This
merge fixes that bug.
jonbob added a commit that referenced this pull request May 31, 2024
… next (PR #6454)

Fix bottom depth deepening in global_ocean init mode

A bug was introduced in #6310 that made the ocean shallower rather than
deeper than the minimum allowed depth. This merge fixes that bug.

Fixes #6453
[BFB] -- mpas-ocean standalone only
jonbob added a commit that referenced this pull request Jun 3, 2024
…6454)

Fix bottom depth deepening in global_ocean init mode

A bug was introduced in #6310 that made the ocean shallower rather than
deeper than the minimum allowed depth. This merge fixes that bug.

Fixes #6453
[BFB] -- mpas-ocean standalone only
xylar added a commit to xylar/compass that referenced this pull request Jun 4, 2024
This merge updates the E3SM-Project submodule from [31e0924](https://github.com/E3SM-Project/E3SM/tree/31e0924) to [8939709](https://github.com/E3SM-Project/E3SM/tree/8939709).

This update includes the following MPAS-Ocean and MPAS-Frameworks PRs (check mark indicates bit-for-bit with previous PR in the list):
- [ ]  (ocn) E3SM-Project/E3SM#6263
- [ ]  (ocn) E3SM-Project/E3SM#6310
- [ ]  (fwk) E3SM-Project/E3SM#6427
- [ ]  (ocn) E3SM-Project/E3SM#6397
- [ ]  (ocn) E3SM-Project/E3SM#6306
- [ ]  (ocn) E3SM-Project/E3SM#6454
jonbob added a commit that referenced this pull request Jun 26, 2024
…(PR #6471)

Fix OpenACC deletes for topographic wave drag

Partially addresses #6470

The errors were introduced in #6310, when variables were introduced and
renamed, including in the OpenACC create directives but corresponding
changes were incomplete for the OpenACC directives for deleting them.

[BFB]
jonbob added a commit that referenced this pull request Jun 27, 2024
Fix OpenACC deletes for topographic wave drag

Partially addresses #6470

The errors were introduced in #6310, when variables were introduced and
renamed, including in the OpenACC create directives but corresponding
changes were incomplete for the OpenACC directives for deleting them.

[BFB]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB MPAS-Ocean standalone Issues and features for standalone MPAS-Ocean code that dont impact E3SM. mpas-ocean NML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants