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

Switch global ocean in init mode to E3SM shared constants #96

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented May 3, 2024

This merge also migrates the computation of the Coriolis parameter to using E3SM shared constants.

The unused constants module imports are removed from several init mode utilities to ensure that they don't use constants that are inconsistent with E3SM's versions.

I have made local symlinks to the shared constants and kinds file for convenience in building using MPAS-Ocean's not-very-sophisticated build system.

@xylar
Copy link
Collaborator Author

xylar commented May 3, 2024

@proteanplanet, this my proposed fix to E3SM-Project#6387. Please have a look, though I am fully aware that MPAS-Ocean init mode is nothing you have detailed knowledge about (nor should you!).

@mark-petersen, I know I'm asking a lot of you at a busy time but please give this a look when you can.

@xylar
Copy link
Collaborator Author

xylar commented May 3, 2024

Testing

I ran the pr suite on Chrysalis with Intel and OpenMPI using this branch, and using MPAS-Dev/compass#810 as a baseline (because this addresses recent non-BFB issue on Chrysalis).

Tests other than global ocean are BFB as expected. Global ocean forward runs show differences in T, S, layer thickness and velocity on the order of 1e-8 (so definitely not negligible).

@proteanplanet
Copy link
Collaborator

We should include a sea ice solution in this. I will take care of that this week.

@xylar
Copy link
Collaborator Author

xylar commented May 6, 2024

@proteanplanet, since these changes are specific to mpas-ocean standalone, we shouldn't mix them with E3SM changes to any component. We should take care of that in a separate PR.

dqwu and others added 22 commits May 6, 2024 11:08
The subroutine ncd_defvar_bygrid internally uses a local varid to
invoke ncd_defvar_bynf. Introduce a new argument to optionally
return the varid to the caller.
This change optimizes the restartvar code in ELM with the following
improvements:

* Utilization of the ncid returned by subroutine ncd_defvar_bygrid,
  eliminating the need for calling PIO_inq_varid.
* Introduction of additional optional arguments in the call to
  ncd_defvar_bygrid to internally set corresponding attributes.
* Elimination of some redundant PIO_put_att calls that are already
  handled within ncd_defvar_bygrid.
Update modules for intel/gnu,impi/mvapich on Bebop to get system
software more update-to-date.
This prevents the last entry from the previous run before a
restart from being clobbered by a new run.
The new ones use a 64-bit build of gpmetis that does not leave
partitions with zero cells.
Setting fates_hist_dimlevel with a single line (i.e. = 2,2) fails during
ELMBuildNamelist.  Adjust the assignment to accomodate this.

This also removes hist_ndens to avoid issue E3SM-Project#1106
ndkeen and others added 9 commits June 13, 2024 09:03
Change MPAS-Ocean high-frequency output mode to append

Without this, the first output is missing after each restart

Fixes E3SM-Project#6432

[BFB]
The `dt` is now included as an input to the subroutine `RootDynamics()`.

Fixes E3SM-Project#5913
[BFB]
intel from 18.0.4 to 20.0.4
intel-mkl from 2018.4.274 to 2020.4.304
intel-mpi from 2018.4.274 to 2019.9.304

hdf5, netcdf, pnetcdf versions are updated (some are downgraded) to
match the new compilers/mpilibs.

Most of the new modules are also used on Anvil.

Also define HDF5_ROOT for both intel and gnu compilers.

[BFB]
Add new RRSwISC6to18E3r5 ocean and sea-ice mesh

Long name: RRSwISC6to18L80E3SMv3r5
This RRS (Rossby-radius scaled) mesh has:
* 6 km resolution near the poles
* 18 km resolution at the equator
This is a proposed E3SM v3 (E3) high resolution (near-eddy-resolving)
mesh. This is revision 5 (r5) of the mesh, which includes a flood fill
of the land-ice mask to ensure correct connectivity
(MPAS-Dev/compass#800). The minimum water-column thickness has been set
to 20 m.

[BFB] for all currently tested configurations
…M-Project#6420)

Modify sss restoring to include under sea ice by default

Work done under the ImPACTS SciDac led to the conclusion that it is
generally preferable to include SSS restoring under sea ice by default
if doing any SSS restoring, and to compute the restoring terms at each
time step.

This is due to the fact that the intended linear taper in the sea ice
region (by sea ice fraction) may change sign entirely due to the removal
of the global mean. This leads to a small negative flux in the
high-latitudes, which reinforces the fresh bias.

Work by Luke, Mat, Fred, Alice and others.

[NML]
[non-BFB] for C- and G-cases using grids with restoring files
This PR effectively reduces some redundant PIO calls for interface
restartvar in ELM.

Fixes E3SM-Project#6384

[BFB]
@xylar
Copy link
Collaborator Author

xylar commented Jun 15, 2024

@mark-petersen and @proteanplanet, this would be nice to have before we create a SORRM mesh. Can you let me know if it's ready to go to E3SM?

@proteanplanet
Copy link
Collaborator

@xylar I will test this in a G-case 1-month run and report back here. Apologies for the delay. Too many things going on.

@xylar
Copy link
Collaborator Author

xylar commented Jun 17, 2024

@proteanplanet, this only involves init mode in standalone so no need to run any simulations, though you could as a do-no-harm test. Just a quick code inspection will suffice before this goes to E3SM.

I will point you to the initial condition this code produces (I just need to find the path).

@xylar xylar force-pushed the ocn/fix-coriolis-shared-constants branch from ee00a88 to 7867f36 Compare June 17, 2024 04:59
@xylar
Copy link
Collaborator Author

xylar commented Jun 17, 2024

I verified that I get slightly different values for fCell with this code than before this change.

Before, see:

/lcrc/group/e3sm/ac.xylar/compass_1.4/chrysalis/test_20240611/sowiscr3_expand_rx1_mask/ocean/global_ocean/SOwISC12to30/WOA23/init/initial_state/initial_state.nc

This change:

/lcrc/group/e3sm/ac.xylar/compass_1.4/chrysalis/test_20240617/sowisc12to30e3r3_fix_coriolis/ocean/global_ocean/SOwISC12to30/WOA23/init/initial_state/initial_state.nc

Before:

 fCell = -0.000140769901625857, -0.000140704437612956, -0.00014083233024888, 
    -0.000140767163629124, -0.00014070142243594, -0.000140635078080735, 
    -0.000141022649665392, -0.000140958688777142, -0.000140894242018246, 
    -0.000140829320395959, -0.000140763846645755, -0.00014069784697824, 
    -0.000141086005207922, -0.000141083668731909, -0.000141019781326067, 
    -0.000140955517779708, -0.000140890854883258, -0.000140825741406588, 
...

This change:

 fCell = -0.000140769969519198, -0.000140704505474723, -0.000140832398172331, 
    -0.000140767231521144, -0.000140701490296253, -0.000140635145909051, 
    -0.000141022717680633, -0.000140958756761535, -0.000140894309971556, 
    -0.000140829388317958, -0.000140763914536176, -0.000140697914836829, 
    -0.00014108607325372, -0.000141083736776579, -0.000141019849339925, 
    -0.000140955585762572, -0.000140890922834935, -0.00014082580932686, 
...

The corresponding latCell (identical for both) is:

 latCell = -1.30628095579851, -1.30456946509689, -1.30792322431631, 
    -1.30620915702955, -1.30449089505505, -1.30276777196906, 
    -1.31299288231411, -1.31127831767717, -1.30956185509607, 
    -1.30784381625203, -1.30612220091371, -1.3043977544342, 
    -1.31470228273184, -1.31463904399789, -1.31291575362999, 
    -1.31119360249962, -1.30947194755724, -1.3077494233053,
...

I verified that the first entry seems right given:

   real(R8),parameter :: SHR_CONST_PI      = 3.14159265358979323846_R8  ! pi
   real(R8),parameter :: SHR_CONST_SDAY    = 86164.0_R8      ! sec in siderial day ~ sec
   real(R8),parameter :: SHR_CONST_OMEGA   = 2.0_R8*SHR_CONST_PI/SHR_CONST_SDAY ! earth rot ~ rad/sec
   ...
   fCell(iCell) = 2.0_RKIND * SHR_CONST_OMEGA * sin(latCell(iCell))

jgfouca and others added 4 commits June 17, 2024 11:03
... to f903115718ebc30669ce557f511abaef231a1d88

Fixes:
1) Several fixes/changes to containerized CI. No impact to E3SM
2) fix an issue with single component runs which do use mediator
3) Fix ERI test type: need to make sure ref2 run dir exists
4) Fixes config ignoring paths containing "tests"
5) More robust approach to waiting for many threads, fixes a bug where
   archiving threads block test reporting.
