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

pixelGroup table going into the spinup event cannot have ages <3. Hard coded fix for now. Can't figure out why. #1

Open
cboisvenue opened this issue Oct 7, 2020 · 7 comments

Comments

@cboisvenue
Copy link
Contributor

cboisvenue commented Oct 7, 2020

from cboisvenue/spadesCBM#34

@cboisvenue
Copy link
Contributor Author

The update CBM_vol2Biomass_RIA.R has a new smoothing procedure that uses the Chapman-Richards fnct. This better-smoothing approach has solved this issue. Still need to update the main branch running the SK example to match the RIA approach.

@cboisvenue
Copy link
Contributor Author

This issue is not solve. All pixelGoups with ages 0 or 1 are set to 2 for the spinup to happen. They are currently reset to their actual values (sim$realAges) before the annual events (the actual simulations pots spinup).

@suz-estella
Copy link
Contributor

Adding a note that when this issue is resolved, CBM_dataPrep_SK will have to be updated to no longer alter the ages to be >= 3.

Moving some details on this issue here from CBM_dataPrep_SK (See PR PredictiveEcology/CBM_dataPrep_SK#14):

  ## TODO: problem with ages<=1
  #####################################################
  # in SK: to solve why growth curve id 52 (white birch, good # productivity) will not
  #run with ages= c(0,1,2) it gets stuck in the spinup need to set ages to 3. Tried ages==1, and
  #ages==2. Maybe because the first few years of growth are 0 ? (to check) it
  #does not grow and it does not fill-up the soil pools.
  #work for this problem for most curves for now: this is from SK runs
  #sim$level3DT[ages==0 & growth_curve_component_id==52,ages:=3]
  #sim$level3DT[ages <= 1, ages := 3]
  # in RIA: won't run for ages 0 or 1 with growth 0, had to change it to two
  # realAges are used to restore the correct ages in CBM_core postspinup event
  ######################################

@suz-estella
Copy link
Contributor

If this issue cannot be resolved easily: is it feasible to alter the ages OTF within the spinup event so that the realAges object doesn't need to be created in data prep? This would be good set up for the underlying algorithm to change because the data prep modules could be updated ASAP to no longer need to send altered age data to CBM_core (which could cause issues down the line if forgotten).

If all ages need to be set to >=3 then this could be easily implemented, however, the above comment from CBM_dataPrep_SK suggests that this might be handled differently in the RIA data prep.

@camillegiuliano
Copy link
Contributor

If all ages need to be set to >=3 then this could be easily implemented, however, the above comment from CBM_dataPrep_SK suggests that this might be handled differently in the RIA data prep.

I went to check and it seems like RIA has a similar problem and hard fix with a generally similar comment in its dataPrep. This is it here:

  ## TODO: problem with ages<=1
  ##################################################### # SK example: can't seem
  #to solve why growth curve id 52 (white birch, good # productivity) will not
  #run with ages= c(0,1,2) it gets stuck in the spinup. Tried ages==1, # and
  #ages==2. Maybe because the first few years of growth are 0 ? (to check) it #
  #does not grow and it does not fill-up the soil pools. # Notes: the GAMs are
  #fit on the cumulative curves of carbon/ha for three # pools. This is to make
  #sure the curves go through 0...but maybe it would # work better for GAMs to
  #be fit on the increments (?). # since all growth curves are for merchantible
  #timber (with diameter limits), it is acceptable to start all increments at
  #the level of year==3.
  #work for this problem for most curves for now: this is from SK runs
  #sim$level3DT[ages==0 & growth_curve_component_id==52,ages:=3]
 ######################################
  ##################### temp fix should

  sim$level3DT[ages <= 1, ages := 3]
  sim$level3DT[order(pixelGroup), ]

@suz-estella suz-estella moved this from Ready to discuss to Ready in @suz-estella's CBM_dataPrep_SK review Jan 6, 2025
@cboisvenue
Copy link
Contributor Author

@camillegiuliano @suz-estella - it just occurred to me that we have not tested this with the python-based spinup function. Can somebody do a run with actual ages and see if this is still an issue? You will know because the spinup doesn't complete, it gets tuck in a loop. Since our small raster has the problem growth curve id (52), we should be able to test this with our small raster.

@camillegiuliano
Copy link
Contributor

Just tried it commented out these lines in CBM_dataPrep_SK:

 # Create 'realAges' output object and set ages to be >= 3
  ## Temporary fix to CBM_core issue: https://github.com/PredictiveEcology/CBM_core/issues/1
  # sim$realAges <- level3DT[, ages]
  # level3DT[ages <= 3, ages := 3]

and this in CBM_core:

 ##TODO: confirm if this is still the case where CBM_vol2biomass won't
  ##translate <3 years old and we have to keep the "realAges" seperate for spinup.
  # sim$level3DT$ages <- sim$realAges

cleared my cache and ran, and It ran with no issues! Going to double check now to make sure this age change isn't elsewhere, but it seems the issue might not exist with the python spinup

@suz-estella suz-estella moved this from Ready to Move to tasks in @suz-estella's CBM_dataPrep_SK review Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants