EDmodel / ED2

Ecosystem Demography Model
79 stars 112 forks source link

Variables in tower analysis files removed #76

Closed Viskari closed 9 years ago

Viskari commented 9 years ago

Tower output files, suffix -T-, used to have bunch of AVG_* variables which calculated outputs corresponding to the tower data, have been removed for the current git version.

This is causing issues for our PEcAn workflow, which tries to read those outputs, so we wanted to check if there was a reason these variables were removed from ED2.

mpaiao commented 9 years ago

@Viskari Most time-aggregated variables have been standardised for clarity in a commit about 2 years ago. The AVG_ variables that remained in the code are used for internal calculations only, all the others have been renamed according to type of time average and type of aggregation (ED hierarchical levels).

Prefix: FMEAN – "Fast" (sub-daily averages, dependent upon FRQFAST) DMEAN – Daily mean MMEAN – Monthly mean MMSQU – Monthly sum of squares QMEAN – Monthly mean diel (dependent upon FRQFAST) QMSQU – Sum of squares associated with QMEAN

Suffix: PY – Polygon level SI – Site level PA – Patch level CO – Cohort level

Viskari commented 9 years ago

I noticed that, but you had moved the FMEAN to the I- and S- analysis files, which are produced at a daily level. Is there a reason they were moved away from the T- analysis files?

-- Toni

On Mon, May 18, 2015 at 3:35 PM, Marcos Longo notifications@github.com wrote:

@Viskari https://github.com/Viskari Most time-aggregated variables have been standardised for clarity in a commit about 2 years ago. The AVG_ variables that remained in the code are used for internal calculations only, all the others have been renamed according to type of time average and type of aggregation (ED hierarchical levels).

Prefix: FMEAN – "Fast" (sub-daily averages, dependent upon FRQFAST) DMEAN – Daily mean MMEAN – Monthly mean MMSQU – Monthly sum of squares QMEAN – Monthly mean diel (dependent upon FRQFAST) QMSQU – Sum of squares associated with QMEAN

Suffix: PY – Polygon level SI – Site level PA – Patch level CO – Cohort level

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-103184509.

mpaiao commented 9 years ago

Most likely I just forgot to add the keys to the -T- files in the vtableedio? calls.

Viskari commented 9 years ago

Ok, if it is that simple do you think you can add them or should I do it?

I didn't want to start messing with the code before getting understanding what was happening, so I am glad to hear, well read, that it wasn't anything more than that.

-- Toni

On Mon, May 18, 2015 at 4:30 PM, Marcos Longo notifications@github.com wrote:

Most likely I just forgot to add the keys to the -T- files in the vtableedio? calls.

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-103200410.

mpaiao commented 9 years ago

Would you mind adding it? I've never used the -T- files, so I think it would be better to have someone who actually uses the files to try it. Unless I'm forgetting something else, I think it should work if you add the ":opti" key to vtableedio? calls for the variables you would like to write.

Viskari commented 9 years ago

Ok, I'll work on it tomorrow.

However, do you mind look at the current radiation term pull request I have in ED2 at the moment? It would make it easier for me to push and pull the changes through if my previous pull request had been handled before it.

-- Toni

On Mon, May 18, 2015 at 7:44 PM, Marcos Longo notifications@github.com wrote:

Would you mind adding it? I've never used the -T- files, so I think it would be better to have someone who actually uses the files to try it. Unless I'm forgetting something else, I think it should work if you add the ":opti" key to vtableedio? calls for the variables you would like to write.

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-103267833.

Viskari commented 9 years ago

Actually, forget my request. I am recreating my ED2 repository and upload the radiation function transfers after I've solved this current issue. That way there is no rush concerning them.

On Mon, May 18, 2015 at 8:19 PM, Toni Viskari ttviskar@bu.edu wrote:

Ok, I'll work on it tomorrow.

However, do you mind look at the current radiation term pull request I have in ED2 at the moment? It would make it easier for me to push and pull the changes through if my previous pull request had been handled before it.

-- Toni

On Mon, May 18, 2015 at 7:44 PM, Marcos Longo notifications@github.com wrote:

Would you mind adding it? I've never used the -T- files, so I think it would be better to have someone who actually uses the files to try it. Unless I'm forgetting something else, I think it should work if you add the ":opti" key to vtableedio? calls for the variables you would like to write.

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-103267833.

mpaiao commented 9 years ago

I checked your changes in radiation earlier today, and it looks fine. Once we migrate all non-derived parameters to xml, we may want to rethink the best place to initialise these parameters that depend on the xml-defined one, just to avoid assigning them every time step.

Viskari commented 9 years ago

I agree and I was trying to figure out how to do that best. I was thinking about setting an if statement that only does the calculation if the value is for example negative and then we basically set that initial value as negative or something similar.

On Mon, May 18, 2015 at 9:05 PM, Marcos Longo notifications@github.com wrote:

I checked your changes in radiation earlier today, and it looks fine. Once we migrate all non-derived parameters to xml, we may want to rethink the best place to initialise these parameters that depend on the xml-defined one, just to avoid assigning them every time step.

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-103286773.

mpaiao commented 9 years ago

In the past I would define a Boolean variable that is locally saved:

logical, save :: first_call = .true.
...
if (first_call) then
   first_call = .false.
   [assign values]
end if

@rgknox, I remember you removed similar "if" blocks in other parts of the code, do you think this would be a problem for shared memory?

Viskari commented 9 years ago

I've been going through the FMEAN variables and there are a few we need that I couldn't find listed, so I thought to ask if you have any suggestions on what to use instead. The variables in question are

AVG_BDEAD AVG_BALIVE AVG_HTROPH_RESP AVG_FSC AVG_STSC AVG_SSC

Also, I am not quite certain what is the difference between stsc and ssc, as I thought both refer to structural soil carbon?

mpaiao commented 9 years ago

AVG_HTROPH_RESP — FMEAN_RH_PY

Fast soil carbon (FSC), structural soil carbon (STSC) and slow soil carbon (SSC) do not change at sub-daily scale, so you could use the instantaneous value instead: AVG_FSC — FAST_SOIL_C_PY AVG_STSC — STRUCT_SOIL_C_PY AVG_SSC — SLOW_SOIL_C

BALIVE is updated daily and BDEAD is updated monthly, so instantaneous values are also fine. You may be able to use BALIVE_PY and BDEAD_PY, respectively, although these are arrays of dimensions (n_pft,n_dbh,npoly). The total biomass for each polygon is just the sum of all PFT/DBH elements.

Viskari commented 9 years ago

Okay, thanks. I feel really smart now as I kept skipping FMEAN_RH_PY thinking it was the relative humidity.

Viskari commented 9 years ago

I am still having difficulties getting the biomass values. If I add the opti key to BDEAD and BALIVE, I get the following error:

HDF5-DIAG: Error detected in HDF5 (1.8.14) MPI-process 0:

000: H5Dio.c line 266 in H5Dwrite(): file selection+offset not within extent

major: Dataspace
minor: Out of range

Variable name: BALIVE
Global dimension: 5 17568 0 0 0 0 Chunk dimension: 5 1 0 0 0 0 Chunk offset: 0 1439 0 0 0 0 Count: 1 1 0 0 0 0 Stride: 1 1 0 0 0 0 :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: :::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::: ::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::


                 !!! FATAL ERROR !!!

---> File:        h5_output.F90
---> Subroutine:  h5_output
---> Reason:      Could not write hyperslab into the dataset

ED execution halts (see previous error message)...

Thought to ask for suggestions?

And by the way, I truly appreciate all the help with this and am sorry about the hassle.

DanielNScott commented 9 years ago

You're talking about the patch level vars right? That's what it looks like from your description. Are you just appending the opti output flag to the key list in ed_state_vars.f90?

You'd get an error if so since the patch vars are vectors with n_cohort elements. Based on the error message it looks like that might be the issue.The tower output requires scalars if I'm not mistaken.

Also, just out of curiosity why not just read balive from daily files and bdead from monthly, given that they don't change below those timescales?

mpaiao commented 9 years ago

I never used the "opti" files, but if tower output requires scalars, then BALIVE_PY and BDEAD_PY won't work either because they have dimensions (n_pft,n_dbh,npolygons). In this case, we may need to add back scalar versions of these two variables, unless someone has another solution...

DanielNScott commented 9 years ago

In my opinion, including variables in output that's produced more frequently than they are updated is potentially confusing, adds unnecessary complexity, and may create an illusion of non-existent resolution. It also adds a weird constraint if we ever want ED to do any lazy operations, which granted is not likely to ever be an issue, but is a design consideration nonetheless.

Viskari commented 9 years ago

The issue from our PEcAn point of view is that we are reading at the moment all the variables from the same tower file, hence the need for also those values there. I'll check with our development team for other thoughts on the matter.

On Wed, May 20, 2015 at 3:18 PM, DanielNScott notifications@github.com wrote:

In my opinion, including variables in output that's produced more frequently than they are updated is potentially confusing, adds unnecessary complexity, and may create an illusion of non-existent resolution. It also adds a weird constraint if we ever want ED to do any lazy operations, which granted is not likely to ever be an issue, but is a design consideration nonetheless.

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-104002412.

DanielNScott commented 9 years ago

You could create a workaround for the time being in ED pretty easily, whereby if you pass a variable such as balive to the output it just applies the appropriate functional.

mdietze commented 9 years ago

The -T- files (i.e. :opti) are supposed to be POLYGON level variables, not patch

Viskari commented 9 years ago

After discussion within PEcAn, the biomass question could be avoided by instead focusing on above ground biomass and wood. Reading in the above ground biomass is simple enough, but is there any suggestions on how to approach the total above ground wood for the tower file and if it would be simpler than the biomasses?

mpaiao commented 9 years ago

I didn't even remember that cgrid%total_agb was still available. There is no equivalent to below ground biomass, but if you track every occurrence of total_agb in the code, you should be able to add similar variables for balive/bdead or specifically below ground biomass (broot + (1-agf_bs)*bdead).

I'm agnostic on whether these variables should be in tower files, but if they are included in these files, just avoid names like "FMEAN_AGB" because they wouldn't be time averages.

DanielNScott commented 9 years ago

I would think that the total above ground wood would be agf_bs *bdead, though I'm not quite familiar enough with the allometry stuff to know for sure.

mdietze commented 9 years ago

What happened to BDEADA and BDEADB? I'd submitted code at one point years ago that split the BDEAD explicitly into Above and Belowground components, but this doesn't seem to be in the mainline. By contrast, SAPWOODA and SAPWOODB do seem to still be in there (seems to be attributed to a commit Naomi did in 2012 for the grass code, but I think I sent in the code a good bit earlier than that). The above/below explicit distinction is necessary for the resprouting code to work (in which case one cannot assume that all trees are partitioning with the agf_bs ratio)

mpaiao commented 9 years ago

I've never seen BDEADA and BDEADB in the mainline. If you sent these variables along with the grass code, then Naomi probably merged it through Abby's branch. @aswann do you remember why BDEADA/BDEADB were not included?

Viskari commented 9 years ago

While still struggling with those BDEAD and above ground wood values, I had a couple of questions relating to other variables.

First concerning evaporation and transportation. For pecan we need evaporation and transpiration per ground area, but the FMEAN variable is those values are given for the leaf. However, when looking over ed_state_vars completely, there is no ground evap/transpiration, but leaf and wood transpirations. So would the per ground area values be the sum of leaf and wood values? And if such, could I address it basically adding a FMEAN for wood evaporation/transpiration and using the sum of those in our outut reading? Or is there some other value in ED2 I should be using here?

All polygon level snow variables, AVG_SNOWDEPTH/SNOWFRACLIQ/SNOWTEMP/SNOWMASS, are missing from the FMEAN values, but there doesn't seem to be any kind of snow values in the outputs. There are, however, surface water values for those, so is snow accounted for in the surface water values?

And finally we are missing soil_fracliq from the output on a FMEAN scale. If I wish to add that calculation, where should I add it to?

mpaiao commented 9 years ago

@Viskari All the terms you need for evaporation and transpiration are already in the output, aggregated to polygon level and in kg/m2_ground/s: FMEAN_TRANSP_PY – transpiration FMEAN_VAPOR_LC – evaporation of water intercepted by the leaf surface FMEAN_VAPOR_WC – evaporation of water intercepted by branches FMEAN_VAPOR_GC – ground evaporation Total evaporation is the sum of these three FMEANVAPOR?C terms.

When I compare ED fluxes with tower estimates of evapotranspiration, I normally use the negative of FMEAN_VAPOR_AC, which is the closest term to what the towers actually measure (unless your tower measurements also include a storage term for water).

The surface water terms also account for snow. They are no longer called snow because they refer to either snowpack or water puddles: FMEAN_SFCW_DEPTH_PY – Sfc. water depth [m] FMEAN_SFCW_MASS_PY – Sfc. water mass [kg/m2] FMEAN_SFCW_TEMP_PY – Sfc. water temperature [K] FMEAN_SFCW_FLIQ_PY – Liquid fraction [0-1]

Viskari commented 9 years ago

Okay, good wanted to check on the surface water terms. As for the evaporation and transpiration, I just wanted to check those, as the metadata defines FMEAN_TRANSP_PY as the leaf transpiration and missed FMEAN_VAPOR_GC.

Thanks again.

On Wed, May 27, 2015 at 2:03 PM, Marcos Longo notifications@github.com wrote:

@Viskari https://github.com/Viskari All the terms you need for evaporation and transpiration are already in the output, aggregated to polygon level and in kg/m2_ground/s: FMEAN_TRANSP_PY – transpiration FMEAN_VAPOR_LC – evaporation of water intercepted by the leaf surface FMEAN_VAPOR_WC – evaporation of water intercepted by branches FMEAN_VAPOR_GC – ground evaporation Total evaporation is the sum of these three FMEANVAPOR?C terms.

When I compare ED fluxes with tower estimates of evapotranspiration, I normally use the negative of FMEAN_VAPOR_AC, which is the closest term to what the towers actually measure (unless your tower measurements also include a storage term for water).

The surface water terms also account for snow. They are no longer called snow because they refer to either snowpack or water puddles: FMEAN_SFCW_DEPTH_PY – Sfc. water depth [m] FMEAN_SFCW_MASS_PY – Sfc. water mass [kg/m2] FMEAN_SFCW_TEMP_PY – Sfc. water temperature [K] FMEAN_SFCW_FLIQ_PY – Liquid fraction [0-1]

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-106015446.

mpaiao commented 9 years ago

Just one correction, I wrote FMEANVAPOR?C, but all these fluxes should be FMEANVAPOR?C_PY

Viskari commented 9 years ago

Noticed that, but thanks for the correction.

On Wed, May 27, 2015 at 10:10 PM, Marcos Longo notifications@github.com wrote:

Just one correction, I wrote FMEANVAPOR?C, but all these fluxes should be FMEANVAPOR?C_PY

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-106145425.

serbinsh commented 9 years ago

@Viskari,

Does this then mean you fully understand the output variables so we can clean up and reconcile the confusing vars related to evap/transpiration, etc?

Viskari commented 9 years ago

I think so? I hesitate to be more confident as I am aware that I might still be wrong about it.

On Thu, May 28, 2015 at 11:35 AM, Shawn P. Serbin notifications@github.com wrote:

Toni,

Does this then mean you fully understand the output variables so we can clean up and reconcile the confusing vars related to evap/transpiration, etc?

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-106417754.

aswann commented 9 years ago

Hi All,

I looked through my notes and found a bunch of references to splitting the sapwood into above and below ground components, but not bdead.

I can no longer recall why bdead was not included, however it might be worth noting that I was never able to get the original code Mike wrote to work in the mainline so I implemented the grass changes from scratch. If the bdead split was not needed for grasses but just for re-sprouting I may not have thought to include it.

Best, Abby

On Thu, May 28, 2015 at 10:04 AM, Viskari notifications@github.com wrote:

I think so? I hesitate to be more confident as I am aware that I might still be wrong about it.

On Thu, May 28, 2015 at 11:35 AM, Shawn P. Serbin < notifications@github.com> wrote:

Toni,

Does this then mean you fully understand the output variables so we can clean up and reconcile the confusing vars related to evap/transpiration, etc?

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-106417754.

— Reply to this email directly or view it on GitHub https://github.com/EDmodel/ED2/issues/76#issuecomment-106492562.