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

Unused space is not always correctly redistributed #368

Closed
dschlaep opened this issue Jul 30, 2019 · 6 comments
Closed

Unused space is not always correctly redistributed #368

dschlaep opened this issue Jul 30, 2019 · 6 comments
Assignees
Labels
bug in progress Task is currently in progress
Milestone

Comments

@dschlaep
Copy link
Member

Function rgroup_Establish is called at the start of each simulated year. After determining establishment, the function adjusts the space parameter among currently established resource groups so that it sums to 1. The parameter space is a user input via the file rgroup.in and is used in the function _transp_contribution_by_group to distribute total resources (from SOILWAT2) among currently established resource groups. The function _transp_contribution_by_group relies that space sums to 1 -- otherwise resources are annihilated or created ex nihilo.

If the space input values of activated (on = 1) resource groups sums to 1; then the current implementation works correctly. If the input sum is not 1, then the current implementation fails: examples based on commit 43f180f on the branch "feature_space" and default values in "testing.sagebrush.master/Stepwat_Inputs/" except for changes in space and on as below. The first set of values for the year 1 and 300 of simulation runs that start with used_space = ... are from my suggested fix; the second set of values that start with unused_space = ... are from the current implementation:

sum of input space = 1: space = 0.01, but space of a.cool.grass = 0.91 and turned on

Year 1: used_space = 0.940000
sagebrush before: 0.010000 -- sagebrush after: 0.010638
a.cool.forb before: 0.010000 -- a.cool.forb after: 0.010638
a.warm.forb before: 0.010000 -- a.warm.forb after: 0.010638
a.cool.grass before: 0.910000 -- a.cool.grass after: 0.968085
Sum of space after adjustment = 1.000000

Year 1: unused_space = 0.060000
sagebrush before: 0.010000 --sagebrush after: 0.010638
a.cool.forb before: 0.010000 --a.cool.forb after: 0.010638
a.warm.forb before: 0.010000 --a.warm.forb after: 0.010638
a.cool.grass before: 0.910000 --a.cool.grass after: 0.968085
Sum of space after adjustment = 1.000000

...

Year 300: used_space = 0.970000
sagebrush before: 0.010000 -- sagebrush after: 0.010309
a.cool.forb before: 0.010000 -- a.cool.forb after: 0.010309
a.warm.forb before: 0.010000 -- a.warm.forb after: 0.010309
a.cool.grass before: 0.910000 -- a.cool.grass after: 0.938144
p.cool.grass before: 0.010000 -- p.cool.grass after: 0.010309
shrub before: 0.010000 -- shrub after: 0.010309
succulents before: 0.010000 -- succulents after: 0.010309
Sum of space after adjustment = 1.000000

Year 300: unused_space = 0.030000
sagebrush before: 0.010000 --sagebrush after: 0.010309
a.cool.forb before: 0.010000 --a.cool.forb after: 0.010309
a.warm.forb before: 0.010000 --a.warm.forb after: 0.010309
a.cool.grass before: 0.910000 --a.cool.grass after: 0.938144
p.cool.grass before: 0.010000 --p.cool.grass after: 0.010309
shrub before: 0.010000 --shrub after: 0.010309
succulents before: 0.010000 --succulents after: 0.010309
Sum of space after adjustment = 1.000000

sum of input space < 1: space = 0.01, but space of a.cool.grass = 0.91 and turned off

Year 1: used_space = 0.030000
sagebrush before: 0.010000 -- sagebrush after: 0.333333
a.cool.forb before: 0.010000 -- a.cool.forb after: 0.333333
a.warm.forb before: 0.010000 -- a.warm.forb after: 0.333333
Sum of space after adjustment = 1.000000

Year 1: unused_space = 0.060000
sagebrush before: 0.010000 --sagebrush after: 0.010638
a.cool.forb before: 0.010000 --a.cool.forb after: 0.010638
a.warm.forb before: 0.010000 --a.warm.forb after: 0.010638
Sum of space after adjustment = 0.031915

--> failed to sum to 1

...

Year 300: used_space = 0.060000
sagebrush before: 0.010000 -- sagebrush after: 0.166667
a.cool.forb before: 0.010000 -- a.cool.forb after: 0.166667
a.warm.forb before: 0.010000 -- a.warm.forb after: 0.166667
p.cool.grass before: 0.010000 -- p.cool.grass after: 0.166667
shrub before: 0.010000 -- shrub after: 0.166667
succulents before: 0.010000 -- succulents after: 0.166667
Sum of space after adjustment = 1.000000

Year 300: unused_space = 0.030000
sagebrush before: 0.010000 --sagebrush after: 0.010309
a.cool.forb before: 0.010000 --a.cool.forb after: 0.010309
a.warm.forb before: 0.010000 --a.warm.forb after: 0.010309
p.cool.grass before: 0.010000 --p.cool.grass after: 0.010309
shrub before: 0.010000 --shrub after: 0.010309
succulents before: 0.010000 --succulents after: 0.010309
Sum of space after adjustment = 0.061856

--> failed to sum to 1

sum of input space > 1: space = 0.01, but space of a.cool.grass and p.cool.grass = 0.91 and turned on

Year 1: used_space = 0.940000
sagebrush before: 0.010000 -- sagebrush after: 0.010638
a.cool.forb before: 0.010000 -- a.cool.forb after: 0.010638
a.warm.forb before: 0.010000 -- a.warm.forb after: 0.010638
a.cool.grass before: 0.910000 -- a.cool.grass after: 0.968085
Sum of space after adjustment = 1.000000

Year 1: unused_space = 0.960000
sagebrush before: 0.010000 --sagebrush after: 0.250000
a.cool.forb before: 0.010000 --a.cool.forb after: 0.250000
a.warm.forb before: 0.010000 --a.warm.forb after: 0.250000
a.cool.grass before: 0.910000 --a.cool.grass after: 22.749989
Sum of space after adjustment = 23.499989

--> failed to sum to 1

...

Year 300: used_space = 1.870000
sagebrush before: 0.010000 -- sagebrush after: 0.005348
a.cool.forb before: 0.010000 -- a.cool.forb after: 0.005348
a.warm.forb before: 0.010000 -- a.warm.forb after: 0.005348
a.cool.grass before: 0.910000 -- a.cool.grass after: 0.486631
p.cool.grass before: 0.910000 -- p.cool.grass after: 0.486631
shrub before: 0.010000 -- shrub after: 0.005348
succulents before: 0.010000 -- succulents after: 0.005348
Sum of space after adjustment = 1.000000

Year 300: unused_space = 0.030000
sagebrush before: 0.010000 --sagebrush after: 0.010309
a.cool.forb before: 0.010000 --a.cool.forb after: 0.010309
a.warm.forb before: 0.010000 --a.warm.forb after: 0.010309
a.cool.grass before: 0.910000 --a.cool.grass after: 0.938144
p.cool.grass before: 0.910000 --p.cool.grass after: 0.938144
shrub before: 0.010000 --shrub after: 0.010309
succulents before: 0.010000 --succulents after: 0.010309
Sum of space after adjustment = 1.927835

--> failed to sum to 1

@dschlaep dschlaep added bug in progress Task is currently in progress labels Jul 30, 2019
@dschlaep dschlaep added this to the feature_space milestone Jul 30, 2019
@dschlaep dschlaep self-assigned this Jul 30, 2019
dschlaep added a commit that referenced this issue Jul 30, 2019
- 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?)
@kpalmqui
Copy link
Member

@dschlaep thank you for documenting this carefully and correcting the code so this implementation works if space values do not sum to 1. @chaukap take a look at Daniel's commit and explanation - just wanted you to be aware of this commit.

With regard to used_space = 0, it seems this condition could occur if no rgroups are established in the current year (the way the new code is now set up). This seems unlikely to occur, but could if you are only simulating a few rgroups.

I then checked the outcome of no established plants (used_space=0 and g->min_res_req = 0 for every rgroup) in the resource partitioning code and everything looks like it should work as expected.

When use_space=0 and g->min_res_req=0,

use_by_group[g] += (RealF) (_roots_active_rel[Iglp(g, l, p)] * transp[Ilp(l, p)]); SHOULD BE 0
sumUsedByGroup += use_by_group[g]; SHOULD BE 0

And the following should never be hit:

ForEachGroup(g){
      if(use_by_group[g] != 0){ //Prevents NaN errors
        //Proportion of total traspiration that this group would recieve without accounting for space
        proportion_total_resources = use_by_group[g] / sumUsedByGroup;
        //Average of the two proportions. This equally weights proportion_total_resources and min_res_req.
        average_proportion = (proportion_total_resources + RGroup[g]->min_res_req) / 2;
        //Recalculate this group's resources.
        use_by_group[g] = sumUsedByGroup * average_proportion;
      }
    }

So it seems that Logerror is not needed? We should try and test extensively with just a few rgroups turned on, low pestab values, and low eind to see if we can trigger this condition and assess if everything is working as expected when use_by_group[g] =0??

This also makes me feel that we ought to re-evaluate the whole code base and identify if there are places where we could skip functions/modules if no individuals are established....

@chaukap @dschlaep what do you think?

@dschlaep
Copy link
Member Author

dschlaep commented Aug 2, 2019

@kpalmqui I agree that the LogError is not helpful; however, we do need to catch those situations where used_space is zero because we are dividing by used_space (line 700). I came across a normal run yesterday where used_space was zero for one year. Since zero means that no plant is established, it should not matter what value min_res_req has because it will never be used. Maybe we should replace the LogError (line 685) with g->min_res_req = 0.; and then have the if (!EQ(used_space, 1.0)) {... in the else-clause of the ZRO(used_space) check?

@kpalmqui
Copy link
Member

kpalmqui commented Aug 2, 2019

@dschlaep but shouldn't the following prevent the division by used_space when it is 0? Line 698:

if (g->est_count > 0) {

@dschlaep
Copy link
Member Author

dschlaep commented Aug 7, 2019 via email

@kpalmqui
Copy link
Member

kpalmqui commented Aug 8, 2019

@dschlaep agreed. I think we are good to go and I will remove the logerror. thanks!

kpalmqui added a commit that referenced this issue Aug 8, 2019
@kpalmqui
Copy link
Member

kpalmqui commented Aug 8, 2019

Issue resolved by above commits and by a3874f4

@kpalmqui kpalmqui closed this as completed Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug in progress Task is currently in progress
Projects
None yet
Development

No branches or pull requests

2 participants