Megan fates pr follow up

[mvdebolskiy]

Follow-up PR to refactor #99

[mvertens]

@mvdebolskiy - thanks for this. If testing has not been done on a PR it should be in draft status. I think that is the case with this PR. Could you please change that.

[mvdebolskiy]

@rosiealice

Few questions:

• Have you checked if fsun24 and fsun240 are accumulated properly when use_fates_sp or use_fates_nocomp are specified?

• In FatesPlantRespPhotosynthDrive you have:

                     if(hlm_use_nocomp)then
                       bc_out(s)%ci_pa(ifp) = internal_co2_z(1,currentpatch%nocomp_pft_label,1)
                    else
                       bc_out(s)%ci_pa(ifp) = -999
                    endif 

This means ci_pa will be -999 when hlm_use_sp is turned on and not nocomp?
So, in the VOCEmissionMod you acutally have no dependency on intercellular co2 concentration in SP.
Do I understand correctly that Fates runs photosynthesis routines in SP mode?
Also, you ci does not distinguish between sunlit in shaded, but from what I see in the Fates code, there is separation.

[rosiealice]

@rosiealice

Few questions

• Have you checked if fsun24 and fsun240 are accumulated properly when use_fates_sp or use_fates_nocomp are specified?
  Not -very- carefully, but I did check that they had values in them...

• In FatesPlantRespPhotosynthDrive you have:

                     if(hlm_use_nocomp)then
                       bc_out(s)%ci_pa(ifp) = internal_co2_z(1,currentpatch%nocomp_pft_label,1)
                    else
                       bc_out(s)%ci_pa(ifp) = -999
                    endif 

This means ci_pa will be -999 when hlm_use_sp is turned on and not nocomp? So, in the VOCEmissionMod you acutally have no dependency on intercellular co2 concentration in SP. Do I understand correctly that Fates runs photosynthesis routines in SP mode? Also, you ci does not distinguish between sunlit in shaded, but from what I see in the Fates code, there is separation.

So, SP is actually a subset of NOCOMP. It is impossible to use SP if NOCOMP is false. NOCOMP is really a flga that says 'the vegetation fraction remains the same and there is one PFT per patch' both of which are prerequisites for SP. This is set somewhere in the COMPSET logic that I don't understand, but if you specify a FATES-SP compset it will set NOCOMP as true. (I just double checked...)

[rosiealice]

Oh right, yes, good point...

[rosiealice]

On the sun and shade leaf leaf_ci,FATES distinguishes these, but it does so at the level of the vertical leaf layers (each of which has a shade and sun portion) so the averaging across them happens here
https://github.com/NorESMhub/fates/blob/f2ea09eaee1a028d9d72007a37774892ad1b397d/biogeophys/FatesPlantRespPhotosynthMod.F90#L1618

So the values for sun and shade ci coming from FATES are the same, but I have kept this distinction to minimize the intrusiveness of the changes (this happens several times in the FATES coupling, fo canopy conductance and for photosynthesis as well).

Also just to confirm, yes, SP mode runs photosynthesis, (and gas exhcnage, surface temperature, hydrology, snow, etc.) It just doesn't do anything with the carbon it assimilates and so LAI is prescribed from the satellite data (the point neing to isolate how well the gs exchange etc. performs when LAI is -not wrong-.

[rosiealice]

The main issue with passing th leaf_ci is that at the moment I have taken the top leaf ci to drive MEGAN.

I am going to do some testing (one these changes are intergrated as they will change answers for the better) to see whether different sorts of vertical averaging have a large or small impact on the output. Finding the full anopy average ci is quite complex as it involves scaling to the cohort level and then averaging the cohort values etc. These calculations are all int he most expensive part of the code and wold likely add a non-trivial cost. We discussed this in the FATES meeting and decided that it wasn't necessarily clear it was worth it, given the uncertainty in how the MEGAN interprets the Ci in the first place. Also, the observed isoprene-Ci relationship on which this is based was strong but observed over a massive range of Ci (150-1200ppm) (Fig 3 in https://www.researchgate.net/profile/Greg-Barron-Gafford/publication/228696525_Effect_of_CO2_concentration_and_vapour_pressure_deficit_on_isoprene_emission_from_leaves_of_Populus_deltoides_during_drought/links/0912f5089ac51a0d03000000/Effect-of-CO2-concentration-and-vapour-pressure-deficit-on-isoprene-emission-from-leaves-of-Populus-deltoides-during-drought.pdf )

[rosiealice]

I think these changes are good, for the record.

[mvdebolskiy]

@rosiealice I've fixed the issue with -999 being sent. I've put the assignment of bad values inside the CanopyFluxes, which was iterating with shrinking patch filter, while the assignment was for the whole array within the patch bounds.

Added CISUN/CISHA history fields and extra output to FatesColdMeganSatPhen test (so you can look up the inactive history variables).

I'll look into the output to see if intercellular CO2 pressure is getting much higher than the atmospheric CO2 (is this possible in IRL?).
Also, you are setting voc_pft_index to 1 within fates, which would rather be set to 0, since now the patches with 0th will be just ignored for VOC emissions. Your code worked because bareground always has itype(p) = 0 in clm, regardless of if it runs fates or not. Though the check for the voc_pftindex would be better, if accidentally there are bad values of voc_pftindex in the parameterfile.

[mvdebolskiy]

Relevant fates issues:

NGEET/fates#852
NGEET/fates#1222
NGEET/fates#1262
NGEET/fates#719