NGEET / fates

repository for the Functionally Assembled Terrestrial Ecosystem Simulator (FATES)
Other
93 stars 90 forks source link

Remove all uses of patch%itype in CLM when FATES is on. #196

Closed rosiealice closed 7 years ago

rosiealice commented 7 years ago

VOCs SHOULD BE INCOMPATIBLE WITH ED

biogeochem/VOCEmissionMod.F90: if (patch%itype(p) > 0) then biogeochem/VOCEmissionMod.F90: epsilon = get_map_EF(patch%itype(p),g, vocemis_inst) biogeochem/VOCEmissionMod.F90: epsilon = meg_cmp%emis_factors(patch%itype(p)) biogeochem/VOCEmissionMod.F90: gamma_a = get_gamma_A(patch%itype(p), elai240(p),elai(p),class_num) biogeochem/VOCEmissionMod.F90: end if ! patch%itype(1:15 only)

METHANE SHOULD BE INCOMPATIBLE WITH ED

biogeochem/ch4Mod.F90: if (patch%itype(p) /= noveg) then biogeochem/ch4Mod.F90: if (wtcol(p) > 0._r8 .and. patch%itype(p) /= noveg) then biogeochem/ch4Mod.F90: integer :: itype ! temporary biogeochem/ch4Mod.F90: if (transpirationloss .and. patch%itype(p) /= noveg) then !allow tloss above WT ! .and. j > jwt(c)) then biogeochem/ch4Mod.F90: if (j > jwt(c) .and. t_soisno(c,j) > tfrz .and. patch%itype(p) /= noveg) then biogeochem/ch4Mod.F90: itype = patch%itype(p) biogeochem/ch4Mod.F90: if (itype == nc3_arctic_grass .or. pftcon%crop(itype) == 1 .or. & biogeochem/ch4Mod.F90: itype == nc3_nonarctic_grass .or. itype == nc4_grass) then

DO NOT USE ENTIRE ROUTINE

biogeochem/DryDepVelocity.F90: clmveg = patch%itype(pi)

DO NOT USE ENTIRE ROUTINE (This is potentially already done. Is within a use_cn in CLM_driver and use_cn is false if use_ed is true...)

biogeochem/NutrientCompetitionCLM45defaultMod.F90: ivt => patch%itype , & ! Input: [integer (:) ] patch vegetation type biogeochem/NutrientCompetitionCLM45defaultMod.F90: ivt => patch%itype , & ! Input: [integer (:) ] patch vegetation type

LAND USE SHOULD BE INCOMPATIBLE WITH ED

biogeochem/dynHarvestMod.F90: ivt => patch%itype , & ! Input: [integer (:)] pft vegetation type biogeochem/dynHarvestMod.F90: ivt => patch%itype , & ! Input: [integer (:) ] pft vegetation type

LUNA SHOULD BE INCOMPATIBLE WITH ED

biogeophys/LunaMod.F90: ft = patch%itype(p) biogeophys/LunaMod.F90: ft = patch%itype(p) biogeophys/LunaMod.F90: ft = patch%itype(p) biogeophys/LunaMod.F90: ft = patch%itype(p)

THIS NEEDS PROPERLY FIXING WITH A PATCH PROPERTY FOR DLEAF SENT FROM FATES

biogeophys/CanopyFluxesMod.F90: cf = 0.01_r8/(sqrt(uaf(p))*sqrt(dleaf(patch%itype(p))))

OZONE SHOULD BE INCOMPATIBLE WITH ED

biogeophys/OzoneMod.F90: tlai=tlai(p), tlai_old=tlai_old(p), pft_type=patch%itype(p), & biogeophys/OzoneMod.F90: tlai=tlai(p), tlai_old=tlai_old(p), pft_type=patch%itype(p), &

DO NOT USE ENTIRE ROUTINE. Might have to be careful with this one incase the root profiles show up somewhere unexpected. --

biogeophys/RootBiophysMod.F90: if (patch%itype(p) /= noveg) then biogeophys/RootBiophysMod.F90: if (patch%itype(p) /= noveg) then biogeophys/RootBiophysMod.F90: beta = pftcon%rootprof_beta(patch%itype(p),varindx) biogeophys/RootBiophysMod.F90: if (patch%itype(p) /= noveg) then

THIS NEEDS PROPERLY FIXING WITH A PATCH PROPERTY FOR z0m AND displa SENT FROM FATES

* FIX - this could be a problem - since there is no use_ed biogeophys/CanopyTemperatureMod.F90: z0m(p) = z0mr(patch%itype(p)) htop(p) biogeophys/CanopyTemperatureMod.F90: displa(p) = displar(patch%itype(p)) htop(p)

PHS SHOULD BE INCOMPATIBLE WITH ED

