Updates winter cereal crop based on Yaqiong Lu et al.,2017

[evasinha]

This feature adds new parameterizations to represent winter wheat crop in ELM.

The implementation is based on Yaqiong Lu’s implementation of winter wheat in CLM4.5. https://doi.org/10.5194/gmd-10-1873-2017

The model was calibrated using the methodology used in Sinha et al. 2022.
https://doi.org/10.1029/2022MS003171

[non BFB] for Crops

[peterdschwartz]

Also, is this BFB due to being a stealth feature?

[evasinha]

@peterdschwartz I am not sure why the results were BFB even though winter wheat had so many edits and it is represented in the LUT's used for crop tests. I am happy to investigate it further, if you want.

[peterdschwartz]

@evasinha I'll defer to your judgement about what's expected/reasonable for this. I asked to make sure the PR is labelled correctly and if it potentially requires a new test.

[evasinha]

@peterdschwartz I figured out why the test was BFB.

1. The crops turn on only in the second year of the simulation and winter wheat is planted in October/November and has a positive LAI in the winter of the following year. Thus, 2-year runs are too short to capture the winter wheat growth.

2. There is only one crop run with LUT (lulcc_sville). It runs for 2 years, but in the first two years, the percent cropland (PCT_CROP) is zero in the LUT.

Adding to the existing comment:
3. The lulcc_sville test was not using a 20TR compset and hence the PCT_CROP, PCT_CFT etc., were not changing over time.

I am thinking of:

1. Making modifications to run the crop test for a larger number of years
2. Save the output for a few variables at the pft/cft/column level for easier comparison.
3. Use I20TRGSWCNPCROP compset for the lulcc_sville test.

[evasinha]

@peterdschwartz I have modified the existing test so that winter wheat implementation can be tested.

Please let me know if any new addition/modification is needed.

Should I label this as non-BFB now (the modifications to the test don't exist in the current master, so there is no direct comparison)?

[peterdschwartz]

That looks good. I believe the PR should be labelled "[non-BFB] for crops" since the test changes were made to capture the science changes.

[evasinha]

@peterdschwartz I have modified the label to non-BFB.

Thanks for your help in fixing issues, finalizing, and approving this PR.

[bbye]

Hi Eva, this looks good to me.

[evasinha]

@bbye Thanks for reviewing and approving the PR.

[peterdschwartz]

submitted e3sm_developer tests with this merged to next on perlmutter, but there's been maintenance interruptions so may not see results till sometime tomorrow

[evasinha]

@peterdschwartz Just wanted to check on the status of this PR merge. Is Perlmutter maintenance still slowing/stopping the merge?

[peterdschwartz]

The maintenance resulted in >20 tests failing -- some were the crop and other land tests. They are still failing as of last night, so i will just do the tests on chrysalis instead, which isn't showing any issues.

[peterdschwartz]

Testing diffs from e3sm_developer using latest next:

  SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-fan (Overall: DIFF) details:
  SMS_Ly5_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-force_netcdf_pio (Overall: DIFF) details:
  SMS_Ly5_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-per_crop (Overall: DIFF) details:
  SMS_Ly6_P1x1_D.1x1_smallvilleIA.I20TRGSWCNPCROP.chrysalis_intel.elm-lulcc_sville (Overall: DIFF) details:

Note: one extra eamxx diff was reported but likely due to another PR. Diffs look as expected to me.

pm-cpu is still showing issues. I'll look further into that later today.

[peterdschwartz]

Merged to next! Been busy at the all hands sorry for the delays and confusion

[evasinha]

@peterdschwartz No worries at all. I understand.

[peterdschwartz]

Testing looks good as far as results but the new compset causes that SMS_Ly6_P1x1_D.1x1_smallvilleIA.I20TRGSWCNPCROP.chrysalis_intel.elm-lulcc_sville to run for >1hr (other tests are a few minutes). I am running that test myself to see about the performance.

It's in debug mode, so I'll look at the performance in optimized mode for an easy improvement and look at how long less years would take. I may end up suggesting reverting to the old compset for that test, and we could coordinate another PR to fill-in the testing gap for crops to give more time for cost-benefit analysis.

Name 	Status	Time	Details	History	Summary
flag SMS_Ly6_P1x1_D.1x1_smallvilleIA.I20TRGSWCNPCROP.chrysalis_intel.elm-lulcc_sville	Failed	1h 9m 10s	Completed (DIFF)
flag SMS_Ly5_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-per_crop	Failed	6m 42s	Completed (DIFF
flag SMS_Ly5_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-force_netcdf_pio	Failed	7m	Completed (DIFF

[evasinha]

@peterdschwartz I had modified the lulcc_sville test to use I20TRGSWCNPCROP compset as the original test was not using a 20TR compset and hence the PCT_CROP, PCT_CFT etc., were not changing over time.

However, the use of GSWP3 data for met forcing is slowing it down. Is there a way to reduce the time spent reading this forcing? For instance, the test needs to run for only 6 hours; maybe read the GSWP3 data for six years only?

[peterdschwartz]

In optimized mode the run time is down to 30 mins but the land is only ~3minutes. Changing the stream year end didn't change performance since the bottleneck is that the file is read every timestep instead of all at once. I'm experimenting with the datm_in nml to read each yearly file all at once. Exploring CPL_BYPASS is another alternative.

Just to be clear - I think changing the test is for the best so if we revert the compset for this PR, then I would open another PR that we could merge next week to better configure the test. That way your work can be in master today while I experiment with datm config.

[peterdschwartz]

Trying to get datm to read the full file throws an error. It's the same as an error I'm trying to fix for reading lnfm data, so I may have a solution for that soon. I just went ahead with changing the test to non-debug and to run for only 1 year so that the total time is now ~6mins on chrysalis with the 20TRGSW compset.

I will go ahead with the merge and plan to revisit this with either a new datm config or OLMT CPL_BYPASS solution.

[evasinha]

@peterdschwartz The crops turn on only in the second year of the simulation, thus, 1-year run is too short to capture the crop growth. Additionally, the percent cropland (PCT_CROP) in the land use time series used for lulcc_sville is zero for the first 2 years.

We should run this test for at least 2 years with a modified surface dataset. I can work on creating the revised surface dataset if the suggestions look okay to you.

[peterdschwartz]

Makes sense. I made an issue #7069 to track and discuss this test. If you want to describe the proposed changes to the surface dataset there, that'd be helpful for me. Thanks

[evasinha]

Thanks @peterdschwartz .

I have created a modified land use time series (LUT) for the lulcc_sville test that has PCT_CROP set to 100% for the fist two years and also has modified values for PCT_CFT and FERTNITRO_CFT for the first two years. The new LUT name and location is:
/compyfs/inputdata/lnd/clm2/surfdata_map/landuse.timeseries_1x1_smallvilleIA_mp50_hist_simyr1850-1855_c250228.nc

[rljacob]

The test SMS_Ly1_P1x1.1x1_smallvilleIA.I20TRGSWCNPCROP.chrysalis_intel.elm-lulcc_sville still needs to be blessed. Are you holding off until you update the length?

[peterdschwartz]

I went ahead and submitted the bless requests earlier today. Mainly had to hold off since one of the other tests was failing due to machine issues (seemingly) on pm-cpu_intel.