6) The xmlchange tool should not raise an exception when a diff is detected.

Changes:
1) New feature: long grid names
2) Update documentation
3) Refactors how/when rebuilds are required. Reduce scenarios in which a full rebuild is required
4) Remove reference to cice5 and ww3dev, they are no longer used

[BFB]
The FATE API has been updated which includes
1) control over the dimensionality of FATES history output and
2) an update to the parameter file format. This set of changes makes
   E3SM compatible with FATES tag sci.1.76.3_api.35.1.0.

[BFB] except for FATES
Update mapping and domain files created with an incorrect T62 scrip file

Several configurations have mapping and domain files made from a T62
scrip file that did not extend to the North pole, which is incorrect and
causes some problems in the moab driver. This PR updates those files
using a correct T62 scrip file for currently used resolutions.

[non-BFB] for tests using T62
…3SM-Project#6460)

Update nvidiagpu_pm-gpu.cmake to use Nvidia cc80 compute capability on pm-gpu
Copy link

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

This all makes sense. Thanks for your effort on a long-standing issue. The code changes look good.

I compiled this with gnu (debug and optimized) on chicoma and intel on chrysalis and tested with the nightly test suite. I compared to master and QU240 init differs by 1e-12. The subsequent tests differ by 1e-10, as expected.

The dt is now included as an input to the subroutine RootDynamics().

Fixes E3SM-Project#5913
[BFB]
Copy link
Collaborator

@proteanplanet proteanplanet left a comment

Choose a reason for hiding this comment

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

Approved based on visual inspection.

jgfouca and others added 4 commits June 20, 2024 12:27
…SM-Project#6476)

Update CIME submodule

... to f903115718ebc30669ce557f511abaef231a1d88

Fixes relevant to E3SM
1) Fix ERI test type: need to make sure ref2 run dir exists
2) Fixes config ignoring paths containing "tests"
3) More robust approach to waiting for many threads, fixes a bug where
   archiving threads block test reporting.
4) The xmlchange tool should not raise an exception when a diff is detected.

Changes relevant to E3SM.
1) New feature: long grid names
2) Update documentation (done for E3SM tutorial)
3) Refactors how/when rebuilds are required. Reduce scenarios in which a full rebuild is required

Other fixes/changes:
1) Several fixes/changes to containerized CI.
2) Remove reference to cice5 and ww3dev, they are no longer used
3) fix an issue with single component runs which do use mediator

[BFB]
Extend moab driver for a case with ROF data

Case tested: --compset GMPAS-IAF --res T62_oQU240wLI

iMOAB_MergeVertices is not used anymore in moab land driver, small correction incorporated into this PR
This merge also migrates the computation of the coriolis parameter
to using E3SM shared constants.

The unused constants module imports are removed from several
init mode utilities to ensure that they don't use constants
that are inconsistent with E3SM's versions.
@xylar
Copy link
Collaborator Author

xylar commented Jun 21, 2024

Thanks @proteanplanet and @mark-petersen!

Closing in favor of E3SM-Project#6481

@xylar xylar closed this Jun 21, 2024
@xylar xylar deleted the ocn/fix-coriolis-shared-constants branch July 1, 2024 17:10
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.