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

Jgfouca/scream downstream 04 11 24 #6343

Merged
merged 633 commits into from
Apr 18, 2024
Merged

Conversation

jgfouca
Copy link
Member

@jgfouca jgfouca commented Apr 11, 2024

Bring latest EAMxx and other changes from scream repo.

Non-eamxx changes:

  1. Change GitHub CI to skip E3SM CI for eamxx (for now)
  2. Add an eamxx-specific workflow in the CI to ensure default input files for SCREAM are available and downloadable
  3. ELM config changes: new namelist defaults added for ne256pg2 and ne1024pg2 land configurations to support F20TR compsets with these resolutions
  4. EAM changes: scream/micro_p3.F90 updates/fixes. Should only impact SCREAM stuff. Also, there's a fix for DP SCREAM output coordinate issue where x-coordinates should be assocated with spherep lon values and y-coordinates should be associated with lat values
  5. Change kokkos build to ensure cxx17 standard

[non-BFB] for HOMMExx cases.


On mappy, this PR is non-BFB for these cases, which I think is expected:

DIFF ERS_D.ne4pg2_oQU480.F2010.mappy_gnu.eam-hommexx (phase BASELINE)
DIFF SMS_Ln9_P24x1.ne4_ne4.FDPSCREAM-ARM97.mappy_gnu (phase BASELINE)
DIFF SMS_R_Ld5.ne4_ne4.FSCM-ARM97.mappy_gnu.eam-scm (phase BASELINE)

mahf708 and others added 30 commits February 28, 2024 07:16
…-SCREAMv1

Automatically Merged using E3SM Pull Request AutoTester
PR Title: Add F20TR-SCREAMv1 compset
PR Author: brhillman
PR LABELS: AT: AUTOMERGE
…fig/20240227

Automatically Merged using E3SM Pull Request AutoTester
PR Title: Update machine config for Livermore Computing machines
PR Author: AaronDonahue
PR LABELS: BFB, Machine File, AT: AUTOMERGE, AT: Skip Stand-Alone Testing, AT: Skip v1 Testing
…-units

Automatically Merged using E3SM Pull Request AutoTester
PR Title: set_string for some radiation diagnostics
PR Author: mahf708
PR LABELS: radiation, AT: AUTOMERGE, diagnostic
Use standard script format. Some minor fixes. Make sure it
will work with py3.6.
AaronDonahue and others added 4 commits April 10, 2024 16:18
…rge_2024_04_04

* origin/master: (122 commits)
  aerosol_optics - fixing name of variable: from tau_g_sw to tau_ssa_g_sw.
  add doc for namelist variables for  MAM4 optics input files
  aerosol_optics - remove code.
  aerosol_optics - fixing run time error; we need to use get_field_out("qc") instead of get_field_in("qc").
  aerosol_optics - remove old comment.
  aerosol_optics - remove aero_tau_forward from field manager.
  aerosol_optics - remove old comment.
  aerosol_optics - remove temporal variables from field manager.
  aerosol_optics - fix typo.
  aerosol_optics - rename: aero_tau_forward -> tau_f_lw .
  aerosol_optics - remove aero from aero_tau_sw.
  aerosol_optics - remove aero from aero_tau_ssa_sw.
  aerosol_optics - remove aero from variable name.
  aerosol_optics - removed duplicate variable.
  aerosol_optics - from Updated to Required.
  aerosol_optics - update comments.
  aerosol_optics - update comments.
  aerosol_optics - comment.
  aerosol_optics - update comment.
  aerosol_optics - rename layout.
  ...
…4_04_04

Jgfouca/upstream merge 2024 04 04
…stream_04_11_24

* scream/master: (539 commits)
  aerosol_optics - fixing name of variable: from tau_g_sw to tau_ssa_g_sw.
  add doc for namelist variables for  MAM4 optics input files
  aerosol_optics - remove code.
  aerosol_optics - fixing run time error; we need to use get_field_out("qc") instead of get_field_in("qc").
  aerosol_optics - remove old comment.
  aerosol_optics - remove aero_tau_forward from field manager.
  Add fallow-arg-mismatch to mappy
  aerosol_optics - remove old comment.
  aerosol_optics - remove temporal variables from field manager.
  aerosol_optics - fix typo.
  aerosol_optics - rename: aero_tau_forward -> tau_f_lw .
  aerosol_optics - remove aero from aero_tau_sw.
  aerosol_optics - remove aero from aero_tau_ssa_sw.
  aerosol_optics - remove aero from variable name.
  Switch mappy over to new toolchain
  aerosol_optics - removed duplicate variable.
  aerosol_optics - from Updated to Required.
  aerosol_optics - update comments.
  aerosol_optics - update comments.
  aerosol_optics - comment.
  ...
@jgfouca jgfouca added the EAMxx PRs focused on capabilities for EAMxx label Apr 11, 2024
@jgfouca jgfouca requested a review from rljacob April 11, 2024 15:54
Copy link

