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

Some BFB changes and Hydraulic redistribution #1187

Merged
merged 12 commits into from
Oct 21, 2020

Conversation

negin513
Copy link
Contributor

@negin513 negin513 commented Oct 14, 2020

Description of changes

Resolves issue: #881, #1020, #1102 and includes Hydraulic redistribution

Specific notes

Contributors other than yourself, if any: @djk2120

CTSM Issues Fixed (include github issue #):
#1020, #881

Are answers expected to change (and if so in what way)? No

Any User Interface Changes (namelist or namelist defaults changes)?

  • maxpatch_pft is removed from the namelist.
  • Four additional fields added to the output:
    • QHR (patch-level hydraulic redistribution)
    • VEGWPLN (local noon vegetation water potential)
    • VEGWPPD (predawn vegetation water potential)
    • VPD_CAN: canopy vapor pressure deficit (functional input to Medlyn model)

Testing performed, if any:

  • aux_clm on Cheyenne.
  • aux_clm on Izumi

@negin513 negin513 requested a review from ekluzek October 14, 2020 19:52
@ekluzek ekluzek added PR status: ready PR: author feels this is ready to merge in tag: simple bfb easy to fix, bit-for-bit type: enhancement new capability or improved behavior of existing capability labels Oct 14, 2020
Copy link
Contributor

@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 questions that I ask about, and one required change. It looks like a section of soilm variables in the namelist xml files was accidentally deleted. So that should go back in. There's a test that should have failed as a result.

It sounds like there's some other tests that are failing as well, that will need to be addressed.

bld/namelist_files/namelist_defaults_ctsm.xml Show resolved Hide resolved
src/biogeophys/PhotosynthesisMod.F90 Show resolved Hide resolved
src/biogeophys/PhotosynthesisMod.F90 Outdated Show resolved Hide resolved
src/biogeophys/PhotosynthesisMod.F90 Show resolved Hide resolved
@negin513 negin513 requested a review from ekluzek October 14, 2020 21:52
Copy link
Contributor

@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.

OK, I missed the reason the soilm fields were removed was just because they were duplicated. So I removed that request. Now, we just need to get the restart tests working. And I had a few other comments that you can decide how to handle.

@dlawrenncar
Copy link
Contributor

dlawrenncar commented Oct 15, 2020 via email

Copy link
Contributor

@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.

@negin513 is testing this with her PR. There are a few changes she'll make there for this PR. @djk2120 is also going to make some future changes that will resolve some other questions I had.

Copy link
Contributor

@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.

So the only thing to do is to get the restart tests passing, by adding the two fields to restart files. And then add the new fields to be active by default.

src/biogeophys/CanopyStateType.F90 Show resolved Hide resolved
@dlawrenncar
Copy link
Contributor

dlawrenncar commented Oct 15, 2020 via email

@ekluzek
Copy link
Contributor

ekluzek commented Oct 15, 2020

@dlawrenncar yep we just went over that with @negin513, and showed her how to add to the restart file. So she is working on that. That should resolve the fails.

@negin513
Copy link
Contributor Author

@djk2120 and @ekluzek : Can you please confirm the name of the second dimensions of VEGWPLN and VEGWPPD?

call restartvar(ncid=ncid, flag=flag, varname='VEGWPLN', xtype=ncd_double, &
dim1name='pft', dim2name='vegwcs', &
long_name='vegetation water matric potential for sun/sha canopy,xyl,root at local noon', units='mm', &
interpinic_flag='skip', readvar=readvar, data=this%vegwp_ln_patch)
call restartvar(ncid=ncid, flag=flag, varname='VEGWPPD', xtype=ncd_double, &
dim1name='pft', dim2name='vegwcs', &
long_name='predawn vegetation water matric potential for sun/sha canopy,xyl,root', units='mm', &
interpinic_flag='skip', readvar=readvar, data=this%vegwp_pd_patch)

At first, I had this as nvegwcs which caused all tests to fail and when I changed it to vegwcs some tests are passing and some are failing during the run phase.

If the second dimension is nvegwcs, should I define another dimension to restart files called nvegwcs? Please look here for the current dimension definitions:

call ncd_defdim(ncid , nameg , numg , dimid)
call ncd_defdim(ncid , namel , numl , dimid)
call ncd_defdim(ncid , namec , numc , dimid)
call ncd_defdim(ncid , namep , nump , dimid)
call ncd_defdim(ncid , nameCohort , numCohort , dimid)
call ncd_defdim(ncid , 'levgrnd' , nlevgrnd , dimid)
call ncd_defdim(ncid , 'levlak' , nlevlak , dimid)
call ncd_defdim(ncid , 'levsno' , nlevsno , dimid)
call ncd_defdim(ncid , 'levsno1' , nlevsno+1 , dimid)
call ncd_defdim(ncid , 'levtot' , nlevsno+nlevgrnd, dimid)
call ncd_defdim(ncid , 'numrad' , numrad , dimid)
call ncd_defdim(ncid , 'levcan' , nlevcan , dimid)
if ( use_hydrstress ) then
call ncd_defdim(ncid , 'vegwcs' , nvegwcs , dimid)
end if
call ncd_defdim(ncid , 'glc_nec', maxpatch_glcmec, dimid)
call ncd_defdim(ncid , 'glc_nec1', maxpatch_glcmec+1, dimid)

@djk2120
Copy link
Contributor

djk2120 commented Oct 19, 2020 via email

@negin513 negin513 merged commit 619b8cc into ESCOMP:master Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR status: ready PR: author feels this is ready to merge in tag: simple bfb easy to fix, bit-for-bit type: enhancement new capability or improved behavior of existing capability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants