Soilwat co2 effects #22

dschlaep · 2017-06-29T18:39:33Z

No description provided.

TODO: * Remove the additional start() parameters once that data is transferred to inputData * Test with rSFSW2 and rSOILWAT2 once changes have been done there (has succesfully worked with local old repos) * Confirm placement of global variable initializations

Includes: * Adding a structure SW_Carbon to avoid clogging up the global space * Moved R CO2 settings to S4 class * onGet/onSet functions to interact with S4 class and its respective object * Removal of additional parameters to SEXP start() SOILWAT2 remains fully functional by itself, and seems fully functional with rSOILWAT2's CO2 branch

…e_CO2_multipliers()"

This changes results massively. Linear equations were previously used for WUE and Biomass multipliers, which have been switched to power equations in this commit. Additionally, the correct source is being used for the PPMs now.

@biomass

* @biomass --> @CO2EFFECTS * Added yearly output * Added a total biomass column * Added a WUE multiplier column * Removed additional column * This slot, and this slot only, now reflects the proper future simulation years

Previously, when future scenarios were run, the original years would be used as output. For instance, a scenario in 2020-2050 would have outputs from 1980-2010. I've utilized the delta year that I pass in for carbon effects and added it to the model year ONLY IN THE OUTPUT FUNCTIONALITY. The rest of the model still needs the 'incorrect' years.

dschlaep · 2017-06-29T18:40:14Z

I created this pull request so that I can comment.

dschlaep · 2017-06-29T19:21:20Z

Commenting on last available commit 050dfbb:

SW_Carbon.h
- 'RCP' should be renamed to 'scenario' or similar and it must be a character string, e.g., "RCP85", "SRES A2", "SSP5-3.4-OS-Ext".
- The currently hard-coded value of 2010 to distinguish between past and future should be turned into an input a0.
- Why are the implemented differences past and future CO2-effects? They all come from the same continuous time-series input.
- It may make sense to have this inputted past/future year 'a0' the year where c->co2_multipliers[i][a0] = 1.0 per definition. Because then the input biomass would be the simulated biomass of that year a0.
SW_Control.c
- Yearly updates of CO2-effects to biomass: this is implemented by calling 'SW_VPD_init()' by function '_begin_year()'. The function 'SW_VPW_init()', however, includes calculations which are not dependent on the CO2-effects, i.e., the monthly interpolation of litter, pct_live, and lai_conv. They should be in a separate function that is indeed only called at the beginning. E.g., 'SW_VPD_init()' and 'SW_VPW_update()' which does the yearly updates.

@Zachary-Kramer

dschlaep · 2017-06-30T12:59:03Z

Commenting on last available commit 050dfbb:

SW_VegProd.h
- The currently implemented CO2-effects do not distinguish between plant functional types, particularly the scaling equations: c1 * ppm ^ c2 uses the same coefficient values for all types, i.e., the inputs co2_biomass_1, co2_biomass_2, co2_stomatal_1, co2_stomatal_2 are defined in struct SW_VEGPROD and not struct VegType
- I interpret the literature to suggest that species, and at least different plant types (e.g., C3 vs. C4 grasses; woody vs. herbaceous), may respond differently to increasing ambient CO2 concentrations (e.g., Gerosa et al. 2012; Lee et al. 2011; Poorter & Navas 2003; Poorter 1993; Temme et al. 2013). Even if we currently do not have sufficient data to estimate c1 and c2 for each type, I believe we should get the code ready to do that.

Literature:
1.Gerosa, G., Mereu, S., Finco, A. & Marzuoli, R. (2012). Stomatal Conductance Modeling to Estimate the Evapotranspiration of Natural and Agricultural Ecosystems. In: Evapotranspiration - Remote Sensing and Modeling (ed. Irmak, A.). InTech, Shanghai, China

2.Lee, T.D., Barrott, S.H. & Reich, P.B. (2011). Photosynthetic responses of 13 grassland species across 11 years of free-air CO2 enrichment is modest, consistent and independent of N supply. Global Change Biology, 17, 2893–2904

3.Poorter, H. (1993). Interspecific variation in the growth response of plants to an elevated ambient CO2 concentration. Vegetatio, 104–105, 77–97

4.Poorter, H. & Navas, M.-L. (2003). Plant growth and competition at elevated CO2: on winners, losers and functional groups. New Phytologist, 157, 175–198

5.Temme, A.A., Cornwell, W.K., Cornelissen, J.H.C. & Aerts, R. (2013). Meta-analysis reveals profound responses of plant traits to glacial CO2 levels. Ecol Evol, 3, 4525–4535
-> "The increase in biomass is about +45% for C3 species and +12% for C4 species at a 50% increase in CO2 concentration (Poorter and Navas 2003). The response of C4 species to increased CO2 is smaller than that of C3 species, probably because the carbon concentrating mechanism of C4 plants already concentrates CO2 around RuBisCO leaving less room for increased photosynthetic rate (Bowes 1993; Ghannoum et al. 2000). Overall, woody species showed a greater response to elevated CO2 than herbaceous species (Curtis and Wang 1998; Poorter and Navas 2003; Ainsworth and Long 2005; Lee et al. 2011)."

Zachary-Kramer · 2017-07-01T01:25:25Z

Caitlin's email response:

Hi Daniel,

Thanks for taking the time to look through the code and figures.

In regards to the implementation difference between past and future CO2 effects, I am uncertain. I agree with you that hypothetically the effects should be applied the same no matter the time period. Perhaps, Zach saw a reason for this when he was writing was the code. I have now included him in this e-mail chain and have addressed the issue to him on Git.

Yes, I think that CO2 effects vary by PFT. The equations in the model for both WUE and biomass come from the spreadsheet attached below. The equations are the best fit of the points established from a literature review, which encompass multiple PFTs. Based on the info in the spreadsheet, it seems there aren't enough points to establish a strong fit for each PFT though. John can answer this more precisely.

Thanks again!,
Caitlin

Zachary-Kramer · 2017-07-01T02:01:13Z

Hi Daniel,

I appreciate the feedback. I want to be clear that this branch is still a work in progress, is not ready to be merged, and still needs things done before a full code review.

'RCP' should be renamed to 'scenario' or similar and it must be a character string, e.g., "RCP85", "SRES A2", "SSP5-3.4-OS-Ext".

I agree, it should be designed this way to support more scenarios, though this specific support is not within our original scope, which is supporting RCP85. While I should have designed this better off the bat, if we have time constraints it will have to be delayed into a future enhancement.

The currently hard-coded value of 2010 to distinguish between past and future should be turned into an input a0.

Why are the implemented differences past and future CO2-effects? They all come from the same continuous time-series input.

It may make sense to have this inputted past/future year 'a0' the year where c->co2_multipliers[i][a0] = 1.0 per definition. Because then the input biomass would be the simulated biomass of that year a0.

For context:

The hard-coded value of 2010 is based off of our definitions of historical (<= 2010) and future (> 2010).
Caitlin requested that we be able to enable/disable the effects based on these definitions.
Changing this to custom input changes our definition of this functionality.
- For instance, it would no longer be a "Historical" flag in the experimental design, but rather something along the lines of "LessThan".

I'm not against this idea, I'm just providing context for why it is this way. Again, if we have time constraints this will have to become a future enhancement, as it is outside of the original scope.

It may make sense to have this inputted past/future year 'a0' the year where c->co2_multipliers[i][a0] = 1.0 per definition. Because then the input biomass would be the simulated biomass of that year a0.

I am not completely understanding your explanation. Why do we want the input biomass to be the simulated biomass of year a0? If you are suggesting that this be the base point, where biomass multipliers before it are <1 and after it are >1, that would change the equation, wouldn't it? Sorry, I'm just a bit confused.

Yearly updates of CO2-effects to biomass: this is implemented by calling 'SW_VPD_init()' by function '_begin_year()'. The function 'SW_VPW_init()', however, includes calculations which are not dependent on the CO2-effects, i.e., the monthly interpolation of litter, pct_live, and lai_conv. They should be in a separate function that is indeed only called at the beginning. E.g., 'SW_VPD_init()' and 'SW_VPW_update()' which does the yearly updates.

I agree. The current approach just has a very small performance effect and is tightly coupled, correct? In which case this should be a lower priority enhancement.

Apply biomass multiplier to tree's fraction live biomass, not total biomass. Note that the output column name still needs to be changed in rSFSW2/rSOILWAT2.

Disregard the description in the last commit, I forgot that SOILWAT2 holds the instance of the rSOILWAT2 output class

…e code Close #29: CO2 multipliers incorrect when scenario doesn't exist Close #34: Properly initialize the CO2 variables

# Conflicts: # makefile --> merged # makefile_n --> removed

Tabs to spaces, reducing reliance on hard-coded values, constructor, comments, change the stomatal term to water-usage efficiency, be more consistent on variable names

Zachary-Kramer · 2017-09-05T02:52:59Z

@dschlaep Could you do another iteration for this review? I have some recent changes, but nothing that significant

dschlaep · 2017-09-06T13:21:28Z

Sure, I will. Give me a few days. Thanks!

…

On Sep 4, 2017, at 22:53, Zachary Kramer ***@***.***> wrote: @dschlaep <https://github.com/dschlaep> Could you do another iteration for this review? I have some recent changes, but nothing that significant — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#22 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEAp2-xqOOkarlSdaBuDAsEDEKjjP6Dqks5sfLeMgaJpZM4OJtXC>.

dschlaep · 2017-09-13T21:20:44Z

Good job!

I opened four new issues (of which I consider only 2 high priority). Please consider them.

Please revert the testing folder to our old default values (plus enable both the CO2-effects).

Zachary-Kramer · 2017-09-13T22:16:48Z

Thanks for the review! I'm off work this week and next week because I'm moving to Germany, so I won't fix these issues until after then. I'll revert the testing folder as well.

dschlaep · 2017-11-30T00:48:49Z

[ RUN      ] CarbonTest.CO2multipliers
coeff1 = 0.000 / coeff2 = 0.000

in appveyor-ci and not travis-ci because the order of unit test execution differs.
On travis-ci: CarbonTest > VegTest
On appveyor-ci: VegTest > CarbonTest

- the order in which tests are executed is not fixed - each TEST() must carefully reset global states -- this is a pain because our old SOILWAT2 c code is rich in functions that manipulate global states

dschlaep · 2017-11-30T01:50:08Z

That did the trick! Unit tests have to carefully reset global states!

- `REvprintf()` returns void unlike `vfprintf()`; thus we cannot check for EOF

- function `LogError` should only be called with argument `logfp` and no longer with `stderr` or `stdout` directly (see commit 4e0cc0e)

- close DrylandEcology/rSOILWAT2#78 - close DrylandEcology/rSOILWAT2#73 - file 'testing/Input/outsetup_v31.in' * fully explain what output columns are * rename output file to 'vegetation' from 'co2_effects' - function '_collect_values()' did not call `SW_OUT_sum_today(eVPD)` - return value of function 'get_co2effects' was 'void' instead of 'static void' as every other 'get_*' function - function 'SW_OUT_flush' did not call `SW_OUT_sum_today(eVPD)` - function 'sumof_vpd' only ever added biomass values into output slots from 'Today' which is 1 -- the correct index is 'SW_Model.doy' - function 'average_for' precluded 'ObjType's other than 'eSWC' and 'eWTH' to perform averaging

- turn on all cover types

- replace 'nullptr' with 'NULL'

- addressing issue DrylandEcology/rSOILWAT2#81 - define 'OBJECTS' and 'SOURCES' for rSOILWAT2 compilation

Coefficients assume that monthly biomass inputs reflect values for conditions at 360 ppm CO2, i.e., Coeff1 * x^Coeff2 = 1 for x = 360 CO2 ppm

- addressing DrylandEcology/rSOILWAT2#81 and DrylandEcology/rSOILWAT2#66 - Makevars, SW_R_init.c, SW_R_lib.c, and SW_R_lib.h are not used by SOILWAT2 -- they are rSOILWAT2 files - remove rSOILWAT2 shared object target from makefile --> this commit requires that the removed files are added to rSOILWAT2/src/ and that the source code of SOILWAT2 is now a submodule in rSOILWAT2/src/SOILWAT2/ -- instead of rSOILWAT2/src/

…sage Also, i was a high number because the simstart year was 1979 and only data for 1980-2010 was provided, so the loop iterated i many times.

If the 'values' object doesn't contain the value of 'year', then the loop was not behaving properly and didn't stop until 'i' was extremely large. Fix: add condition that i remains less than maximum number of years. This and commit 48f3588 close DrylandEcology/rSOILWAT2#80

# Conflicts: # SW_Output.c # SW_Site.c # generic.c # makefile # testing/Input/siteparam.in # testing/Input/veg.in # testing/Input/years.in # testing/files_v31.in

# Conflicts: # SW_Output.c

siteparam.in still uses RCP85 by default

Zachary-Kramer added 8 commits March 23, 2017 13:27

Add support for compiling on UNIX systems

71bcd9f

Change "Retro" to "Historical", Change "SW_Carbon_Get()" to "calculat…

1a53bcf

…e_CO2_multipliers()"

Change multiplier equations and PPMs

2ff6863

This changes results massively. Linear equations were previously used for WUE and Biomass multipliers, which have been switched to power equations in this commit. Additionally, the correct source is being used for the PPMs now.

Changes to output

1de06cc

* @biomass --> @CO2EFFECTS * Added yearly output * Added a total biomass column * Added a WUE multiplier column * Removed additional column * This slot, and this slot only, now reflects the proper future simulation years

@CO2EFFECTS: Change yearly sum to yearly avg

b9e2f19

Zachary-Kramer mentioned this pull request Jul 1, 2017

Apply biomass effects in its own function #23

Closed

Zachary-Kramer added 10 commits June 30, 2017 20:01

Close #16

0649b9f

Apply biomass multiplier to tree's fraction live biomass, not total biomass. Note that the output column name still needs to be changed in rSFSW2/rSOILWAT2.

@CO2EFFECTS Column "TreeBiomass" --> "TreePctLive"

cfd7b66

Disregard the description in the last commit, I forgot that SOILWAT2 holds the instance of the rSOILWAT2 output class

Remove the historical and future differentiation

0259df9

Close #28: Add support for non-RCP scenarios

469652c

Remove compilation warnings

c17ce29

"# RCP" --> "# Scenario"

2f644d1

Add a constructor for the SW_CARBON structure, remove legacy defensiv…

0810780

…e code Close #29: CO2 multipliers incorrect when scenario doesn't exist Close #34: Properly initialize the CO2 variables

Close #24: Change @CO2EFFECTS output

9b075bb

Merge branch 'master' into soilwat_co2_effects

06ecd23

# Conflicts: # makefile --> merged # makefile_n --> removed

Close #15: Reformat CO2 code

9940e62

Tabs to spaces, reducing reliance on hard-coded values, constructor, comments, change the stomatal term to water-usage efficiency, be more consistent on variable names

Zachary-Kramer added this to the soilwat_co2_effects milestone Sep 5, 2017

dschlaep mentioned this pull request Sep 13, 2017

Allow CO2 coefficients to vary for each plant functional type #57

Closed

Fix stupid mistake in previous commit

0166500

Unit tests: properly reset global states

bf4826c

- the order in which tests are executed is not fixed - each TEST() must carefully reset global states -- this is a pain because our old SOILWAT2 c code is rich in functions that manipulate global states

dschlaep and others added 10 commits November 29, 2017 20:50

Remove debug setting

4175066

Improve 'LogError'

d787ce0

- `REvprintf()` returns void unlike `vfprintf()`; thus we cannot check for EOF

Fix sumof_vpd

fe029d6

- function `LogError` should only be called with argument `logfp` and no longer with `stderr` or `stdout` directly (see commit 4e0cc0e)

Update to inputs for testing simulation

c243ec3

- turn on all cover types

'nullptr' is c++11 which is not good for rSOILWAT2

cff992a

- replace 'nullptr' with 'NULL'

Removed duplicated declaration in 'SW_R_lib.ch'

69da0d9

Remove superfluous #ifndef #endif from 'SW_Main_lib.c'

f029b38

Attempt to fix rSOILWAT #81

bb34a97

- addressing issue DrylandEcology/rSOILWAT2#81 - define 'OBJECTS' and 'SOURCES' for rSOILWAT2 compilation

Update CO2 multiplier power equation coefficients (fix #59)

5387c0d

Zachary-Kramer force-pushed the soilwat_co2_effects branch from f790f94 to 5387c0d Compare December 1, 2017 14:26

dschlaep and others added 13 commits December 1, 2017 10:05

More detailed explanation in prodin comments for CO2 effects

613db62

Coefficients assume that monthly biomass inputs reflect values for conditions at 360 ppm CO2, i.e., Coeff1 * x^Coeff2 = 1 for x = 360 CO2 ppm

Fix fatal crash that occured when trying to send a specific error mes…

48f3588

…sage Also, i was a high number because the simstart year was 1979 and only data for 1980-2010 was provided, so the loop iterated i many times.

Merge branch 'master' into soilwat_co2_effects

21f1bbf

Add README to explain SOILWAT2 example inputs

4217f97

Merge branch 'master' into soilwat_co2_effects

188fc58

# Conflicts: # SW_Output.c # SW_Site.c # generic.c # makefile # testing/Input/siteparam.in # testing/Input/veg.in # testing/Input/years.in # testing/files_v31.in

Update gtest to reflect changed testing/files.in name

2d8944d

Add debug print statements to 'SW_F_read'

51d03e4

Merge branch 'master' into soilwat_co2_effects

8204258

# Conflicts: # SW_Output.c

Merge branch 'master' into soilwat_co2_effects

522ac34

Remove compilation warning: "unused variable"

61cca29

Close #96: Add 'Default' scenario to carbon.in

8eb69d2

siteparam.in still uses RCP85 by default

dschlaep merged commit 96ee5ed into master Dec 14, 2017

dschlaep deleted the soilwat_co2_effects branch December 14, 2017 18:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Soilwat co2 effects #22

Soilwat co2 effects #22

dschlaep commented Jun 29, 2017

dschlaep commented Jun 29, 2017

dschlaep commented Jun 29, 2017 •

edited by CaitlinA

Loading

dschlaep commented Jun 30, 2017 •

edited

Loading

Zachary-Kramer commented Jul 1, 2017

Zachary-Kramer commented Jul 1, 2017

Zachary-Kramer commented Sep 5, 2017

dschlaep commented Sep 6, 2017 via email

dschlaep commented Sep 13, 2017

Zachary-Kramer commented Sep 13, 2017

dschlaep commented Nov 30, 2017

dschlaep commented Nov 30, 2017

Soilwat co2 effects #22

Soilwat co2 effects #22

Conversation

dschlaep commented Jun 29, 2017

dschlaep commented Jun 29, 2017

dschlaep commented Jun 29, 2017 • edited by CaitlinA Loading

dschlaep commented Jun 30, 2017 • edited Loading

Zachary-Kramer commented Jul 1, 2017

Zachary-Kramer commented Jul 1, 2017

Zachary-Kramer commented Sep 5, 2017

dschlaep commented Sep 6, 2017 via email

dschlaep commented Sep 13, 2017

Zachary-Kramer commented Sep 13, 2017

dschlaep commented Nov 30, 2017

dschlaep commented Nov 30, 2017

dschlaep commented Jun 29, 2017 •

edited by CaitlinA

Loading

dschlaep commented Jun 30, 2017 •

edited

Loading