noaa-oar-arl / canopy-app

Stand-alone/column canopy codes and parameterizations
MIT License
6 stars 7 forks source link

Leaf Age Response function for all Biogenic VOCs consistent with MEGAN #92

Closed quaz115 closed 1 year ago

quaz115 commented 1 year ago

See Issue #79 . @drnimbusrain @MaggieMarvin These are the changes I have added so far for the Leaf Age response function to work for all BVOCs.

Think we need more discussion on how to make it work with the time feature (as I have currently tried to make tdays, i.e. time period between Past and Current LAI (whichever time period we decide to use) to be dynamic and user-defined. This si the only component (LAI and timestep inputs) that would need more work.

@drnimbusrain feel free to point out any further changes I need to make before we can finalize this. I have made this a draft PR currently, but it is almost complete I think

Let me know if you have questions about specific changes

drnimbusrain commented 1 year ago

@quaz115 There are build errors in your codes, as well as formatting issues too. Please make sure you are following the "develop" practices in the README, using things like the pre-commit hooks to check indentation etc.

quaz115 commented 1 year ago

@zmoon Even though i made a new conda env which has pre-commit installed, seems like findent isn't working ?

pre-commit install --install-hooks
pre-commit installed at .git/hooks/pre-commit
(CanopyApp) |19:02:01|Quazi.Rasool@hfe03:[canopy-app-LeafAge]> pre-commit run --all-files
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
Format Fortran code using findent........................................Failed
- hook id: wfindent-pypi
- exit code: 1

/home/Quazi.Rasool/.cache/pre-commit/repowh0abga7/py_env-python3.9/bin/wfindent: findent not found, exiting

pyupgrade................................................................Passed
isort....................................................................Passed
black-jupyter............................................................Passed
flake8...................................................................Passed
nbstripout...............................................................Passed
drnimbusrain commented 1 year ago

@quaz115 I tested your repo/branch and using the pre-commit run --all-files on GMU Hopper system and it works OK, and finds wfindent:

fix end of files.........................................................Passed
check yaml...............................................................Passed
Format Fortran code using findent........................................Failed
- hook id: wfindent-pypi
- files were modified by this hook

/home/pcampbe8/.cache/pre-commit/repoevazfzjg/py_env-python3.10/bin/wfindent using: findent version 4.2.3
wfindent: indented files: 28

Of course here I am using a later version of python3.10 in my install of pre-commit, compared to your version of python 3.9 installed in your environment.

Maybe try installing pre-commit again, but with later version of python?

conda install -c conda-forge pre-commit python=3.10.5

drnimbusrain commented 1 year ago

Eventually, when merged this will close #79

quaz115 commented 1 year ago

@quaz115 I tested your repo/branch and using the pre-commit run --all-files on GMU Hopper system and it works OK, and finds wfindent:

fix end of files.........................................................Passed
check yaml...............................................................Passed
Format Fortran code using findent........................................Failed
- hook id: wfindent-pypi
- files were modified by this hook

/home/pcampbe8/.cache/pre-commit/repoevazfzjg/py_env-python3.10/bin/wfindent using: findent version 4.2.3
wfindent: indented files: 28

Of course here I am using a later version of python3.10 in my install of pre-commit, compared to your version of python 3.9 installed in your environment.

Maybe try installing pre-commit again, but with later version of python?

conda install -c conda-forge pre-commit python=3.10.5

Had to use 3.11 to make findent work, also resolving other build errros now

quaz115 commented 1 year ago

@drnimbusrain It passes all checks and compiles now, but will prefer to merge only after run(s) with this new functionality. Also, let me know when you have reviewed the code changes once and have free time to discuss them sometime early this week.

drnimbusrain commented 1 year ago

Yes, please let us review the code carefully, and it would be best to show tests and results here before merging.

I'd like it if Maggie cof help with that too

On Sat, Sep 16, 2023, 5:00 PM Quazi Ziaur Rasool @.***> wrote:

@drnimbusrain https://github.com/drnimbusrain It passes all checks and compiles now, but will prefer to merge only after run(s) with this new functionality. Also, let me know when you have reviewed the code changes once and have free time to discuss them sometime early this week.

— Reply to this email directly, view it on GitHub https://github.com/noaa-oar-arl/canopy-app/pull/92#issuecomment-1722315360, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGLFYNVHA7MUCTB2FLGAQL3X2YHPFANCNFSM6AAAAAA4Y6U4SY . You are receiving this because you were mentioned.Message ID: @.***>

drnimbusrain commented 1 year ago

@MaggieMarvin Would be great if you could help provide a close code review here, as well as run some tests with for Quazi's PR/branch to gauge if it is working as expected and the impacts.

Thanks!

quaz115 commented 1 year ago

@quaz115 Finished my initial review, and looks pretty good. My major comment is that you set the above canopy air temperature to the canopy resolved leaf temperature. I think this should be changed to use the more appropriate 1D/2D 2-meter temperature from model/obs inputs. This seems more relevant, and then the TABOVECANOPY variable does not need to be an array indexed on canopy model levels (modlays, ZK, etc.).

Once updated for this, @MaggieMarvin could review and test the impacts of leaf age factors.

Nice work!

@drnimbusrain @MaggieMarvin I have edited the code (compiles as well) based on Patrick's recommendation to pass TEMP2 (above 2-m air temp) for GAMMA_LEAFAGE.

drnimbusrain commented 1 year ago

Looks good Quazi, before approving, I'd like it if @MaggieMarvin do a test with the impacts of turning the leaf age factor on and off for isoprene.

Thanks!

quaz115 commented 1 year ago

Should add the new namelist option to the main readme. Also could be cool to have a 1-D example demonstrating the leaf age option in the examples notebook.

Incorporated @zmoon 's review suggestions in latest commit

drnimbusrain commented 1 year ago

@quaz115 I approve this now given latest changes, but still would like to see the tests/plots here showing that it works as expected, maybe, and then get any further input from @zmoon and @MaggieMarvin . Great work!

zmoon commented 1 year ago

tests/plots

@quaz115 this could be in the examples notebook if you are up to trying it

quaz115 commented 1 year ago

tests/plots

@quaz115 this could be in the examples notebook if you are up to trying it

@zmoon are these in the canopy-app folder? or in the python/examples.ipynb (didn't see any plotting code snippets there?)

zmoon commented 1 year ago

@quaz115 I was referring to python/examples.ipynb. There are plots there but you have to run the notebook to see them.

MaggieMarvin commented 1 year ago

@quaz115 I tested the code from your last commit and it compiles and runs fine whether leaf age is turned on or off, but it looks to me like the emissions in the output file are empty when leaf age is turned on (at least for the global gridded simulation). Emissions are as expected when leaf age is turned off. Should I have changed anything else in the settings or the input file to run the leaf age code?

quaz115 commented 1 year ago

@quaz115 I tested the code from your last commit and it compiles and runs fine whether leaf age is turned on or off, but it looks to me like the emissions in the output file are empty when leaf age is turned on (at least for the global gridded simulation). Emissions are as expected when leaf age is turned off. Should I have changed anything else in the settings or the input file to account for the new time dimension?

@MaggieMarvin I know what you are referring to, i tested with SE US domain so you testing for global case is complementary. I will paste the results below in a while for SE US, as i am seeing the effects as they should be when leafage_opt is turned on (=0) for different scenarios. To test the different scenarios i add this code in the canopy_calcs.F90 and recompile whenever testing different scenarios [I didn't add this temporary copde block in PR and just tested in my local repo, as we are going to integrate with timefeature i.e. for LAI inputs across time steps]:
**! TEmporary leafge test Conditionally set pastlai, currentlai, and tsteplai based on leafage_opt (NOTE: INDENT THIS CODE BLOCK AS THE OTHER CODE BLOCKS ARE ABOVE IT)

                            if (leafage_opt .eq. 0) then
                                ! Assign the current value of lairef to both PASTLAI and CURRENTLAI
                                pastlai = lairef
                                currentlai = 1.8_rk * lairef
                                ! Setting tsteplai to 0
                                tsteplai = 30.0_rk !7.0_rk !1.0_rk !0.0_rk  (in days)
                            end if

! ... user option to calculate in-canopy biogenic emissions (the addition is just above this code block for ifcanbio) if (ifcanbio) then

So, like in the case above currentlai > pastlai (you can change the factors say now current lai is 80% higher than past lai). In the current algorithm the time step b/w LAI inputs would matter for this case where Current LAI> past LAI (for other two cases = or current LAI < past LAI, tests for changing tstep won't matter as Fnew, Fold, Fmat, Fgro are all scalars in those cases) . Does this addresses your question?

quaz115 commented 1 year ago

@quaz115 I tested the code from your last commit and it compiles and runs fine whether leaf age is turned on or off, but it looks to me like the emissions in the output file are empty when leaf age is turned on (at least for the global gridded simulation). Emissions are as expected when leaf age is turned off. Should I have changed anything else in the settings or the input file to account for the new time dimension?

@MaggieMarvin I know what you are referring to, i tested with SE US domain so you testing for global case is complementary. I will paste the results below in a while for SE US, as i am seeing the effects as they should be when leafage_opt is turned on (=0) for different scenarios. To test the different scenarios i add this code in the canopy_calcs.F90 and recompile whenever testing different scenarios [I didn't add this temporary copde block in PR and just tested in my local repo, as we are going to integrate with timefeature i.e. for LAI inputs across time steps]: **! TEmporary leafge test Conditionally set pastlai, currentlai, and tsteplai based on leafage_opt (NOTE: INDENT THIS CODE BLOCK AS THE OTHER CODE BLOCKS ARE ABOVE IT)

                            if (leafage_opt .eq. 0) then
                                ! Assign the current value of lairef to both PASTLAI and CURRENTLAI
                                pastlai = lairef
                                currentlai = 1.8_rk * lairef
                                ! Setting tsteplai to 0
                                tsteplai = 30.0_rk !7.0_rk !1.0_rk !0.0_rk  (in days)
                            end if

! ... user option to calculate in-canopy biogenic emissions (the addition is just above this code block for ifcanbio) if (ifcanbio) then

So, like in the case above currentlai > pastlai (you can change the factors say now current lai is 80% higher than past lai). In the current algorithm the time step b/w LAI inputs would matter for this case where Current LAI> past LAI (for other two cases = or current LAI < past LAI, tests for changing tstep won't matter as Fnew, Fold, Fmat, Fgro are all scalars in those cases) . Does this addresses your question?

Note how, pastlai is assigned to lairef (the current lai input) and current lai can be scaled to it (say 0.5 i.e. 50% reduction in LAI or 1.5 i.e. 50% increase in LAI) for doing different tests

quaz115 commented 1 year ago

@MaggieMarvin @drnimbusrain : emi_isop (on scale of 10^-10 kg/m^2/s) "sum_mono" ((on scale of 10^-11 kg/m^2/s)) = emi_myrc + emi_sabi + emi_limo + emi_care + emi_ocim + emi_bpin + emi_apin + emi_mono
"sum_sesq" (on scale of * 10^-12 kg/m^2/s) = emi_farn + emi_cary + emi_sesq

1) LEAFAGE OFF (for SE US case):
image 2) LEAFAGE ON (Past LAI=Current LAI) : image 3) LEAFAGE ON (Past LAI > Current LAI: Time interval b/w LAI inputs matter in this case): a) Current LAI=0.8Past LAI: image b) Current LAI = 0.5Past LAI: image c) Current LAI =0.2Past LAI
image 4) LEAFAGE ON (Past LAI < Current LAI): A1) Current LAI = 1.2
Past LAI (Monthly time interval): image A2) Current LAI =1.2Past LAI (Weekly time interval): image B1) Current LAI =1.5Past LAI (Monthly time interval): image B2) Current LAI = 1.5Past LAI (Weekly time interval): image C1) Current LAI = 1.8Past LAI (Monthly time interval): image C2) Current LAI = 1.8*Past LAI (Weekly time interval): image

drnimbusrain commented 1 year ago

@quaz115 Thank you for your local tests, where seems like so far when the leafage factor is turned on in your tests, it decreases the biogenic emissions slightly.

As @MaggieMarvin pointed out though, some work is needed still because when testing your default [develop] branch, the because it is not fully functional with pastlai and currentlai.

Missing values are always produced in your branch:

pastlai=   6.9498915653698011E-310 currentlai   6.9498918392339459E-310

Thus the gammaleafage is always =1 right now in your branch.

I know you are going to modify this with the newly merged timestep functionality, so looking forward to seeing these updates in your updated branch.

Also, there is a warning to rectify in the canopy_utils code with the condition of the two floating point numbers:

canopy_utils_mod.F90:351:16:

  351 |         ELSEIF (LAIpast == LAIcurrent) THEN !If LAI remains same
      |                1
Warning: Equality comparison for REAL(8) at (1) [-Wcompare-reals]

This is a reminder that exact equality of floating point values is very rarely true, and this should be understood and fixed.

quaz115 commented 1 year ago

@drnimbusrain I also noticed the floating point equality Warning , wanted to add timestep feature also and then commit together after testing that along with the remaining fix. Will do it asap today

drnimbusrain commented 1 year ago

@drnimbusrain I also noticed the floating point equality Warning , wanted to add timestep feature also and then commit together after testing that along with the remaining fix. Will do it asap today

Great, yes, we always want to compile in debug mode and be as comprehensive as possible to fix all warnings up front. Thanks!

drnimbusrain commented 1 year ago

@quaz115 To accomodate the multiple time steps and using actual current vs. past lai in the same timestep now, I would suggest modifying canopy_app.F90 and canopy_calcs.F90 and add the time step index, nn, as an input.

Simlpy in canopy_app.F90 about line 88 you could change to:

!-------------------------------------------------------------------------------
! Main canopy model calculations.
!-------------------------------------------------------------------------------

        call canopy_calcs(nn)

In canopy_calcs.F90 you could add


SUBROUTINE canopy_calcs(nn)

...

INTEGER,     INTENT( IN )       :: nn         ! Input time step

...

REAL(rk)          :: pastlai                 !Past LAI [cm2/cm2]
REAL(rk), save :: currentlai            ! Current LAI [cm2/cm2] (saved from one timestep to the next)
REAL(rk)          :: tsteplai                !Number of days between the past and current LAI

...then after input variables are assigned for each timestep (e.g., lairef), but before the biogenic calls you could add:

if (nn .eq. 1) then
    currentlai=lairef
    pastlai = currentlai
else
    pastlai = currentlai
    currentlai = lairef
end if
tsteplai = time_intvl/86400.0_rk   !Convert timestep into days for biogenic leafage 
quaz115 commented 1 year ago

@drnimbusrain @MaggieMarvin @zmoon : One thing to notice is Monoterpene(s) see increase , while hemiterpenes (Isoprene and MBO) and SESQ would see slight decrease with leafage_opt turned ON (=0). This has to do with the F (old, new, amt, gro) factors being different between these BVOC classes.

Let me know if this PR is ready to be merged now.

1) LAI across different time steps seems to be ingested in the code correctly, see this printed diagnostic : LAIprintdiags.txt

NOTE: from top to bottom steps =1,2,3 (@drnimbusrain are these monthly LAI changes then do we need to edit the timestamp accordingly? currently it iterates as 2022-07-01 11:00:00 to 13:00:00). image

image

quaz115 commented 1 year ago

@drnimbusrain @MaggieMarvin @zmoon : One thing to notice is Monoterpene(s) see increase , while hemiterpenes (Isoprene and MBO) and SESQ would see slight decrease with leafage_opt turned ON (=0). This has to do with the F (old, new, amt, gro) factors being different between these BVOC classes.

Let me know if this PR is ready to be merged now.

  1. LAI across different time steps seems to be ingested in the code correctly, see this printed diagnostic : LAIprintdiags.txt

image

image

@MaggieMarvin @drnimbusrain We should also look more at spatial variations in LAI and how it correlate with changes in the BVOC emissions with leafage_opt (=0) ON

drnimbusrain commented 1 year ago

@quaz115 Thanks for the updates. For these default, short tests over SE U.S., it seems that at latest timestep the relative decrease in mean ISOP is only about ~ 5% with leafage on. These tests suggest that the leafage factor will not account for our larger ISOP predictions (by a factor of ~ 1.5 or 2) compared to MEGANv2 (HEMCO) or MEGAN3 (standalone) runs; however, larger global tests across more time periods is necessary (hopefully @MaggieMarvin can do these).

drnimbusrain commented 1 year ago

@zmoon or @MaggieMarvin Do you have any other pertinent questions/issues (other than running longer tests, which could be done later) with this PR before merging?

MaggieMarvin commented 1 year ago

@quaz115 @drnimbusrain Since we only have monthly LAI inputs at the moment, I'm not understanding how LAI is changing hourly and whether this is a good indicator of seasonal variations in leaf age as it stands right now. Wondering whether we should consider a different approach to the coarser LAI timestep for now - maybe updating the input files to include both the current and past month's LAI, and then reading those values directly? I'm concerned the leaf age time scale is not compatible with smaller timestep needs (hourly, daily, etc)

drnimbusrain commented 1 year ago

@quaz115 @drnimbusrain Since we only have monthly LAI inputs at the moment, I'm not understanding how LAI is changing hourly and whether this is a good indicator of seasonal variations in leaf age as it stands right now. Wondering whether we should consider a different approach to the coarser LAI timestep for now - maybe updating the input files to include both the current and past month's LAI, and then reading those values directly? I'm concerned the leaf age time scale is not compatible with smaller timestep needs (hourly, daily, etc)

The LAI does not change hourly, but is indeed a monthly step change across the default inputs (i.e., from June value in gfs.t12z.20220630.sfcf023.canopy.nc file to July value in gfs.t12z.20220701.sfcf000.canopy.nc. In other words, the current input files include both the current and past month's LAI, and that is why these three default files are chosen to allow for the monthly LAI change. If you wanted to run a both met and LAI monthly step change, you could download and use the files gfs.t12z.20220601.sfcf000.canopy.nc and gfs.t12z.20220701.sfcf000.canopy.nc, and set the NL time_intvl to 2592000.

However, the met data changes on an hourly timestep, and so this is indeed a limitation of the coarser LAI temporal resolution. The hourly timestep is converted to days in the code for the leafage. So, developing it this way allows canopy-app to easily be run over a step change of a true month, using both met and LAI values over this step change, or truly one could run with an hourly changing met and LAI if such data was feasible and available. Thus, I think it may be more flexible in this way right now.

Another aspect is that Wei-Ting is updating the canopy-app LAI right now to be a daily value, instead of monthly. Even then, a user could run a one month met and LAI changing time step to get the seasonal change.

Let me know what you think.

drnimbusrain commented 1 year ago

@quaz115 For Maggie's question: " I'm concerned the leaf age time scale is not compatible with smaller timestep needs (hourly, daily, etc)" Please just confirm this, but it should work with shorter timesteps, like hourly and especially daily right?

quaz115 commented 1 year ago

@quaz115 For Maggie's question: " I'm concerned the leaf age time scale is not compatible with smaller timestep needs (hourly, daily, etc)" Please just confirm this, but it should work with shorter timesteps, like hourly and especially daily right?

@drnimbusrain @MaggieMarvin yes, it should work once we have shorter time step LAI inputs as well. We can discuss this more in tomorrow’s meeting as well. It’s just currently we are working with monthly LAI inputs, and timestamp is dictated by the hourly timestep of inputs and as defined in namelist.

drnimbusrain commented 1 year ago

@quaz115 For Maggie's question: " I'm concerned the leaf age time scale is not compatible with smaller timestep needs (hourly, daily, etc)" Please just confirm this, but it should work with shorter timesteps, like hourly and especially daily right?

@drnimbusrain @MaggieMarvin yes, it should work once we have shorter time step LAI inputs as well. We can discuss this more in tomorrow’s meeting as well. It’s just currently we are working with monthly LAI inputs, and timestamp is dictated by the hourly timestep of inputs and as defined in namelist.

@quaz115 Thanks for the confirmation, but I think the question is does the adopted leafage theory from MEGAN in HEMCO hold for such short LAI timesteps?

quaz115 commented 1 year ago

@quaz115 For Maggie's question: " I'm concerned the leaf age time scale is not compatible with smaller timestep needs (hourly, daily, etc)" Please just confirm this, but it should work with shorter timesteps, like hourly and especially daily right?

@drnimbusrain @MaggieMarvin yes, it should work once we have shorter time step LAI inputs as well. We can discuss this more in tomorrow’s meeting as well. It’s just currently we are working with monthly LAI inputs, and timestamp is dictated by the hourly timestep of inputs and as defined in namelist.

@quaz115 Thanks for the confirmation, but I think the question is does the adopted leafage theory from MEGAN in HEMCO hold for such short LAI timesteps?

@drnimbusrain @MaggieMarvin Given, the MEGAN leafage theory considers the scale of bud-break to induce emissions and reach peak emissions (ti and tm variables) in order of days. So resolution of days/week should work. Not sure about hourly though (but then I assume LAI won't be changing hourly normally) and don't see the validity of going to an hourly scale for LAI, given the leaf age process is either on a daily or longer (weekly/Monthly) scale. Have you seen an example of MEGAN applied with LAI data with higher temporal resolution than the Monthly period? or say interpolation of Monthly LAI to a higher resolution? Testing with @angehung5 's high-res (daily) LAI product would be the best way to represent the Leaf age impact in my opinion. As prior applications have mostly utilized Monthly LAI (MODIS) and we would be going at the most representative resolution of the actual process (i.e. days)

quaz115 commented 1 year ago

@drnimbusrain @MaggieMarvin Also, been going through the LAI estimation literature, and the broader point on the need for high-frequency measurements of LAI is required on the daily scale to precisely capture rapid phenological events, such as leaf bud burst and senescence i.e. deterioration, which can occur over short time frames (daily) in temperate ecosystems. Even using PAR which has higher temporal heterogeneity (diurnal i.e. hourly), LAI estimates have been estimated on a daily scale to the high temporal resolution.

MaggieMarvin commented 1 year ago

@quaz115 @drnimbusrain Thanks for the explanation! I didn't realize these tests were done using consecutive model inputs from different months to capture the change in LAI. I do think that will work, but by my understanding there is an error in the leaf age calculation where the change in LAI (monthly) is not consistent with the timestep used (hourly).

It also appears that in this implementation, the first timestep always assumes that past LAI = current LAI, and that multiple input files with evolving LAI (currently, across different months) are needed for the leaf age calculation. I think these assumptions/requirements should be made super clear in the documentation, and perhaps might mean that turning leaf age on should remain an option at this point rather than the default setting. In the future, these issues could potentially be addressed with a spin-up/restart capability for canopy-app.

quaz115 commented 1 year ago

@quaz115 @drnimbusrain Thanks for the explanation! I didn't realize these tests were done using consecutive model inputs from different months to capture the change in LAI. I do think that will work, but by my understanding there is an error in the leaf age calculation where the change in LAI (monthly) is not consistent with the timestep used (hourly).

It also appears that in this implementation, the first timestep always assumes that past LAI = current LAI, and that multiple input files with evolving LAI (currently, across different months) are needed for the leaf age calculation. I think these assumptions/requirements should be made super clear in the documentation, and perhaps might mean that turning leaf age on should remain an option at this point rather than the default setting. In the future, these issues could potentially be addressed with a spin-up/restart capability for canopy-app.

Thanks @MaggieMarvin! Also in our discussion, @drnimbusrain pointed me to https://github.com/noaa-oar-arl/canopy-app/blob/develop/src/canopy_utils_mod.F90#L26 (interpolation scheme) i need to call to interpolate the current Monthly LAI inputs to the model time step (hourly), same will apply for daily LAI inputs to be interpolated to the model hourly time step as well. So, the results might change after these corrections

quaz115 commented 1 year ago

@drnimbusrain @MaggieMarvin @zmoon can you see the latest commit and check if the interpolation from Monthly LAI input to hourly model step is being done correctly? Want to confirm this first, before further analyzing what factors are influencing the emi_* changes due to leafage_opt ON. Figure 1 intrp This log file gives the before and after interpolation LAI (pastlai and currentlai): LAIbeforeandafterInterpolation.txt

angehung5 commented 1 year ago

@quaz115 Just discussed with @drnimbusrain, agreed it is critical to get LAI in higher resol for leaf age and other bio applications. Now I am working on daily LAI and having issues with the latitude regions. For monthly LAI, I extended it to full global coverage by using the ratio of monthly averages. We would get static values if we do the same thing to daily LAI. We may want to assume a daily variability (say based on surface type) for the high latitude regions.

Regarding hourly inputs, I am not sure if this is a good idea. VIIRS LAI provides 8-day average products so it should be ok to interpolate it into daily interval. But I am less comfortable to go hourly. The data quality could be worse and it is hard to evaluate. Based on the data I have, the daily difference is quite small. I think a better way would be assuming the hourly variability is minor and use constant daily value through out the day.

quaz115 commented 1 year ago

@quaz115 Just discussed with @drnimbusrain, agreed it is critical to get LAI in higher resol for leaf age and other bio applications. Now I am working on daily LAI and having issues with the latitude regions. For monthly LAI, I extended it to full global coverage by using the ratio of monthly averages. We would get static values if we do the same thing to daily LAI. We may want to assume a daily variability (say based on surface type) for the high latitude regions.

Regarding hourly inputs, I am not sure if this is a good idea. VIIRS LAI provides 8-day average products so it should be ok to interpolate it into daily interval. But I am less comfortable to go hourly. The data quality could be worse and it is hard to evaluate. Based on the data I have, the daily difference is quite small. I think a better way would be assuming the hourly variability is minor and use constant daily value through out the day.

@angehung5 I agree using a daily LAI input makes sense, as hourly variations in LAI would be hard to evaluate and generally unlikely as well (given daily differences are quite small).

drnimbusrain commented 1 year ago

@quaz115 Just discussed with @drnimbusrain, agreed it is critical to get LAI in higher resol for leaf age and other bio applications. Now I am working on daily LAI and having issues with the latitude regions. For monthly LAI, I extended it to full global coverage by using the ratio of monthly averages. We would get static values if we do the same thing to daily LAI. We may want to assume a daily variability (say based on surface type) for the high latitude regions. Regarding hourly inputs, I am not sure if this is a good idea. VIIRS LAI provides 8-day average products so it should be ok to interpolate it into daily interval. But I am less comfortable to go hourly. The data quality could be worse and it is hard to evaluate. Based on the data I have, the daily difference is quite small. I think a better way would be assuming the hourly variability is minor and use constant daily value through out the day.

@angehung5 I agree using a daily LAI input makes sense, as hourly variations in LAI would be hard to evaluate and generally unlikely as well (given daily differences are quite small).

Yes, I think much of the issues I raise in the review on LAI interpolation inconsistencies with lairef in canopy-app could be avoided if we just use the native LAI (and other canopy variables) input time step and avoid interpolating, i.e., tsteplai = tsteplaiinput/86400.0_rk. @quaz115 and @MaggieMarvin Wouldn't this and the new NL variable tsteplaiinput simply fix our initial concern of inconsistency and thus no need for LAI interpolation? We will lose the temporal variability of LAI if not interpolating to model time_intvl (e.g., hourly), but seems some concern that this is not needed and may incur error, as discussed with @angehung5 .

quaz115 commented 1 year ago

@drnimbusrain I have resolved the latest comments/review

drnimbusrain commented 1 year ago

@quaz115 How's it going on updating this PR by removing the interpolation steps for the LAI and just using the NL option lai_tstep?

quaz115 commented 1 year ago

@quaz115 How's it going on updating this PR by removing the interpolation steps for the LAI and just using the NL option lai_tstep?

@drnimbusrain was waiting on the daily LAI inputs to test with before committing to do that

quaz115 commented 1 year ago

@quaz115 How's it going on updating this PR by removing the interpolation steps for the LAI and just using the NL option lai_tstep?

@drnimbusrain was waiting on the daily LAI inputs to test with before committing to do that

or i can just go ahead and commit the changes to remove interpolation regardless?

drnimbusrain commented 1 year ago

@quaz115 How's it going on updating this PR by removing the interpolation steps for the LAI and just using the NL option lai_tstep?

@drnimbusrain was waiting on the daily LAI inputs to test with before committing to do that

or i can just go ahead and commit the changes to remove interpolation regardless?

Yes, good point. I would go forward now and make those changes and test with monthly LAI data in the current SE test files in this branch. Note: The daily LAI has been updated in the feature/pavd branch, and the PAVD changes will need time for further developments, testing, and review there. The global canopy inputs on AWS have daily LAI inputs now, but you would have to use the new script in the feature/pavd branch.

quaz115 commented 1 year ago

@angehung5 just passed on the daily LAI inputs on Hera: /scratch1/RDARCH/rda-arl-gpu/Wei-ting.Hung/Global_canopy ( dailys files in daily_viirs_lai folder) merged file: canopy_leaf_area_index.1km.daily.2020.nc @drnimbusrain think some modification in source code would be needed to handle this new LAI input with the default gfs input ?

quaz115 commented 1 year ago

@quaz115 How's it going on updating this PR by removing the interpolation steps for the LAI and just using the NL option lai_tstep?

@drnimbusrain was waiting on the daily LAI inputs to test with before committing to do that

or i can just go ahead and commit the changes to remove interpolation regardless?

Yes, good point. I would go forward now and make those changes and test with monthly LAI data in the current SE test files in this branch. Note: The daily LAI has been updated in the feature/pavd branch, and the PAVD changes will need time for further developments, testing, and review there. The global canopy inputs on AWS have daily LAI inputs now, but you would have to use the new script in the feature/pavd branch. @drnimbusrain @angehung5 is it just the new python script i need to use to have the merged daily LAI data with other default gfs input?

angehung5 commented 1 year ago

@quaz115 Yes, the updated script handles the new daily LAI and creates input file with the same format as previous. I don't think any modification in source code is needed. @drnimbusrain Please correct me if I am wrong.

drnimbusrain commented 1 year ago

he updated script handles the new daily LAI and creates i

@angehung5 Can you make another PR to @quaz115 's fork of develop, with your updated SE text/nc files(with same daily LAI and pavd updates as before) and updated python script for daily LAI merge from global AWS canopy/GFS files? This will facilitate @quaz115 testing of daily LAI in his develop branch for the leaf age PR.

quaz115 commented 1 year ago

@drnimbusrain Attaching the SE US output with @angehung5 's new daily LAI inputs showing difference b/w LEAFAGE OFF vs ON: Figure 5 (2) Figure 6 (2) Figure 7 (1)