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 ELM in the definition of multiple V3 compsets #6108

Merged
merged 8 commits into from
Jan 25, 2024

Conversation

bishtgautam
Copy link
Contributor

@bishtgautam bishtgautam commented Dec 7, 2023

The following compsets are updated to change ELM's mode from SP to active BGC mode with TOP solar parameterization:

  1. F1850
  2. F20TR
  3. F2010
  4. WCYCL1850_chemUCI-Linozv3
  5. WCYCL1850_chemUCI-Linozv3-mam5
  6. WCYCL20TR_chemUCI-Linozv3-mam5
  7. WCYCL1850-4xCO2
  8. WCYCL1850
  9. WCYCL1850-1pctCO2
  10. WCYCL20TR
  11. WCYCL1850NS

[CC]
[non-BFB] but v3 sims already using these by runscript.
[NML]

Defines a new long compset for ELM that includes BGC and TOP solar parameterization.
The following compsets is updated to change ELM's SP mode to
active BGC with TOP solar parameterization:
- F1850
- F20TR
- F2010
- WCYCL1850_chemUCI-Linozv3
- WCYCL1850_chemUCI-Linozv3-mam5
- WCYCL20TR_chemUCI-Linozv3-mam5
- WCYCL1850-4xCO2
- WCYCL1850
- WCYCL1850-1pctCO2
- WCYCL20TR
Copy link

github-actions bot commented Dec 7, 2023

PR Preview Action v1.4.6
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6108/
on branch gh-pages at 2024-01-24 19:40 UTC

@bishtgautam bishtgautam changed the title [WIP] Updates the definition of multiple V3 compsets Updates the definition of multiple V3 compsets Dec 7, 2023
@bishtgautam
Copy link
Contributor Author

@rljacob @wlin7 This PR will cause DIFFs for multiple tests.

@rljacob rljacob changed the title Updates the definition of multiple V3 compsets Update ELM in the definition of multiple V3 compsets Dec 7, 2023
@rljacob rljacob added Land compset BGC non-BFB PR makes roundoff changes to answers. labels Dec 7, 2023
@rljacob
Copy link
Member

rljacob commented Dec 7, 2023

Aren't these climate changing (if you were running master with SPBC ) ?

@rljacob
Copy link
Member

rljacob commented Dec 13, 2023

Make sure user e3sm_integration runs with these changes BEFORE merging to next.

@rljacob rljacob self-assigned this Dec 14, 2023
@rljacob rljacob added the CC PR is climate changing label Dec 18, 2023
@wlin7
Copy link
Contributor

wlin7 commented Jan 4, 2024

@bishtgautam , the config changes introduced in this PR would become incompatible with master for some compsets (e.g., F2010). This would cause many tests to fail during setup stage, if merging the PR to next, or rebasing it with master.

A test like REP_Ln5.ne4_oQU240.F2010.pm-cpu_intel would complain
ERROR : CLM build-namelist::ELMBuildNamelist::add_default() : No default value found for stream_year_first_fan

Look like certain recent land PRs in master have not accounted for F2010 with the intended ELM config opts.

Note that the problem does not exist with this PR branch itself, which was based off an older master.

@rljacob
Copy link
Member

rljacob commented Jan 4, 2024

@bbye see above. If FAN is not on, then the buildnamelist should not even try to set stream_year_first_fan should it?

@wlin7
Copy link
Contributor

wlin7 commented Jan 4, 2024

@bbye see above. If FAN is not on, then the buildnamelist should not even try to set stream_year_first_fan should it?

In ELMBuildNamelist.pm, add_default indeed is inside if( $opts->{'fan'} ). So it is interesting why it still tried to add. In another standalone F2010-NARRM test, adding use_fan=.false. in user_nl_elm didn't help.

@bbye
Copy link
Contributor

bbye commented Jan 5, 2024

@bbye see above. If FAN is not on, then the buildnamelist should not even try to set stream_year_first_fan should it?

In ELMBuildNamelist.pm, add_default indeed is inside if( $opts->{'fan'} ). So it is interesting why it still tried to add. In another standalone F2010-NARRM test, adding use_fan=.false. in user_nl_elm didn't help.

It shouldn't set the fan stream if fan isn't used, and this worked with the original PR, but I think the problem is in the namelist_default.xml file where the fan_stream is set in lines 730-766. I think changing the use_cn condition to a use_fan condition will resolve this issue. I have a commit to test this in my branch, but I don't know how to test it on the failing cases.

@wlin7
Copy link
Contributor

wlin7 commented Jan 6, 2024

It shouldn't set the fan stream if fan isn't used, and this worked with the original PR, but I think the problem is in the namelist_default.xml file where the fan_stream is set in lines 730-766. I think changing the use_cn condition to a use_fan condition will resolve this issue. I have a commit to test this in my branch, but I don't know how to test it on the failing cases.

@bbye , did you mean the test mentioned above? It can be created using the command as follows if doing it on pm-cpu.

$E3SMROOT/cime/scripts/create_test REP_Ln5.ne4_oQU240.F2010.pm-cpu_intel --machine=pm-cpu

@bishtgautam
Copy link
Contributor Author

bishtgautam commented Jan 6, 2024

Using @bbye's suggestion, I have opened #6142 which fixes the issue. After merging this branch and #6142 to next, the ELM setup proceeds without an error.

@rljacob
Copy link
Member

rljacob commented Jan 11, 2024

This should be merged along with #6142

rljacob added a commit that referenced this pull request Jan 11, 2024
The following compsets are updated to change ELM mode from SP to active BGC mode with TOP solar parameterization:

F1850
F20TR
F2010
WCYCL1850_chemUCI-Linozv3
WCYCL1850_chemUCI-Linozv3-mam5
WCYCL20TR_chemUCI-Linozv3-mam5
WCYCL1850-4xCO2
WCYCL1850
WCYCL1850-1pctCO2
WCYCL20TR

[CC]
[non-BFB] but v3 critical path is already using these.
[NML]
@wlin7
Copy link
Contributor

wlin7 commented Jan 12, 2024

Verified master + #6108 and #6113 can BFB reproduce the current v3.LR.piControl-spinup after removing all non-IO related configuration settings in run script.

Notes:

  1. use of nlmap for atm to ice, ocn, lnd and rof (fmap, e.g., atm2ice_fmapname_nonlinear ) still need to be specified in run script. Otherwise the field is "idmap_ignore'
  2. fsurdat still needs to be specified, otherwise it would pick up surfdata_0.5x0.5_simyr1850_c211019.nc, which does not include '_TOP' and the run would fail. This is going to be fixed in a new PR.

@rljacob
Copy link
Member

rljacob commented Jan 12, 2024

These compsets aren't changed. Should they be?
WCYCLSSP370
WCYCLSSP585
WCYCL1850NS

@rljacob
Copy link
Member

rljacob commented Jan 12, 2024

There are runtime errors from ELM in the prod suite fully coupled cases. Probably because they are all using wrong finidat files?

finidat = '$DIN_LOC_ROOT/lnd/clm2/initdata_map/clmi.WCYCL1850-1pctCO2.ne30pg2_EC30to60E2r2.SMS_Ld1.c20230213.nc
 finidat = '$DIN_LOC_ROOT/lnd/clm2/initdata_map/clmi.WCYCL1850-4xCO2.ne30pg2_EC30to60E2r2.SMS_Ld1.c20230213.nc
 finidat = '$DIN_LOC_ROOT/lnd/clm2/initdata_map/clmi.WCYCL1850.ne30pg2_EC30to60E2r2.SMS_Ld1.c20230213.nc'
 finidat = '${DIN_LOC_ROOT}/lnd/clm2/initdata_map/clmi.WCYCL1850.ne30pg2_r05_EC30to60E2r2.SMS_Ld1.c20230213.nc'
 finidat = '${DIN_LOC_ROOT}/lnd/clm2/initdata_map/clmi.WCYCL1850.northamericax4v1pg2_WC14to60E2r3.SMS_PS.c20230213.nc'

ENDRUN:
ERROR:: litr1c is required on an initialization datasetERROR in ColumnDataType.
F90 at line 2570

@rljacob
Copy link
Member

rljacob commented Jan 12, 2024

There are also runtime errors in the prod suite F-cases. finidat is commented out in those.
ne30pg2_r05_oECv3.F20TR.mach_comp.eam-wcprod_F20T
ne30pg2_r05_oECv3.F2010.mach_comp.eam-wcprod_F2010

 20:  surfrd_veg_all ERROR: sum of wt_nat_patch not 1.0 at nl=        4550  and t=
 20:            1
 20:  sum is:   5.088610182091506E+018
 20:  ENDRUN:
 20:  ERROR in surfrdUtilsMod.F90 at line 75

@wlin7
Copy link
Contributor

wlin7 commented Jan 12, 2024

@rljacob , It appears so - that finidat files are incompatible with the land in BGC mode. I am going to test a fix, either using null finidat, or pulling one from v3.LR. piControl-spinup.

Do you plan to proceed with merge this and #6142 to master today?

@rljacob
Copy link
Member

rljacob commented Jan 12, 2024

No I want to test the fixes. And if we can't fix the F-case fails, I'll revert this PR but will merge #6142

@rljacob
Copy link
Member

rljacob commented Jan 12, 2024

Since we're headed in to a 3-day weekend, I'm going to force-push this off next and we can try again Tuesday.

@rljacob
Copy link
Member

rljacob commented Jan 13, 2024

This was reverted from next.

@wlin7
Copy link
Contributor

wlin7 commented Jan 18, 2024

The error like surfrd_veg_all ERROR: sum of wt_nat_patch not 1.0 at nl= 4550 and t= ... as seen in F-cases would also appear in coupled cases after resetting the incompatible finitdat (emptying it). There must be a systematic issue. Working on this, with an option to switch to v3 LR production grid for the wcprod tests.

@wlin7
Copy link
Contributor

wlin7 commented Jan 19, 2024

The new commits 0549495, 8fe3157, ede5796 fix the issues for the wcprod test suite and other F-cases.

The wcprod test suite changes to use new v3.LR standard grid, ne30pg2_r05_IcoswISC30E3r5. wcprod test in atm_integration suite still uses bigrid ne30pg2_EC30to60E2r2. Ok for now to use the same testmod when not specifying an finidat. We can decide if to continue testing both bigrid and tri-grid for wcprod.

surfrd_veg_all ERROR: sum of wt_nat_patch not 1.0 at nl= 4550 and t= ... is fixed after resetting create_crop_landunit = .false. for v3 wcycl config.

@bishtgautam , the default setting for r05 simyr1850 fsurdat is set using the idea you proposed by adding additional attribute use_top_solar_rad = .true., as follows in namelist_defaults.xml, and similarly for create_crop_landunit to distinguish from the other entry that also has use_cn=.true.. It has to take the order as is otherwise it wouldn't work for v3 wcycl land config. We could alternatively specify them in the use_case files, and need to do so for each simulation period.

<fsurdat hgrid="r05"          sim_year="1850" use_crop=".false." use_cn=".false.">
lnd/clm2/surfdata_map/surfdata_0.5x0.5_simyr1850_c200609_with_TOP.nc</fsurdat>
<fsurdat hgrid="r05"          sim_year="1850" use_crop=".false." use_cn=".true." use_top_solar_rad=".true.">
lnd/clm2/surfdata_map/surfdata_0.5x0.5_simyr1850_c200609_with_TOP.nc</fsurdat>
<fsurdat hgrid="r05"          sim_year="1850" use_crop=".false." use_cn=".true.">
lnd/clm2/surfdata_map/surfdata_0.5x0.5_simyr1850_c211019.nc</fsurdat>


<create_crop_landunit use_crop=".false."                 >.false.</create_crop_landunit>
<create_crop_landunit use_crop=".false." hgrid="r05" use_cn=".true." use_top_solar_rad=".true.">.false.</create_crop_landunit>
<create_crop_landunit use_crop=".false." hgrid="r05" use_cn=".true.">.true.</create_crop_landunit>

@bishtgautam
Copy link
Contributor Author

I compared the lnd_in files for all the tests in e3sm_land_developer test suite that have an active BGC and there are no differences in the baseline w.r.t. this branch. Thank you, Wuyin for those three commits to fix the test suite.

@rljacob I believe we are ready to re-merge this PR.

@bishtgautam
Copy link
Contributor Author

Forgot one thing: the WCYCL1850-WW3 compset isn't updated, yet.

@rljacob
Copy link
Member

rljacob commented Jan 19, 2024

Yes @sbrus89 agreed we should update that one. We can merge this early next week.

@bishtgautam
Copy link
Contributor Author

@rljacob Definition of WCYCL1850-WW3 has been updated.

rljacob added a commit that referenced this pull request Jan 23, 2024
The following compsets are updated to change ELM's mode from SP to active BGC mode with TOP solar parameterization:

F1850
F20TR
F2010
WCYCL1850_chemUCI-Linozv3
WCYCL1850_chemUCI-Linozv3-mam5
WCYCL20TR_chemUCI-Linozv3-mam5
WCYCL1850-4xCO2
WCYCL1850
WCYCL1850-1pctCO2
WCYCL20TR
WCYCL1850NS

[CC]
[non-BFB] but v3 sims already using these by runscript.
[NML]
@rljacob
Copy link
Member

rljacob commented Jan 23, 2024

merged to next

@rljacob
Copy link
Member

rljacob commented Jan 24, 2024

There are 4 runtime fails on integration. All are BGC cases and all have the same error:
ERROR: New format surface datasets require create_crop_landunit TRUE
ERROR in surfrdMod.F90 at line 1215

The cases and compset longname.

  • SMS_Ln5.ne30pg2_r05_EC30to60E2r2.BGCEXP_LNDATM_CNPRDCTC_20TR
    • 20TR_EAM%CMIP6_ELM%CNPRDCTCBC_MPASSI%PRES_DOCN%DOM_MOSART_SGLC_SWAV_BGC%LNDATM
  • SMS_Ln5.ne30pg2_r05_EC30to60E2r2.BGCEXP_LNDATM_CNPRDCTC_1850
    • 1850_EAM%CMIP6_ELM%CNPRDCTCBC_MPASSI%PRES_DOCN%DOM_MOSART_SGLC_SWAV_BGC%LNDATM
  • SMS_Ld2.ne30pg2_r05_EC30to60E2r2.BGCEXP_CNTL_CNPECACNT_1850.xx_xx.elm-bgcexp
    • 1850_EAM%CMIP6_ELM%CNPECACNTBC_MPASSI%BGC_MPASO%OIECO_MOSART_SGLC_SWAV_BGC%BCRC
  • SMS_Ld2.ne30pg2_r05_EC30to60E2r2.BGCEXP_CNTL_CNPRDCTC_1850.xx_xx.elm-bgcexp
    • 1850_EAM%CMIP6_ELM%CNPRDCTCBC_MPASSI%BGC_MPASO%OIECO_MOSART_SGLC_SWAV_BGC%BCRC

The value of `use_top_solar_rad` is also included in the logic to create crop landunits.
rljacob added a commit that referenced this pull request Jan 25, 2024
Second merge of this branch.  Fix 4 failing tests.
@rljacob rljacob merged commit fd65dbe into master Jan 25, 2024
3 checks passed
@rljacob rljacob deleted the bishtgautam/lnd/v3-compset branch January 25, 2024 22:38
jonbob added a commit that referenced this pull request Sep 24, 2024
…(PR #6625)

Updating CRYO compsets to v3 ELM settings

Updates the ELM settings in the following compsets to the v3 settings in
PR #6108:
* CRYO1850
* CRYO1850-DISMF
* CRYO1850-4xCO2
* CRYO1950
* CRYO1950-DISMF
* CRYO20TR
* CRYOSSP370
* CRYOSSP585
Adds following compsets matching the WCYCL counterparts:
* CRYO1850-1pctCO2
* CRYOSSP245

[non-BFB] only for CRYO* compsets
jonbob added a commit that referenced this pull request Sep 25, 2024
Updating CRYO compsets to v3 ELM settings

Updates the ELM settings in the following compsets to the v3 settings in
PR #6108:
* CRYO1850
* CRYO1850-DISMF
* CRYO1850-4xCO2
* CRYO1950
* CRYO1950-DISMF
* CRYO20TR
* CRYOSSP370
* CRYOSSP585
Adds following compsets matching the WCYCL counterparts:
* CRYO1850-1pctCO2
* CRYOSSP245

[NML] only for CRYO* compsets
[non-BFB] only for CRYO* compsets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BGC CC PR is climate changing compset Land non-BFB PR makes roundoff changes to answers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants