PredictiveEcology / LandR

Landscape Ecosystem Modelling in R
https://landr.predictiveecology.org/
GNU General Public License v3.0
17 stars 7 forks source link

.createCohortData not doing what it says it does? #37

Open CeresBarros opened 4 years ago

CeresBarros commented 4 years ago

.createCohortData is supposed to (among other things) message(blue("Set B to 0 where cover > 0 and age = 0, because B is least quality dataset")). However this doesn't seem to be enforced in the code. Both B (biomass) and age are indeed being corrected (to 0) when cover == 0. However, having cover > 0 and age == 0 doesn't seem to be assessed, nor to influence/correct B to 0.

eliotmcintire commented 4 years ago

We need a table of all-way combinations. Make sure all cases are covered as desired.

On Fri, Feb 14, 2020, 12:09 PM Ceres Barros notifications@github.com wrote:

.createCohortData is supposed to (among other things) message(blue("Set B to 0 where cover > 0 and age = 0, because B is least quality dataset")). However this doesn't seem to be enforced in the code. Both B (biomass) and age are indeed being corrected (to 0) when cover == 0. However, having cover > 0 and age == 0 doesn't seem to be assessed, nor to influence/correct B to 0.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/PredictiveEcology/LandR/issues/37?email_source=notifications&email_token=AAIMVW47WSOKMT4UXWDDVRDRC327VA5CNFSM4KVO6XF2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4INVLCVQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIMVW6C5OBL3NNTSAG3STTRC327VANCNFSM4KVO6XFQ .

CeresBarros commented 4 years ago

Just noticed that the described behaviour is enforced at the end of the makeAndCleanInitialCohortData call (after the .createCohortData call). However, I suggest that it is "copied" to .createCohortData, for consistency with the message and the other checks/fixes that .createCohortData makes.

I also wonder if it makes sense to leave cover > 0 when age == 0 (and B is forced to 0). Thoughts?

CeresBarros commented 4 years ago

see https://github.com/PredictiveEcology/LandR/pull/38

achubaty commented 2 months ago

has this been resolved?

CeresBarros commented 2 months ago

I don't think so