Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge feature_space into master #376

Merged
merged 30 commits into from
Aug 21, 2019
Merged

Merge feature_space into master #376

merged 30 commits into from
Aug 21, 2019

Conversation

kpalmqui
Copy link
Member

@kpalmqui kpalmqui commented Aug 8, 2019

I believe we are almost ready to merge feature_space into master. We need to carefully check all of the logic and commits on the branch. I have run simulations with this branch many times and no additional testing is needed to identify bugs or memory leaks.

kpalmqui and others added 26 commits December 4, 2018 21:51
…issue #197

- added the space parameter back to the resource partitioning code
- also adjust key parameters for big sagebrush for testing purposes
Changed the maximum biomass value for PHHO, based on PHHO biomass data collected by Lexi Smith.
- turned on BOGR as the default C4 grass species as this is one of the most common C4 grass species in big sagebrush ecosystems
-turned off SPCR
- set eind to be 20 as default (similiar to C3 grass species)
Because all cases of the switch statement did the exact same thing, the entire block has been replaced with `transp = SXW.transpTotal`
To rescale space I had to modify three files:
in ST_structs.h: added a new variable, baseline_min_res_req, to the rgroup struct. This variable records the space parameter from inputs, since we are going to change min_res_req every year.

in ST_params.c: assigned input "space" to baseline_min_res_req.

in ST_resgroups.c: rescaled space. Once an rgroup is done establishing we betermine is any species established. If they have not we add the group's min_res_req to the total unused space variable. Once all groups are done establishing we loop through the rgroups again and redistribute any unused space.
* In ST_resgroups.c: set min_res_req to 0 if no species establish in that group. This ensures that min_res_req for all RGroups always sums to 1.
* In sxw_resource: _roots_active_rel[group, layer, month] now reflects the groups roots relative to all groups. This ensures that for a given layer and month the sum of _roots_active_rel for all groups will be 1. The new implementation does not require the RGroup's vegtype, so I removed the variable t from transp_contribution_bygroup. Transpiration distribution is now implemented by the new algorithm discussed in the issue.
I didn't realize this, but our contiguous integration checks use a c standard < c99. Before c99 you were not allowed to declare variables inside for loops. This isn't a big deal, I just moved all declarations back to the top of the functions.
This commit:
-updates res_avail to include res_extra
-moves the calculation of g->pr to right before it is used and after res_extra is determined.
the variable `tgmod` was un-initialized/had outdated value if `s->temp_class == NoSeason`

- close #332
Use macro `GE` instead of `==` to check that the two float variables `ndv->res_avail` and `ndv->res_required` represent sufficient resources

- close #333

- this commit does not change the testing output on my local computer
Use macro `ZRO` instead of `== 0` to check whether the float variable `sum_size` is zero

- close #334

- this commit does not change the testing output on my local computer (this condition is never triggered in the test run)
…D_init`

- close #335

The `SOILWAT2` function `SW_VPD_init` is called by `_begin_year` which in turn is called by `SW_CTL_run_current_year`. That the function `_begin_year` calls `SW_VPD_init` is "new" since `SOILWAT2` incorporated CO2-effects (commit DrylandEcology/SOILWAT2@1755c28 merged into [v3.5.0](https://github.com/DrylandEcology/SOILWAT2/releases/tag/v3.5.0)).

Thus, the `STEPWAT2` function `_sxw_sw_setup` no longer needs to call `SW_VPD_init` because it will be called anyhow in due time by `SOILWAT2`.

- this commit does not change testing output
fix inconsistencies in units between documentation, inputs, and how the code uses these variables:
- inputs are in units of number per square meter (so that these inputs do not depend on plot size)
- internally, code converts these to number per plot (because that is the unit of the simulation)
- fix function `rgroup_Establish` so that input values of `space` do not need to sum to exactly 1 (based on activated resource groups), i.e., make inputs `space` and `on` in file `rgroup.in` independent from each other
--> calculate "used_space" instead of "unused_space" which missed values from de-activated resource groups and didn't adjust if the sum of space (`min_res_req`) was larger than 1

- I'm not sure what to do if `used_space = 0`: currently, the code sends a fatal signal (@kpalmqui - any suggestions?)
Copy link
Contributor

@chaukap chaukap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I posed two questions, but I assume that both of them will be easy to fix or require no change. Therefore, I approve.

ST_params.c Outdated Show resolved Hide resolved
ST_resgroups.c Show resolved Hide resolved
@chaukap chaukap added this to the feature_space milestone Aug 9, 2019
ST_resgroups.c Outdated Show resolved Hide resolved
ST_resgroups.c Outdated Show resolved Hide resolved
ST_structs.h Outdated Show resolved Hide resolved
sxw_resource.c Outdated Show resolved Hide resolved
sxw_soilwat.c Show resolved Hide resolved
Copy link
Member Author

@kpalmqui kpalmqui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chaukap a few minor suggestions here

Previously, the function `_sxw_sw_setup` called `SW_VPD_init` to setup SOILWAT2's vegetation data.

We no longer need to call `SW_VPD_init` since SOILWAT2 v3.5.0. SOILWAT2 calls `SW_VPD_init` at the beginning of each year (for dynamic CO2-effects).

Remove commented out code to address #376 (comment)
Chandler Haukap added 2 commits August 19, 2019 14:22
These changes are only regarding comments, mainly printf statements.
@kpalmqui
Copy link
Member Author

@chaukap I have @dschlaep 's approval to merge feature_space into master. I believe you approve as well, after your recent commits to address my comments?

@kpalmqui kpalmqui self-assigned this Aug 20, 2019
@chaukap
Copy link
Contributor

chaukap commented Aug 20, 2019

@kpalmqui I'm ready to merge! If you would like me to do it just give the word and I'll have feature_space in all development branches.

@kpalmqui
Copy link
Member Author

@chaukap if you could merge into master that would be fantastic! Thanks for all of your work on this.

@chaukap chaukap merged commit d075207 into master Aug 21, 2019
@kpalmqui kpalmqui deleted the feature_space branch August 24, 2019 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants