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

Add Kennedy simphydro to ELM #3001

Merged
merged 10 commits into from
Oct 28, 2019
Merged

Add Kennedy simphydro to ELM #3001

merged 10 commits into from
Oct 28, 2019

Conversation

pnlfang
Copy link

@pnlfang pnlfang commented Jun 17, 2019

Adding plant hydraulics model of Kennedy et al. (2019).

Kennedy, D., S. Swenson, K. W. Oleson, D. M. Lawrence, R. Fisher, A. C. L. da Costa,
and P. Gentine (2019), Implementing Plant Hydraulics in the Community Land Model,
Version 5, J Adv Model Earth Sy, 11(2), 485-513, doi:10.1029/2018MS001500.

[BFB]

@thorntonpe
Copy link
Contributor

thorntonpe commented Jun 17, 2019

@pnlfang has this PR gone through any testing with the test suites? Is it BFB?

@rljacob
Copy link
Member

rljacob commented Jun 17, 2019

Is this a v2 feature? Its needs a pointer to a design document.

https://acme-climate.atlassian.net/wiki/spaces/ED/pages/976946346/W2%2B-%2BPlant%2BHydraulics

@rljacob rljacob changed the title Pnlfang elm simphydro Add Kennedy simphydro to ELM Jun 17, 2019
@bishtgautam bishtgautam added this to the v2.0alpha milestone Jun 17, 2019
@pnlfang pnlfang closed this Jun 17, 2019
@pnlfang
Copy link
Author

pnlfang commented Jun 17, 2019

@pnlfang has this PR gone through any testing with the test suites? Is it BFB?
@thorntonpe I'm working on it.

@pnlfang pnlfang reopened this Jun 17, 2019
Copy link
Contributor

@bishtgautam bishtgautam left a comment

Choose a reason for hiding this comment

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

There are multiple new lines that have been commented. These new lines were probably used for debugging the model and can be deleted now.

components/clm/src/biogeophys/SoilWaterMovementMod.F90 Outdated Show resolved Hide resolved
components/clm/src/biogeophys/PhotosynthesisMod.F90 Outdated Show resolved Hide resolved
components/clm/src/biogeophys/PhotosynthesisMod.F90 Outdated Show resolved Hide resolved
lai_z_sun => canopystate_inst%laisun_z_patch ! Input: [real(r8) (:,:) ] leaf area index for canopy layer, sunlit or shaded
vcmaxcint_sun => surfalb_inst%vcmaxcintsun_patch ! Input: [real(r8) (:) ] leaf to canopy scaling coefficient
alphapsn_sun => photosyns_inst%alphapsnsun_patch ! Input: [real(r8) (:) ] 13C fractionation factor for PSN ()
!o3coefv_sun => ozone_inst%o3coefvsun_patch ! Input: [real(r8) (:) ] O3 coefficient used in photosynthesis calculation
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete the commented out lines

! Photosynthesis and stomatal conductance parameters, from:
! Bonan et al (2011) JGR, 116, doi:10.1029/2010JG001593
!==============================================================================!
! calculate root-soil interface conductance
Copy link
Contributor

Choose a reason for hiding this comment

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

Please indent this comment and multiple comments that are below


! Compare with Ball-Berry model: gs_mol = m * an * hs/cs p + b