PR Preview Action v1.4.7
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6343/
on branch gh-pages at 2024-04-11 15:56 UTC

@@ -0,0 +1,62 @@
name: inputdata
Copy link
Member

Choose a reason for hiding this comment

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

what does this do?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it checks that certain inputs are downloaded.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that's right. It checkes that eamxx default files in the xml are actually present on the server, and if they are not present, it will open an issue. It runs periodically behind the scenes.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can disable it for now if this is an issue (or have it run only on the scream side downstream).

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be harmless for E3SM, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it is harmless. In fact, it should be a good addition (early notification for server/user errors). It won't try to figure out what's wrong with E3SM inputfiles at all, but we can extend that later if desired.

Copy link
Member

@rljacob rljacob left a comment

Choose a reason for hiding this comment

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

PR description should describe all changes outside of components/eamxx.

@@ -4480,6 +4480,11 @@
<map name="ROF2LND_FMAPNAME">lnd/clm2/mappingdata/maps/ne240np4/map_0.1x0.1_nomask_to_ne240np4_nomask_aave_da_c120706.nc</map>
</gridmap>

<gridmap lnd_grid="ne256np4.pg2" rof_grid="r0125">
<map name="LND2ROF_FMAPNAME">cpl/gridmaps/ne256pg2/map_ne256pg2_to_r0125_mono.200212.nc</map>
<map name="ROF2LND_FMAPNAME">cpl/gridmaps/ne256pg2/map_r0125_to_ne256pg2_mono.200212.nc</map>
Copy link
Member

Choose a reason for hiding this comment

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

"mono" is no longer a valid key in a map name.

Copy link
Member Author

Choose a reason for hiding this comment

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

@brhillman , can you chime in here? Looks like you added this on the eamxx side with this commit:

commit b23a4b48fe0fe967a3f3442a4fb52a0ecd9267d5
Author: Benjamin Hillman <[email protected]>
Date:   Fri Feb 2 17:10:48 2024 -0500

    Add grid maps for ne256 with MOSART r0125

diff --git a/cime_config/config_grids.xml b/cime_config/config_grids.xml
index 614eaf0..f877867 100755
--- a/cime_config/config_grids.xml
+++ b/cime_config/config_grids.xml
@@ -4385,6 +4385,11 @@
       <map name="ROF2LND_FMAPNAME">lnd/clm2/mappingdata/maps/ne240np4/map_0.1x0.1_nomask_to_ne240np4_nomask_aave_da_c120706.nc</map>
     </gridmap>
 
+    <gridmap lnd_grid="ne256np4.pg2" rof_grid="r0125">
+      <map name="LND2ROF_FMAPNAME">cpl/gridmaps/ne256pg2/map_ne256pg2_to_r0125_mono.200212.nc</map>
+      <map name="ROF2LND_FMAPNAME">cpl/gridmaps/ne256pg2/map_r0125_to_ne256pg2_mono.200212.nc</map>
+    </gridmap>

Copy link
Contributor

Choose a reason for hiding this comment

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

@rljacob what are we calling the conservative monotonic tempest remap algorithm now?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@brhillman , @rljacob , so should this be changed to:
map_ne256pg2_to_r0125_traave.200212.nc
map_r0125_to_ne256pg2_traave.200212.nc

@jgfouca jgfouca added the non-BFB PR makes roundoff changes to answers. label Apr 11, 2024
@@ -274,10 +274,13 @@ ic_tod="0" sim_year="2000" glc_nec="0" use_crop=".true." irrigate=".true." nu_co
<!-- Initial conditions for DYAMOND1, DYAMOND2 and for seasons of ENSO neutral year 2013 -->
<finidat hgrid="ne1024np4.pg2" mask="ICOS10" ic_ymd="20160801">lnd/clm2/initdata/20220928.I2010CRUELM.ne1024pg2_ICOS10.elm.r.2016-08-01-00000.nc</finidat>
<finidat hgrid="ne1024np4.pg2" mask="ICOS10" ic_ymd="20200120">lnd/clm2/initdata/20221218.F2010-CICE.ne30pg2_ne1024pg2.elm.r.2020-01-20-00000.nc</finidat>
<finidat hgrid="ne1024np4.pg2" mask="ICOS10" ic_ymd="19941001">lnd/clm2/initdata/20231226.I2010CRUELM.ne1024pg2_ICOS10.elm.r.1994-10-01-00000.nc</finidat>
Copy link
Member Author

Choose a reason for hiding this comment

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

@brhillman , looks like you made these changes to ELM too. Can you give a quick explanation? The commit was:

commit f8e9303e7fcc7867883300b52df3c0bd44f7f593
Author: Benjamin Hillman <[email protected]>
Date:   Thu Feb 22 13:51:09 2024 -0500

    Fix finidat for elm ne1024

Copy link
Contributor

Choose a reason for hiding this comment

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

Additional inputfiles for different start dates for decadal runs. Only affect ne1024 and ne256, I don't see a problem here.

@@ -45,6 +45,10 @@

<fsurdat hgrid="r05">lnd/clm2/surfdata_map/surfdata_0.5x0.5_simyr1850_c200609_with_TOP.nc</fsurdat>

<!-- Hack to override files for ne1024 for SCREAM, even though the sim years do not match up -->
Copy link
Member Author

Choose a reason for hiding this comment

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

@brhillman , same here:

commit 3e74f881fa314a16c8c8d97a066f29b06fe511e3
Author: Benjamin Hillman <[email protected]>
Date:   Tue Jan 16 02:30:15 2024 -0600

    Add hacks to elm use cases to use correct ne1024 land data

Copy link
Contributor

Choose a reason for hiding this comment

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

These were added to get the decadal run to configure. Only affects ne1024.

@@ -585,8 +585,8 @@ subroutine stepon_run3(dtime, cam_out, phys_state, dyn_in, dyn_out)
if (dp_crm) then

do ie=1,nelemd
out_gridx(:,:) = dyn_in%elem(ie)%spherep(:,:)%lat
out_gridy(:,:) = dyn_in%elem(ie)%spherep(:,:)%lon
out_gridx(:,:) = dyn_in%elem(ie)%spherep(:,:)%lon
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it fixed a bug.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep:

commit 4802bc8aed0ecb2f32dbeaa5405e9192e8aad4d6
Author: Peter Bogenschutz <[email protected]>
Date:   Tue Jan 9 13:47:55 2024 -0800

    fixes DP SCREAM output coordinate issue where x-coordinates should be assocated with spherep lon values and y-coordinates should be associated with lat values

diff --git a/components/eam/src/dynamics/se/stepon.F90 b/components/eam/src/dynamics/se/stepon.F90
index 831fd6d..38a3769 100644
--- a/components/eam/src/dynamics/se/stepon.F90
+++ b/components/eam/src/dynamics/se/stepon.F90
@@ -585,8 +585,8 @@ subroutine stepon_run3(dtime, cam_out, phys_state, dyn_in, dyn_out)
    if (dp_crm) then
 
      do ie=1,nelemd
-       out_gridx(:,:) = dyn_in%elem(ie)%spherep(:,:)%lat
-       out_gridy(:,:) = dyn_in%elem(ie)%spherep(:,:)%lon
+       out_gridx(:,:) = dyn_in%elem(ie)%spherep(:,:)%lon
+       out_gridy(:,:) = dyn_in%elem(ie)%spherep(:,:)%lat
        call outfld('crm_grid_x', out_gridx, npsq, ie)

I've updated the description.

@rljacob
Copy link
Member

rljacob commented Apr 17, 2024

PR description is better but still needs some words about the changes to eam code.

@jgfouca
Copy link
Member Author

jgfouca commented Apr 17, 2024

@rljacob , done.

@rljacob
Copy link
Member

rljacob commented Apr 17, 2024

Also, can you describe what cases this in non-BFB for?

@rljacob
Copy link
Member

rljacob commented Apr 17, 2024

Oh I see you did. Nevermind.

jgfouca added a commit that referenced this pull request Apr 17, 2024
Bring latest EAMxx and other changes from scream repo.

Non-eamxx changes:
1) Change GitHub CI to skip E3SM CI for eamxx (for now)
2) Add an eamxx-specific workflow in the CI to ensure default input files for SCREAM are available and downloadable
3) ELM config changes: new namelist defaults added for ne256pg2 and ne1024pg2 land configurations to support F20TR compsets with these resolutions
4) EAM changes: scream/micro_p3.F90 updates/fixes. Should only impact SCREAM stuff. Also, there's a fix for DP SCREAM output coordinate issue where x-coordinates should be assocated with spherep lon values and y-coordinates should be associated with lat values
5) Change kokkos build to ensure cxx17 standard

On mappy, this PR is non-BFB for these cases, which I think is expected:

DIFF ERS_D.ne4pg2_oQU480.F2010.mappy_gnu.eam-hommexx (phase BASELINE)
DIFF SMS_Ln9_P24x1.ne4_ne4.FDPSCREAM-ARM97.mappy_gnu (phase BASELINE)
DIFF SMS_R_Ld5.ne4_ne4.FSCM-ARM97.mappy_gnu.eam-scm (phase BASELINE)

[non-BFB] for HOMMExx cases.
@jgfouca
Copy link
Member Author

jgfouca commented Apr 17, 2024

Merged to next.

@jgfouca jgfouca merged commit baef174 into master Apr 18, 2024
14 checks passed
@jgfouca jgfouca deleted the jgfouca/scream_downstream_04_11_24 branch April 18, 2024 15:03
@rljacob
Copy link
Member

rljacob commented Apr 25, 2024

@jgfouca this caused some diffs in integration tests that use SCREAM. Please make bless requests.

@jgfouca
Copy link
Member Author

jgfouca commented Apr 25, 2024

@rljacob , done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EAMxx PRs focused on capabilities for EAMxx non-BFB PR makes roundoff changes to answers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.