biogeophys/SoilMoistStressMod.F90: !smp_node = max(smpsc(patch%itype(p)), -sucsat(c,j)*s_node*(-bsw(c,j))) biogeophys/SoilMoistStressMod.F90: smp_node = max(smpsc(patch%itype(p)), smp_node) biogeophys/SoilMoistStressMod.F90: (smp_node - smpsc(patch%itype(p))) / (smpso(patch%itype(p)) - smpsc(patch%itype(p))), 1._r8) biogeophys/SoilMoistStressMod.F90: !smp_node_lf = max(smpsc(patch%itype(p)), -sucsat(c,j)(h2osoi_vol(c,j)/watsat(c,j))*(-bsw(c,j))) biogeophys/SoilMoistStressMod.F90: smp_node_lf = max(smpsc(patch%itype(p)), smp_node_lf) biogeophys/SoilMoistStressMod.F90: btran2(p) = btran2(p) +rootfr(p,j)min((smp_node_lf - smpsc(patch%itype(p))) / & biogeophys/SoilMoistStressMod.F90: (smpso(patch%itype(p))

  • smpsc(patch%itype(p))), 1._r8)

PHS SHOULD BE INCOMPATIBLE WITH ED -This usage is within a PHS-specific routine.

biogeophys/SoilWaterMovementMod.F90: ivt => patch%itype

THINK THIS SHOULD BE REPLICATED BY ED. ASK @ckoven TO CONFIRM

soilbiogeochem/SoilBiogeochemVerticalProfileMod.F90: if (patch%itype(p) /= noveg) then soilbiogeochem/SoilBiogeochemVerticalProfileMod.F90: write(iulog, *) 'p, itype(p), wtcol(p): ', p, patch%itype(p), patch%wtcol(p)

DYN LU SHOULD BE INCOMPATIBLE WITH ED. (ask Bill where to turn off)

dyn_subgrid/dynpftFileMod.F90: m = patch%itype(p) dyn_subgrid/dyncropFileMod.F90: m = patch%itype(p) dyn_subgrid/dynPatchStateUpdaterMod.F90: my_flux1_fraction = flux1_fraction_by_pft_type(patch%itype(p))

DEFINITION OF ITYPE

main/initGridCellsMod.F90: integer :: pitype ! patch itype main/GetGlobalValuesMod.F90: write(iulog,*)'pft type = ',patch%itype(ipft)

DEFINITION OF ITYPE

main/subgridWeightsMod.F90: ! the pct_pft_diagnostics referens to patch%itype(p) which is not used by ED main/subgridWeightsMod.F90: integer :: ptype ! patch itype main/subgridWeightsMod.F90: integer :: ptype_1indexing ! patch itype, translated into 1-indexing for the given landunit type main/subgridWeightsMod.F90: ptype = patch%itype(p)

DEFINITION OF ITYPE

main/subgridRestMod.F90: interpinic_flag='skip', readvar=readvar, data=patch%itype)

DEFINITION OF ITYPE

main/PatchType.F90: integer , pointer :: itype (:) ! patch vegetation main/PatchType.F90: ! currently the logic checking if patch%itype(p) is not equal to noveg main/PatchType.F90: allocate(this%itype (begp:endp)); this%itype (:) = ispval main/PatchType.F90: deallocate(this%itype )

DEFINITION OF ITYPE

main/initSubgridMod.F90: ! currently patch%itype is used in CanopyTemperatureMod to calculate main/initSubgridMod.F90: patch%itype(pi) = ptype main/histFileMod.F90: call ncd_io(varname='pfts1d_itype_veg', data=patch%itype , dim1name=namep, ncid=ncid, flag='write')

ckoven commented 7 years ago

@rosiealice do we assume currently that if use_ed is true, then use_cn is false?

rgknox commented 7 years ago

self assigning

I think main/controlMod.F90 is where module incompatibility is handled @ckoven. And yes, it will gracefully abort if both are true.

rgknox commented 7 years ago

Following Rosie's list, I will add new logic to controlMod.F90:

if(use_ed)then

     if(use_hydstress) fail    ! Is this PHS @rosiealice ?
     if(use_voc)  fail
     if(use_lch4) fail
end

still looking through this module...

@billsacks , could you comment on what flags relevant to dynamic LU and should be on/off with ed? or other?

ckoven commented 7 years ago

@rgknox great, thanks. given that, there ought to be no issue with the soilbiogeochem/SoilBiogeochemVerticalProfileMod.F90. that subroutine (SoilBiogeochemVerticalProfile) is called by CNDriverMod.F90 (which assumes use_cn==.true.), and in clm_driver.F90, (in a use_cn == .true. bracket).

the functionality of that block of code is duplicated in flux_into_litter_pools in EDPhysiologyMod.F90

billsacks commented 7 years ago