hs = (gb_mol(p)*ceair + gs_m
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete the commented out lines


! Compare with Ball-Berry model: gs_mol = m * an * hs/cs p + b

hs = (gb_mol(p)*ceair + gs_m
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete the commented out lines


! Compare with Ball-Berry model: gs_mol = m * an * hs/cs p + b

hs = (gb_mol(p)*ceair + gs_m
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete the commented out lines


! Compare with Ball-Berry model: gs_mol = m * an * hs/cs p + b

hs = (gb_mol(p)*ceair + gs_m
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete the commented out lines


! Compare with Ball-Berry model: gs_mol = m * an * hs/cs p + b

hs = (gb_mol(p)*ceair + gs_m
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete the commented out lines

@jqyin
Copy link
Contributor

jqyin commented Jul 12, 2019

@pnlfang just to follow up, is this PR bit-for-bit against the test suites?

@pnlfang
Copy link
Author

pnlfang commented Jul 15, 2019

@jqyin yes

@jqyin jqyin added the BFB PR leaves answers BFB label Jul 15, 2019
@rljacob
Copy link
Member

rljacob commented Aug 7, 2019

@bishtgautam please re-review to see if your requested changes were made.

Copy link
Contributor

@bishtgautam bishtgautam left a comment

Choose a reason for hiding this comment

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

Please delete the commented out lines.

! if ( stomatalcond_mtd == stomatalcond_mtd_bb1987 )then
gsminsun = bbb(p)
gsminsha = bbb(p)
! else if ( stomatalcond_mtd == stomatalcond_mtd_medlyn2011 )then
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete the commented out lines.

@bishtgautam bishtgautam self-requested a review August 7, 2019 22:21
@rljacob
Copy link
Member

rljacob commented Aug 8, 2019

@jqyin please start merging this.

@rljacob
Copy link
Member

rljacob commented Aug 8, 2019

Nevermind, still needs a code review document approval from @kvcalvin

@rljacob rljacob modified the milestones: v2.0alpha, v2.0BGC Aug 22, 2019
Copy link
Contributor

@thorntonpe thorntonpe left a comment

Choose a reason for hiding this comment

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

@pnlfang I'm sorry for the slow action on this review. I have two questions/suggestions: If there is a new namelist variable that triggers this capability, could you add that as documentation in the design document? And have you considered adding a new test to the land developer's test suite to protect this new capability against unintended harm by subsequent PRs? I think with so much new mechanism within the model it would be wise to have both a multi-day smoke test and an exact restart test.

@pnlfang
Copy link
Author

pnlfang commented Aug 27, 2019

@thorntonpe I added a new namelist variable "use_hydrstress" to trigger this capability. This capability also requires an updated clm_params parameter file, which needs to be parameterized for application. I'll add this in the design document. I have a global test case that could potentially be added to the land developer's test suit.

@rljacob rljacob modified the milestones: v2.0alpha, v2.0alphaBGC Aug 30, 2019
@rljacob rljacob assigned bishtgautam and unassigned jqyin Oct 17, 2019
@rljacob
Copy link
Member

rljacob commented Oct 17, 2019

Reassigning to balance load.

@rljacob
Copy link
Member

rljacob commented Oct 24, 2019

@thorntonpe can you please finish the review for this?

@rljacob
Copy link
Member

rljacob commented Oct 24, 2019

@bishtgautam looks like this can finally be merged.

@bishtgautam
Copy link
Contributor

Today I re-merged #3107, so I will merge this PR tomorrow.

bishtgautam added a commit that referenced this pull request Oct 25, 2019
Adding plant hydraulics model of Kennedy et al. (2019).

Kennedy, D., S. Swenson, K. W. Oleson, D. M. Lawrence, R. Fisher, A. C. L. da Costa,
and P. Gentine (2019), Implementing Plant Hydraulics in the Community Land Model,
Version 5, J Adv Model Earth Sy, 11(2), 485-513, doi:10.1029/2018MS001500.

[BFB]
bishtgautam added a commit that referenced this pull request Oct 28, 2019
Adding plant hydraulics model of Kennedy et al. (2019).

Kennedy, D., S. Swenson, K. W. Oleson, D. M. Lawrence, R. Fisher, A. C. L. da Costa,
and P. Gentine (2019), Implementing Plant Hydraulics in the Community Land Model,
Version 5, J Adv Model Earth Sy, 11(2), 485-513, doi:10.1029/2018MS001500.

[BFB]
@bishtgautam bishtgautam merged commit 83435ac into master Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB Land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants