E3SM-Project / E3SM

Energy Exascale Earth System Model source code. NOTE: use "maint" branches for your work. Head of master is not validated.
https://docs.e3sm.org/E3SM
Other
336 stars 338 forks source link

Add compsets used in v2 BGC simulations #6416

Closed evasinha closed 1 week ago

evasinha commented 1 month ago

Add compsets used in v2 BGC simulations to master for better provenance and reproducibility.

The branch is a rebase of Xiaoying Shi's acme-y9s/BGCV2_compsets .

[BFB]


All tests are passing except for NLCOMP fail in several of them. The NLCOMP fail is because of three new variables being added to elm_in :

Differences in namelist 'elm_inparm':
   found extra variable: 'tw_irr'
   found extra variable: 'use_atm_downscaling_to_topunit'
   found extra variable: 'use_hydrstress'

Since all these variables are set to false by default with the new update, I think we can ignore these fails.

github-actions[bot] commented 1 month ago

PR Preview Action v1.4.7 :---: :rocket: Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6416/ on branch gh-pages at 2024-05-24 04:02 UTC

evasinha commented 3 weeks ago

@rljacob I have struggling to get the various tests in e3sm_land_developer to PASS. Few commits made since the original PR was submitted have helped resolve some of those fails but not all. Most of the test are still failing due to difference in namelist (NLCOMP fail) which is expected since three new name list variables are added to lnd_in :

tw_irr = .false.
use_atm_downscaling_to_topunit = .false.
use_hydrstress = .false.

But the bigger issue is that these:

1. ERS.ELM_USRDAT.I1850ELM.chrysalis_intel.elm-usrdat
2. ERS.ELM_USRDAT.IELM.chrysalis_intel.elm-surface_water_dynamics
3. ERS.f09_g16.I1850ELMCN.chrysalis_intel.elm-bgcinterface
4. ERS.f19_f19.I1850ELMCN.chrysalis_intel
5. ERS.f19_f19.I20TRELMCN.chrysalis_intel
6. ERS.f19_g16.I1850ELM.chrysalis_intel.elm-betr
7. ERS.f19_g16.I1850ELM.chrysalis_intel.elm-vst
8. ERS.r05_r05.IELM.chrysalis_intel.elm-lnd_rof_2way
9. SMS_Ld1.hcru_hcru.I1850CRUELMCN.chrysalis_intel
10. SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-fan
11. SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-force_netcdf_pio
12. SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-per_crop
13. SMS.r05_r05.I1850ELMCN.chrysalis_intel.elm-qian_1948

are failing due to case build failed with error required more processors, 161, than this machine can provide, 160.

And three more tests are failing due to the new namelist additions:

  1. ERS.f19_g16.I20TRGSWCNPRDCTCBC.chrysalis_intel.elm-ctc_f19_g16_I20TRGSWCNPRDCTCBC: case.setup fails with error CLM build-namelist::ELMBuildNamelist::setup_logic_do_transient_crops() : do_transient_crops should be set to true when running with irrigate = true

  2. ERS.r05_r05.IELM.chrysalis_intel.elm-V2_ELM_MOSART_features: Namelist item tw_irr contradicts the command-line option -tw_irr, use the command line option

  3. SMS.r05_r05.IELM.chrysalis_intel.elm-topounit: Namelist item use_atm_downscaling_to_topunit contradicts the command-line option -topounit

I think these three tests can be resolved by modifying the tests themselves for e.g., by using different compsets in which the test features (transient crops, tw_irr, or topounit) are on by default.

But I am not sure what is causing the 13 tests listed above to fail. @rljacob do you have any advice on where to look or how to proceed?

rljacob commented 3 weeks ago

We've seen that "required more processors" message and think it has to do with executable sharing within the land test suite. Run each test individually to see if they still fail.

For the tests failing because of namelist additions from a testmod, yes either the feature the testmod activcatesw needs a new compset or maybe the set of features doesn't make sense anymore. Do you know where to find the testmods? The first one is in components/elm/cime_config/testdefs/testmods_dirs/elm/ctc_f19_g16_I20TRGSWCNPRDCTCBC.

evasinha commented 3 weeks ago

The minor omissions identified in this commit were causing several tests to fail without any good error message to identify the cause of failure.

evasinha commented 3 weeks ago

@rljacob FINALLY after the recent few commits (typos and omits that took me a very long time to identify) all tests are passing except for NLCOMP fail in several of them. The NLCOMP fail is because of three new variables being added to elm_in :

Differences in namelist 'elm_inparm':
   found extra variable: 'tw_irr'
   found extra variable: 'use_atm_downscaling_to_topunit'
   found extra variable: 'use_hydrstress'

Since all these variables are set to false by default with the new update, I think we can ignore these fails.

The PR is now ready to be reviewed and approved.

evasinha commented 1 week ago

@bbye and @rljacob I tested running the I1850GSWCNP compset with the crop parameter file (clm_params_c230517_phs_50pfts.nc) but got the following error message:

ERROR: The provided user buffer does not have space for copying strings (space available to copy only 25 strings) pio_support::pio_die:: myrank= -1 : ERROR: pionfget_mod.F90:

I think it requires more changes to use the 50pft parameter file when crops are off and number of parameters is less than 50.

In the future, we can modify the code such that the model only reads values of the first 25 parameters (if pft=25) and ignores the rest. But in the current version the code does not support it.

rljacob commented 1 week ago

@evasinha did you want to this to be merged to maint-2.1? Because the target is currently master.

evasinha commented 1 week ago

@rljacob We would like to merge it to master. I had rebase to the current master at the time of PR submission.

rljacob commented 1 week ago

Please edit the description then. The first sentence says "Add compsets used in v2 BGC simulations to maint-2.1 for better provenance and reproducibility."

evasinha commented 1 week ago

@rljacob I have edit the description to state "Add compsets used in v2 BGC simulations to master and maint-2.1 for better provenance and reproducibility."

rljacob commented 1 week ago

But this PR is only adding it to master. I fixed it.