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

Update maint1.2: 3D varying GM #487

Merged

Conversation

mark-petersen
Copy link
Contributor

@mark-petersen mark-petersen commented Mar 24, 2020

This update adds the following PRs from ocean/develop:

vanroekel and others added 12 commits March 24, 2020 07:42
In the GM bolus calculation, there is a specified diffusivity and phase
speed (see Ferrari et al. 2010).  Currently MPAS-O assumes these values
are fixed in time and space.  This PR allows separate flags to enable
2D varying (+time) phase speed (based on the first baroclinic mode phase
speed) and a separate flag for 3D (+time) varying diffusivity.
Removes the vertical averaging of the GM kappa bolus diffusivity.
Testing shows this functionality is not beneficial to simulations
…te_maint1.2

Adds options for 3d varying GM bolus and 2d varying phase speed MPAS-Dev#288

In the GM bolus calculation, there is a specified diffusivity and phase
speed (see Ferrari et al. 2010). Currently MPAS-O assumes these values
are fixed in time and space. This PR allows separate flags to enable
2D varying (+time) phase speed (based on the first baroclinic mode phase
speed) and a separate flag for 3D (+time) varying diffusivity.

* pr_test2:dd
  Removes 2D GM code and debug statements
  Adds 2d variable GM and fixes phase speed bug
  Adds options for 3d varying GM bolus and 2d varying phase speed
…_update_maint1.2

Fix threading issue in MPAS-O GM routine MPAS-Dev#376
This PR fixes a threading issue that was introduced in E3SM PR #3057
that brought in the initial implementation of 3D varying GM bolus. After
that commit, some E3SM threading tests were observed to fail with
different results.

Tested with:

SMS_P12x2.T62_oQU240.CMPASO-NYF.compy_intel - BFB results 10 times
SMS_P12x2.ne4_oQU240.A_WCYCL1850.compy_intel.allactive-mach_mods
- BFB results 10 times

This last test is one that was failing on sandiatoss3.
…ate_maint1.2

Add GM bolus eddy stats MPAS-Dev#339
Added monthly mean zonal and meridional bolus velocities as well as
bolus velocity times temperature to eddy stats for Labrador Sea bias/ GM
testing
@mark-petersen mark-petersen requested a review from vanroekel March 24, 2020 15:13
@mark-petersen mark-petersen self-assigned this Mar 24, 2020
@mark-petersen mark-petersen requested a review from jonbob March 24, 2020 15:13
@mark-petersen
Copy link
Contributor Author

I used cherry-pick on these commits, and there were no conflicts. Compiles with gnu and intel.

Are there any other PRs needed from ocean/develop?

@@ -375,6 +375,22 @@
description="If true, the standard GM for the tracer advection and mixing is turned on. This includes the redi portion which is captured in hmix."
possible_values=".true. or .false."
/>
<nml_option name="config_gm_lat_variable_c2" type="logical" default_value=".false." units="NA"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonbob note there are 6 new GM flags added to maint-1.2 with this update.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mark-petersen - I'm only seeing 4? Aren't the others just new arrays?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, just 4. My mistake.

@darincomeau
Copy link

@mark-petersen I think we need #392 to prevent build errors when using python 3.

@mark-petersen
Copy link
Contributor Author

@darincomeau thanks, I just added that.

@xylar can you think of anything else we might need for maint-1.2?

@xylar
Copy link
Collaborator

xylar commented Mar 24, 2020

@mark-petersen, yes, I believe we need: #447

This disables computing the land-ice mask in forward mode, which I think is necessary with the new initial condition we're about to set up.

@xylar
Copy link
Collaborator

xylar commented Mar 24, 2020

Actually, given the complexity of COMPASS changes in #447, you might want to just cherry-pick xylar@da1f7c9.

xylar and others added 2 commits March 24, 2020 11:01
With this merge, landIceMask is no longer computed based on
landIceFraction.  Instead, it is always read in from an input file.
This allows landIceMask for global_ocean test cases to be computed
from geometric_features while landIceFraction is computed as before
during initialization.
…atures' into ocean/gm_update_maint1.2

Only used the single commit needed to remove landIceMask computation
from diagnostics:
  Read in landIceMask from input
@jonbob
Copy link
Contributor

jonbob commented Mar 24, 2020

@mark-petersen , @darincomeau - what about changing ocean output to single precision in this? It would help with run storage...

@mark-petersen
Copy link
Contributor Author

mark-petersen commented Mar 24, 2020

Actually, given the complexity of COMPASS changes in #447, you might want to just cherry-pick xylar/MPAS-Model@da1f7c9.

Done, thanks. I'll merge this in and set up an E3SM PR.

@mark-petersen
Copy link
Contributor Author

@mark-petersen , @darincomeau - what about changing ocean output to single precision in this? It would help with run storage...

@jonbob good idea, I was thinking the same thing. I can add that on the E3SM PR in a minute.

@jonbob
Copy link
Contributor

jonbob commented Mar 24, 2020

@mark-petersen - I was just looking at PR's from the last year or so. Do any of these make sense to include:
2948
3056
I can't remember what updates to master we actually also merged into maint-1.2, but it doesn't seem like many...

@mark-petersen
Copy link
Contributor Author

@xylar or @matthewhoffman, it looks like we should include
E3SM-Project/E3SM#2948
on the E3SM update for the cryo branch (maint-1.2). Do you agree?

maltrud and others added 3 commits March 24, 2020 11:53
…ocean/gm_update_maint1.2

bug fixes for the nonlocal source term in KPP MPAS-Dev#305

This PR fixes several bugs in the calculation of the nonlocal source
term in KPP. for temperature, we need to subtract off the contribution
from thickness fluxes that don't change buoyancy. for salinity, we need
to add virtual salinity fluxes that do change buoyancy.

these fixes have been tested in E3SM G-cases and behave as expected.
@xylar
Copy link
Collaborator

xylar commented Mar 24, 2020

@matthewhoffman will have a better sense than me. It seems like including this would do no harm. It only matters if we set config_land_ice_flux_mode = 'standalone'. Is that how we run typically in E3SM? I guess maybe so. (I should know this!)

@darincomeau
Copy link

@matthewhoffman will have a better sense than me. It seems like including this would do no harm. It only matters if we set config_land_ice_flux_mode = 'standalone'. Is that how we run typically in E3SM? I guess maybe so. (I should know this!)

Yes, this is the namelist option used in ISMF compsets.

@mark-petersen
Copy link
Contributor Author

@mark-petersen - I was just looking at PR's from the last year or so. Do any of these make sense to include:
E3SM-Project/E3SM#3056

@jonbob thanks for the suggestion. I added the KPP nonlocal fix, which is that 3056 (MPAS PR #305)

@mark-petersen mark-petersen merged commit 49ce08c into MPAS-Dev:e3sm/maint-1.2 Mar 24, 2020
@jonbob
Copy link
Contributor

jonbob commented Mar 24, 2020

@mark-petersen - just let me know when you think you're done and we can start the corresponding E3SM PR

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

Successfully merging this pull request may close these issues.

7 participants