@rgknox - search for use_ed in the CLM source: src/dyn_subgrid/dynSubgridControlMod : check_namelist_consistency. At least, I think that's what you're asking about....

rgknox commented 7 years ago

thanks @billsacks . Let me learn more about what goes on in dyn_SubgridControlMod, and then I will retro-actively decide what I was asking about!

rgknox commented 7 years ago

I have a new branch working on cleaning some of these up. It is partial progress.

There are a few things going on. First, we need FATES to determine the leaf characteristic length (dleaf), the roughness length (z0m) and the displacement height (displa) for its patches. Currently, the model is estimating these values on the HLM side, using the itype associated with the pft number, which is kinda-nonsense from the FATES perspective. My plan is to get the new system to transfer meaningful values over from fates, but maintain b4b.

I would also like fates to set the patch%itype on patches under its juristiction to a force fail value, so it does not accidentally get used when its not supposed to. Although, without debug flags, gnu just chuggs along without complaint when I request variables using an index of -999 from patch%itype. Although I am pretty confident tests will detect this no-problemo, as we have plenty of bounds checking flags turned on.

https://github.com/NGEET/ed-clm/compare/master...rgknox:rgknox-clean-itypes

bandre-ucar commented 7 years ago

Force fail should probably be nan or spval for consistency.

rgknox commented 7 years ago

got it, I found ispval in clm_varcon which should do the trick

rgknox commented 7 years ago

I checked through your list @rosiealice , thanks, that was a very helpful guide. I should have a PR soon that addresses each of these issues. I also double checked dynSubgridControlMod and indeed there is a check to make sure use_ed is not true if the "do_transient_pfts" flag is true.

As part of the solution I added a new logical filter on the HLM side: patch%is_fates. This filter is true for all patches that are under fates jurisdiction. This was also suggested by the NCAR engineering team, to help filtering on how soil to root water fluxes are handled in hydraulics.

In cases where the HLM was using itypes in a process that FATES had an alternative implementation for, the HLM process was simply checked against the is_fates logical filter, and asked for a boundary condition from fates as an alternative if true.

In cases where FATES could provide no alternative implementation of a process and the process assumed that patches = pft, (such as methane, ozone, dry-deposition, luna, and voc'), a check on the namelist flags in controlMod.F90 forces the user to set these flags to false.

In some of the cases lists, such as the harvesting, it was not being called anyway, because it was being called via CN.

Compatibility with PHS will be covered in a later changeset.

@ckoven : the stuff in soilbiogeochem/SoilBiogeochemVerticalProfileMod.F90 is not called for FATES, but I think we still need to identify if we need something similar implemented in FATES. Could you help with that?

rosiealice commented 7 years ago

Just cleaning up my messages. This sounds like a very robust scheme to me. Should help a great deal for the more complicated interactions we discussed (crops, etc)

On 19 March 2017 at 22:43, Ryan Knox notifications@github.com wrote:

I checked through your list @rosiealice https://github.com/rosiealice , thanks, that was a very helpful guide. I should have a PR soon that addresses each of these issues. I also double checked dynSubgridControlMod and indeed there is a check to make sure use_ed is not true if the "do_transient_pfts" flag is true.

As part of the solution I added a new logical filter on the HLM side: patch%is_fates. This filter is true for all patches that are under fates jurisdiction. This was also suggested by the NCAR engineering team, to help filtering on how soil to root water fluxes are handled in hydraulics.

In cases where the HLM was using itypes in a process that FATES had an alternative implementation for, the HLM process was simply checked against the is_fates logical filter, and asked for a boundary condition from fates as an alternative if true.

In cases where FATES could provide no alternative implementation of a process and the process assumed that patches = pft, (such as methane, ozone, dry-deposition, luna, and voc'), a check on the namelist flags in controlMod.F90 forces the user to set these flags to false.

In some of the cases lists, such as the harvesting, it was not being called anyway, because it was being called via CN.

Compatibility with PHS will be covered in a later changeset.

@ckoven https://github.com/ckoven : the stuff in soilbiogeochem/ SoilBiogeochemVerticalProfileMod.F90 is not called for FATES, but I think we still need to identify if we need something similar implemented in FATES. Could you help with that?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NGEET/ed-clm/issues/196#issuecomment-287678024, or mute the thread https://github.com/notifications/unsubscribe-auth/AMWsQ826YsPXH1_ThzZkJ3OgiZQ2wttWks5rngPcgaJpZM4MYffF .

--

Dr Rosie A. Fisher

Staff Scientist Terrestrial Sciences Section Climate and Global Dynamics National Center for Atmospheric Research 1850 Table Mesa Drive Boulder, Colorado, 80305 USA. +1 303-497-1706

http://www.cgd.ucar.edu/staff/rfisher/