Skip to content

Add generic BGC interface into ACME land model#330

Merged
daliwang merged 24 commits intoE3SM-Project:masterfrom
wanggangsheng:wanggangsheng/lnd/bgc-interface
Oct 6, 2015
Merged

Add generic BGC interface into ACME land model#330
daliwang merged 24 commits intoE3SM-Project:masterfrom
wanggangsheng:wanggangsheng/lnd/bgc-interface

Conversation

@wanggangsheng
Copy link
Copy Markdown
Contributor

  1. These code modifications are for implementing a generic BGC interface in ACME land model.

  2. Three flags are added into the namelist group 'clm_inparm':
    2.1. 'use_bgc_interface': runtime flag to turn on/off BGC interface.
    2.2. 'use_clm_bgc': runtime flag to turn on/off running ALM soil BGC sub-model through BGC interface.
    2.3. 'use_pflotran': runtime flag to turn on/off pflotran

  3. New namelist group ('clm_pflotran_inparm') is created and two flags are added into 'clm_pflotran_inparm':
    3.1. 'pflotran_prefix': pflotran file prefix.
    3.2. 'plfotran_inputdir': pflotran input directory.

  4. Major code changes include:
    4.1. Three new F90 files are added into 'lnd/clm/src/main':
    (1) clm_bgc_interface_data.F90: define and initialize clm_bgc_interface_data_type
    (2) clm_bgc_interfaceMod.F90: include subroutines to implement BGC interface
    (3) clm_pflotran_interfaceMod.F90: include subroutines to implement coupling of pflotran to ALM.

    4.2. break down the subroutine 'CNEcosystemDynNoLeaching' into two subroutines:
    (1) CNEcosystemDynNoLeaching1: called before running soil BGC sub-model
    (2) CNEcosystemDynNoLeaching2: called after running soil BGC sub-model

    4.3. break down the subroutine 'CNDecompAlloc' into two subroutines:
    (1) CNDecompAlloc: the standalone ALM soil BGC module that keeps the majority of code in original 'CNDecompAlloc', only Phase-2 of CNAllocation (i.e., CNAllocation2_ResolveNPLimit) is called in this subroutine.
    (2) CNDecompAlloc2: integration of net_nmin/pmin, gross_nmin/pmin to column level is moved from original CNDecompAlloc to this subroutine; Phase-3 of CNAllocation (i.e., CNAllocation3_PlantCNPAlloc) is implemetned in this subroutine. Additional calculations required by specific soil BGC sub-model may also be included in this subroutine, e.g, integration of plant N/P uptake to column level and calculation of fpg and fpi while pflotran is coupled to ALM.

    4.4. break down the subroutine 'CNAllocation' into three subroutines
    (1) CNAllocation1_PlantNPDemand: calculate plant N/P demand; called in CNEcosystemDynNoLeaching1.
    (2) CNAllocation2_ResolveNPLimit: Resolve N/P Limitation; called in CNDecompAlloc
    (3) CNAllocation3_PlantCNPAlloc: Plant C/N/P Allocation; called in CNDecompAlloc2

  5. This PR will not be answer-changing if 'use_bgc_interface' and 'use_clm_bgc' are turned on.

[NML], [BFB]
LG-31, LG-80

@wanggangsheng wanggangsheng changed the title Wanggangsheng/lnd/bgc interface add generic BGC interface into ACME land model Sep 21, 2015
@wanggangsheng wanggangsheng changed the title add generic BGC interface into ACME land model Add generic BGC interface into ACME land model Sep 21, 2015
@daliwang
Copy link
Copy Markdown
Contributor

@wanggangsheng and @rljacob. I have problem to merge wanggangsheng/lnd/bgc-interface with next after the CIME merge into origin/next. I was able to do this merge on Monday evening before CIME merge.

wangd/ACME> git merge --no-ff wanggangsheng/lnd/bgc-interface
CONFLICT (modify/delete): models/lnd/clm/bld/CLMBuildNamelist.pm deleted in HEAD and modified in wanggangsheng/lnd/bgc-interface. Version wanggangsheng/lnd/bgc-interface of models/lnd/clm/bld/CLMBuildNamelist.pm left in tree.
Auto-merging components/mpasli/model
Adding as components/mpasli/modelHEAD instead
Auto-merging components/mpas-o/model
Adding as components/mpas-o/model
HEAD instead
......
Auto-merging components/clm/bld/namelist_files/namelist_definition_clm4_5.xml
Auto-merging components/clm/bld/namelist_files/namelist_defaults_clm4_5.xml
warning: inexact rename detection was skipped due to too many files.
warning: you may want to set your merge.renamelimit variable to at least 6041 and retry the command.
Automatic merge failed; fix conflicts and then commit the result.

https://gist.github.com/835b12488ffc6cae4e82
daliwang / gist:835b12488ffc6cae4e82

@rljacob
Copy link
Copy Markdown
Member

rljacob commented Sep 23, 2015

If you do "git status" what does it list for "Unmerged paths:" ?

@daliwang
Copy link
Copy Markdown
Contributor

@rljacob Thanks. I will work to solve the CLMBuildNamelist.pm today.

@wanggangsheng
Copy link
Copy Markdown
Contributor Author

@bishtgautam & @daliwang: This PR is focused on bgc-interface and ensures clm-bgc could be run through the interface. The clm_pflotran code has also been tested on point-scale run. I will keep working on the clm_pflotran code based on bgc-interface after V1.

@daliwang
Copy link
Copy Markdown
Contributor

"# Unmerged paths:# (use "git add/rm ..." as appropriate to mark resolution)
"# added by us: components/mpas-o/model
"# added by us: components/mpasli/model

@rljacob, I tried "git add components/mpas-o/model" and get the message:

error: unable to index file components/mpas-o/model
fatal: updating files failed

These two folders are there, but just empty? Any suggestion?

@rljacob
Copy link
Copy Markdown
Member

rljacob commented Sep 25, 2015

Maybe @douglasjacobsen can help.

@daliwang
Copy link
Copy Markdown
Contributor

@douglasjacobsen, can you offer some help? We need this PR for collaborations with BNL.

@douglasjacobsen
Copy link
Copy Markdown
Member

@daliwang Yes. So, the thing you need to do is have the submodules checked out prior to performing the merge.

And the submodules need to be checked out in the place they are going to be. (i.e. components/mpas-o/model)

You should probably do the following (again before doing the merge):

git fetch origin
git checkout origin/next
git submodule update --init
git merge <feature_branch_to_merge>

Then, if it lists conflicts again with components/mpas-o/model (or similar) you should be able to git add them without any issues. This is only due to the submodule moving places for cime.

@daliwang
Copy link
Copy Markdown
Contributor

@douglasjacobsen . Thanks. I will try to make the merge tonight and keep you posted on the progress.

@daliwang
Copy link
Copy Markdown
Contributor

@rljacob and @douglasjacobsen. I merged the branch on my local repo. Where I can find the new information on create_test? The test scripts have been moved too?

@bishtgautam
Copy link
Copy Markdown
Contributor

scripts/create_test is now available at cime/scripts/create_test

daliwang added a commit that referenced this pull request Sep 29, 2015
* wanggangsheng/lnd/bgc-interface: (24 commits)

[CC], [NML], [BFB]
LG-31, LG-80

Conflicts solved by Dali Wang 09/29/2015
	components/mpas-o/model
	components/mpasli/model
	models/lnd/clm/bld/CLMBuildNamelist.pm
@rljacob
Copy link
Copy Markdown
Member

rljacob commented Sep 29, 2015

I forgot to say this: don't push any more changes to origin/next until we're done with the cime testing. You don't have to undo your recent merge.

@rljacob
Copy link
Copy Markdown
Member

rljacob commented Sep 29, 2015

Looks like this will have to be reverted. The conflicts in CLMBuildNamelist.pm were not resolved correctly. The CIME-changes were discarded in favor of the old build system.

@rljacob
Copy link
Copy Markdown
Member

rljacob commented Sep 29, 2015

I've fixed this by committing a new CLMBuildNamelist.pm directly to next. That should allow testing to run. Don't revert it but this branch will have to be updated when its merged to master.

@rljacob
Copy link
Copy Markdown
Member

rljacob commented Sep 29, 2015

Is this an answer-changing commit?

@bishtgautam
Copy link
Copy Markdown
Contributor

@rljacob: Though the commit message has both [CC] and [BFB], I believe this PR should be BFB.

@wanggangsheng, can you confirm that this PR is BFB?

@rljacob
Copy link
Copy Markdown
Member

rljacob commented Sep 30, 2015

Yes it should be one or the other.

@rljacob
Copy link
Copy Markdown
Member

rljacob commented Sep 30, 2015

I did a force push on next to remove this and keep the changes @jgfouca checked in afterwards. You'll have to merge this to next again but wait until CIME clears.

@daliwang
Copy link
Copy Markdown
Contributor

@rljacob. OK. I will merge it later. It is a BFB commit.

@wanggangsheng
Copy link
Copy Markdown
Contributor Author

@rljacob, @bishtgautam: @daliwang is right, this PR is BFB.

