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

Added new files to allow Gross Unrepresented Land Use transition #309

Merged
merged 37 commits into from
Mar 16, 2023

Conversation

lawrencepj1
Copy link

This is the beginning of a pull request to get Gross Unrepresented Land Use transitions working in CLM5

@ekluzek ekluzek self-requested a review March 2, 2018 00:28
@ekluzek ekluzek self-assigned this Mar 2, 2018
@ekluzek ekluzek requested a review from dlawrenncar March 2, 2018 00:28
@ekluzek
Copy link
Collaborator

ekluzek commented Mar 2, 2018

@lawrencepj1 I think some of the changes in soilbiogeochem files aren't really supposed to be there. I think they are undoing a previous commit.

@lawrencepj1
Copy link
Author

lawrencepj1 commented Mar 2, 2018 via email

@ekluzek
Copy link
Collaborator

ekluzek commented Mar 2, 2018

@lawrencepj1 the files in soilbiogeochem that I mention above are causing your build problems. They are undoing part of the clm4_5_18_r268 commit. Here's the files that were updated in that tag:

M components/clm/src/soilbiogeochem/SoilBiogeochemNitrogenStateType.F90 --- Change ncrit to totvegcthresh and SetNCrit to SetTotVgCThresh
add abort if totvegcthresh is unset, and check that it's greater than zero
M components/clm/src/soilbiogeochem/SoilBiogeochemPrecisionControlMod.F90 - Set totvegcthresh to 0.1 and pass down via setTotVgCThresh methods
M components/clm/src/soilbiogeochem/SoilBiogeochemCarbonStateType.F90 ----- Change ccrit to totvegcthresh and SetNCrit to SetTotVgCThresh
add abort if totvegcthresh is unset, and check that it's greater than zero

These are the problems you are running into.

@billsacks
Copy link
Member

Is this due to copying in files that were originally in SourceMods, or perhaps were on an svn branch off of an older version of the code? The best way to deal with this, in my experience is:

  1. Create a branch it git off of the trunk version that was the baseline for your changes

  2. Copy the changed files in and commit them

  3. git merge up to latest master

…ivation.

These files were actually changes in r267 for Keith's simulations. By including
the updated files this resulted in a build error.
@lawrencepj1
Copy link
Author

lawrencepj1 commented Mar 5, 2018 via email

@billsacks
Copy link
Member

@lawrencepj1 - the PR is automatically updated when you push to the branch (which you already did). So no need to create a new PR.

Peter Lawrence and others added 2 commits April 2, 2018 15:57
CLM tests all pass now with the note that the new code changes the DWT_SLASH_CFLUX
history variable from the column to the grid cell to be consistent with the other
DWT fluxes. An additional variable DWT_SLASH_CFLUX_PATCH has been added to allow
subgrid calculation at the patch and column levels as done with other DWT fluxes.
This change results in a failure in the ERS_Ly3.f10_f10.I1850Clm50BgcCrop test
that requires the user_nl_clm file to specify the hist_fincl3 to use DWT_SLASH_CFLUX_PATCH
rather than DWT_SLASH_CFLUX. After this change the test passes.
. The result from the test suite are:
================================================================================
These tests failed
================================================================================
ERP_D_Lm9.f10_f10_musgs.IHistClm50BgcCrop.cheyenne_intel.clm-ciso_monthly               EXPECTED
ERS_Ly5_P72x1.f10_f10_musgs.IHistClm45BgcCrop.cheyenne_intel.clm-cropMonthOutput                EXPECTED
NCK_Ld1.f10_f10_musgs.I2000Clm50Sp.cheyenne_intel.clm-default           EXPECTED
SMS_D_Ld5.f45_f45_mg37.I2000Clm50Fates.cheyenne_intel.clm-Fates         EXPECTED
SMS_D_Lm13.f10_f10_musgs.IHistClm50BgcCrop.cheyenne_intel.clm-ciso_monthly              EXPECTED
SMS_D_Lm6.f45_f45_mg37.I2000Clm50Fates.cheyenne_intel.clm-Fates         EXPECTED
SMS_D_Lm6_P144x1.f45_f45_mg37.I2000Clm50Fates.cheyenne_intel.clm-Fates          EXPECTED
SMS_Ld1.f09_g16.I1850Clm40SpCruGs.cheyenne_intel.clm-40default          EXPECTED
@ekluzek ekluzek added this to the future milestone Apr 12, 2018
@ekluzek
Copy link
Collaborator

ekluzek commented Apr 12, 2018

We want to bring this change in after the CESM2.0 release.

@ekluzek
Copy link
Collaborator

ekluzek commented Jul 23, 2018

@lawrencepj1 here are the notes on adding namelist items to CTSM:

https://wiki.ucar.edu/display/ccsm/Adding+New+Namelist+Items+to+CLM

Update cime to cime5.7.3

Update cime from cime5.6.10 to cime5.7.3. To support this change, there
are also minor code changes related to the pause-resume implementation
(from Erik Kluzek).

Fixes ESCOMP#384
@ekluzek
Copy link
Collaborator

ekluzek commented Sep 21, 2018

We talked about this. We are going to pull this to master as a second priority for the cesm2.1 critical tasks (the critical tasks are new ndep, paero, and co2 datasets). We do want the switch in here for both cesm2.2 and cesm2.1 if possible. We also plan on updating the surface datasets for cesm2.1/cmip6 with JUST the change adding in the new field MAX PFT/CFT so we can speed up the model for cesm2.1. As a later change we will also do another update of the rawpft files (with changes to crops and coastlines, as well as adding non-zero GRU fields), but that will JUST go on the cesm2.2 development cycle on master (and won't go into the clm5.0 release branch that is being used for cesm2.1 and cmip6 development.

@ekluzek
Copy link
Collaborator

ekluzek commented Sep 22, 2018

@lawrencepj1 to update this PR, you just need to:

cd /gpfs/fs1/work/lawrence/clm5development/ctsmshiftingcultivatio
git push

You've already checked in your changes we just need to push them to your branch on your fork, and this PR will update.

Peter Lawrence and others added 2 commits September 22, 2018 08:36
… do_grossunrep is set to .false. When set to .true. do_grossunrep turns on shifting cultivation which is read off the landuse_timeseries file.
Add water tracer consistency checks, and other water tracer work

1. Add water tracer consistency checks

2. Add infrastructure for looping over all water tracers - currently
   just used for the tracer consistency checks

3. Breakout of atm2lnd and lnd2atm water variables, needed for water tracers

4. Add some namelist control over the addition of water tracers

5. Add a system test that exercises the water tracer consistency checks

6. Add a 'ratio' variable for each water tracer

7. Add some unit tests of the new water tracer infrastructure

Issues fixed:
- Partially addresses ESCOMP#357
- Resolves ESCOMP#479
- Resolves ESCOMP#492
@ekluzek
Copy link
Collaborator

ekluzek commented Sep 24, 2018

Hmmm. OK. I updated to ctsm1.0.dev011 and ran standard testing. I only saw expected fails. But, strangely, I see the following tests as changing answers:

ERP_P36x2_Lm13.f10_f10_musgs.IHistClm50Bgc.cheyenne_gnu.clm-monthly
ERP_P36x2_Lm13.f10_f10_musgs.IHistClm50Bgc.cheyenne_intel.clm-monthly
ERP_P36x2_Lm25.f10_f10_musgs.I2000Clm50BgcDvCrop.cheyenne_intel.clm-monthly
ERP_P72x2_Lm25.f10_f10_musgs.I2000Clm50BgcDvCrop.cheyenne_intel.clm-monthly
ERS_D_Ld3.f19_f19_mg16.I1850Clm40SpCruGs.cheyenne_intel.clm-40default
ERS_D_Ld3.f19_f19_mg16.I2000Clm40SpCruGs.cheyenne_intel.clm-40default
ERS_D_Ld3.f19_f19_mg16.IHistClm40SpCruGs.cheyenne_intel.clm-40default
SMS_D_Ld3.f19_f19_mg16.I1850Clm40SpCruGs.cheyenne_intel.clm-40default
SMS_D_Ld3.f19_f19_mg16.I2000Clm40SpCruGs.cheyenne_intel.clm-40default
SMS_D_Ld3.f19_f19_mg16.IHistClm40CnCruGs.cheyenne_intel.clm-40default

I haven't looked into why this is yet.

@billsacks billsacks added enhancement new capability or improved behavior of existing capability and removed type: enhance - science labels May 24, 2019
@ekluzek
Copy link
Collaborator

ekluzek commented Aug 6, 2019

These are the changes in CLM to take advantage of shifting cultivation from the surface datasets. The other part of this is to add new data to the surface datasets.

Note, there are some conflicts that need to be resolved to bring this to master at this point.

ekluzek added 2 commits August 6, 2019 15:32
Modularize snow cover fraction method

This tag moves the calculation of frac_sno - and the related updates of
snow_depth - into a new set of classes, with one class for each
parameterization (Niu & Yang 2007 and Swenson & Lawrence 2012).

Previously, the code always calculated frac_sno the new way, but then
possibly overwrote it if using the older Niu & Yang method. The new code
cleans this up, only doing the calculations that are needed for each
method.

In addition, other code that is specific to one of the two methods is
now moved to a home that makes this dependence on method explicit. This
includes the addition of newsnow to int_snow: previously, int_snow was
always updated using an equation specific to the newer CLM5
parameterization of frac_sno, which was not appropriate if using the Niu
& Yang parameterization; this doesn't make a difference currently, since
int_snow is only referenced if using the Swenson & Lawrence
parameterization, but this clears up some confusion. Also, time-constant
parameters read from namelist or the netCDF parameter file now reside in
the appropriate class rather than being more global.

This tag also renames two namelist options to increase clarity:
- subgridflag is renamed to use_subgrid_fluxes, and is now a logical
- oldfflag is renamed to snow_cover_fraction_method, and is now a string

 Conflicts:
	bld/CLMBuildNamelist.pm
	bld/namelist_files/namelist_definition_ctsm.xml
	cime_config/usermods_dirs/cmip6_output/user_nl_clm
	src/biogeochem/CNVegCarbonFluxType.F90
	src/dyn_subgrid/dynSubgridControlMod.F90
@ekluzek ekluzek marked this pull request as ready for review February 17, 2023 17:01
@slevis-lmwg
Copy link
Contributor

@fischer-ncar told me that he had to change his default shell from tcsh to bash on izumi to get past this error.
@fischer-ncar said that I needed to get in touch with cgd support to change my default shell.

@slevis-lmwg
Copy link
Contributor

Test-suites completed and dev119 baselines are ready on cheyenne and izumi.

@ekluzek this PR is ready to merge, though you may wish to look over my ChangeLog entry first.

@slevis-lmwg slevis-lmwg added PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete and removed PR status: work in progress labels Feb 23, 2023
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

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

I have a couple small comments that could be acted on. But, I'm not sure it's worth doing since testing is done.

There's a larger question for Peter to answer though. Depending on his answer we can do the small changes I ask about. We could also go ahead with my suggestions, but only repeat a small part of the testing since the changes are small.

cime_config/testdefs/testlist_clm.xml Show resolved Hide resolved
bld/unit_testers/build-namelist_test.pl Outdated Show resolved Hide resolved
src/dyn_subgrid/dynSubgridControlMod.F90 Outdated Show resolved Hide resolved
ekluzek and others added 4 commits February 27, 2023 14:55
This was an unintentional line removal that Erik realized.
I also reordered the error checks that Erik recommended in
dynSubgridControlMod.
@lawrencepj1
Copy link
Author

Hi @slevisconsulting and @ekluzek

I just started going through the PR 309 code and noticed some things straight up. The first is that the dynHarvestMod.F90 file that the dynGrossUnrepMod.F90 file is based on has moved to the dyn_subgrid directory from the biogeochem directory since the branch I used back in 2019. There are also subtle changes in the module such as fates flags specifying that harvest is not done with fates on.

! we only need to keep this summary variable in CN veg type
if ( .not. use_fates ) then  

I am going to do some checks between the updated files from the latest branch without the PR and the version of CTSM I branched from. This may take some work. I have a couple of other tasks that I need to focus on today and tomorrow but I wanted to start on this today. I will let you know more once I have had some more time.

Thanks
Peter

@wwieder
Copy link
Contributor

wwieder commented Mar 9, 2023

@lawrencepj1 do you know when you're going to have time to look at this?

@ekluzek is suggesting that we bring this in soon, to clear up some other tags, with subsequent changes that need to made for GULU coming in on the CTSM5.2 branch (where we actually have the datasets needed for these transitions).

@lawrencepj1
Copy link
Author

Hi @wwieder

Yes this is the next task after finishing the rankings for the CESM Tutorial applications which are due today. I will make it my first priority for this afternoon or tomorrow morning once I get the applications completed.

Peter

@lawrencepj1 do you know when you're going to have time to look at this?

@ekluzek is suggesting that we bring this in soon, to clear up some other tags, with subsequent changes that need to made for GULU coming in on the CTSM5.2 branch (where we actually have the datasets needed for these transitions).

@ekluzek
Copy link
Collaborator

ekluzek commented Mar 16, 2023

OK, in order to facilitate the next tag that needs to come in, we are bringing in this tag as is. If @lawrencepj1 finds changes that need to happen it can happen in a future tag either on main-dev or in the ctsm5.2 branch.

@ekluzek ekluzek merged commit f72479b into ESCOMP:master Mar 16, 2023
@ekluzek ekluzek deleted the clm5_shiftingcultivation branch March 16, 2023 20:17
@wwieder
Copy link
Contributor

wwieder commented Mar 16, 2023

YES Erik!

@samsrabin samsrabin added the science Enhancement to or bug impacting science label Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new capability or improved behavior of existing capability PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete science Enhancement to or bug impacting science
Development

Successfully merging this pull request may close these issues.

6 participants