daliwang added a commit that referenced this pull request Oct 3, 2015
* wanggangsheng/lnd/bgc-interface: (24 commits)

  [CC], [NML], [BFB]
  LG-31, LG-80

  add control on re-calculate sminn_to_plant_vr
  clean-up code
  Simplify var list for CNDecompAlloc & CNAllocation2
  Simplify var list for CNAllocation2 & CNDecompAlloc
  redo comments
  clean up code
  bgc_interface initialization
  clean up code
  correct ARGUMENT intent: in or inout
  add plant_ndemand_col(c) to CNNitrogenFluxType and clean up code
  Exclude Fire fluxes from pflotran external input accounting
  set initial t_soisno_col to its default value (274K)
  avoid NaN in pflotran fpi_vr calculation
  change CNDecompAlloc1 to CNDecompAlloc
  add restart variables for interface
  generic clm_bgc_interface
  add run CNDecompAlloc1 through clm_bgc_interface
  complete test run of clm_pf based on clm_bgc_interface
  Simplify clm_pflotran_interface based on clm_bgc_interface
  use clm_bgc_data instead of original CLM types in clm_pflotran_interface
  ...

Conflicts:
	components/mpas-o/model
	components/mpasli/model
	models/lnd/clm/bld/CLMBuildNamelist.pm
@rljacob
Copy link
Copy Markdown
Member

rljacob commented Oct 3, 2015

@daliwang, the merge commit message on next has " [CC], [NML], [BFB]". It should only have one of [BFB] or [CC]. @wanggangsheng , edit the initial message at the top of this PR to remove [CC] if this is not climate changing.

@daliwang
Copy link
Copy Markdown
Contributor

daliwang commented Oct 5, 2015

@rljacob. Removed [CC] tag.

@daliwang
Copy link
Copy Markdown
Contributor

daliwang commented Oct 5, 2015

@rljacob and @jgfouca, I am not sure how to interpret the result on http://my.cdash.org/index.php?project=ACME_Climate&date=. My test on Titan are always good, but is it an OK to push this branch onto master, considering the result on CDASH?

@jgfouca
Copy link
Copy Markdown
Member

jgfouca commented Oct 5, 2015

@daliwang , just worry about melvin and skybridge for now. Melvin has 1 expected fail, ERS.f45_g37.B1850C5. Skybridge has two expected fails, the aquaplanet stuff that I think Rob just fixed and SMS_D. So it looks like the only changes were to namelists, so you're good to merge.

@daliwang daliwang merged commit bc85852 into E3SM-Project:master Oct 6, 2015
daliwang added a commit that referenced this pull request Oct 6, 2015
* wanggangsheng/lnd/bgc-interface: (24 commits)
  [NML], [BFB]
  LG-31, LG-80
  =====================
  add control on re-calculate sminn_to_plant_vr
  clean-up code
  Simplify var list for CNDecompAlloc & CNAllocation2
  Simplify var list for CNAllocation2 & CNDecompAlloc
  redo comments
  clean up code
  bgc_interface initialization
  clean up code
  correct ARGUMENT intent: in or inout
  add plant_ndemand_col(c) to CNNitrogenFluxType and clean up code
  Exclude Fire fluxes from pflotran external input accounting
  set initial t_soisno_col to its default value (274K)
  avoid NaN in pflotran fpi_vr calculation
  change CNDecompAlloc1 to CNDecompAlloc
  add restart variables for interface
  generic clm_bgc_interface
  add run CNDecompAlloc1 through clm_bgc_interface
  complete test run of clm_pf based on clm_bgc_interface
  Simplify clm_pflotran_interface based on clm_bgc_interface
  use clm_bgc_data instead of original CLM types in clm_pflotran_interface
  ...

Conflicts fixed by Dali Wang 10/06/2015
	components/mpas-o/model
	components/mpasli/model
	models/lnd/clm/bld/CLMBuildNamelist.pm
@bishtgautam
Copy link
Copy Markdown
Contributor

@wanggangsheng : I'm trying to better understand the changes made in this PR.

  • CNAllocation() was broken into three subroutines
    • CNAllocation1_PlantNPDemand()
    • CNAllocation2_ResolveNPLimit()
    • CNAllocation3_PlantCNPAlloc()

Compared to the default call sequence, the source code in

  • CNAllocation1_PlantNPDemand() was moved up
  • CNAllocation2_ResolveNPLimit() remained in the same location
  • CNAllocation3_PlantCNPAlloc() was moved down

Did I interpret the code modifications correctly?

@wanggangsheng wanggangsheng deleted the wanggangsheng/lnd/bgc-interface branch January 16, 2017 20:48
@bishtgautam bishtgautam added ELM land model and removed Land labels Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ELM land model

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants