Skip to content

Pecan disturbance da - #4123

Open
DongchenZ wants to merge 3 commits into
PecanProject:developfrom
DongchenZ:pecan_disturbanceDA
Open

DongchenZ wants to merge 3 commits into
PecanProject:developfrom
DongchenZ:pecan_disturbanceDA

Conversation

@DongchenZ

Copy link
Copy Markdown
Contributor

Add the disturbance DA algorithm to the existing SDA analysis pipeline

Description

In this PR, I added three things:

  1. The scripts for executing the example DoD disturbance (wildfire) SDA workflow (data preparation, site selection, downscale ML, prescribing disturbance, etc.) -- /pecan/modules/assim.sequential/inst/anchor/disturbanceDA_scripts.
  2. The disturbance_DA function for incorporating disturbance dynamics into the DA workflow -- /pecan/modules/assim.sequential/R/disturbance_DA.R.
  3. The implementation of the newly added disturbance_DA function inside the SDA analysis function /pecan/modules/assim.sequential/R/Analysis_sda_block.R.

Motivation and Context

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • My name is in the list of CITATION.cff
  • I agree that PEcAn Project may distribute my contribution under any or all of
    • the same license as the existing code,
    • and/or the BSD 3-clause license.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@mdietze mdietze left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

modules/assim.sequential/inst/anchor/disturbanceDA_scripts would really benifit from a README explaining what the other scripts are and the order they need to be run in. For example, 5 different scripts (everything but downscale) seem to be associated with data prep. It's ambiguous which script actually runs the SDA. I'd also suggest a git mv to move the entire directory out of /inst/anchor and instead just have in /inst as it's a completely distinct analysis from the continental SDA.

library(lgarch)
library(parallel)
#read settings.
settings <- PEcAn.settings::read.settings("/projectnb/dietzelab/dongchen/SDA_disturbance/example_polygon/pecan.xml")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please add this pecan.xml to the PR

```{r}
library(randomForest)
basedir <- "/projectnb/dietzelab/dongchen/SDA_disturbance/example_polygon/downscale_dist_DA/"
rf.train.downscale <- function(dat.train, predictor) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

probably a future direction, but this seems like a place where we really need to converge on a set of tools for doing the downscaling, rather than reinventing (or copy-pasting) long functions from script-to-script. In the coming months I'd really like to see some harmonization across what you're doing, and what Julian, Yang, and David are doing.

CA <- array(NA, dim = c(n.comp, n.state, n.state))
wA <- array(NA, dim = c(n.comp))

# select undisturbed / disturbed forecast

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Future direction: you don't need to assign each ensemble member to disturbed or undisturbed, that just throws away 1/2 of your SIPNET runs. Instead just make a second ensemble that applies disturbance to a copy of every member of the first ensemble


# kalman update to compute analysis means, covariances, weights
n.obs <- length(Y)
pD <- c(0.9, 0.1) # prior probability of disturbance 0.1, default value which can be tweaked

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These are really bad defaults. Need to be able to pass in data-driven priors by pixel and year.

for (i in 1:n.obs) {
H.mat[i,H[i]] <- 1
}
for (d in 1:2) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Improve documentation. This looks like you're just applying a standard Kalman update to each of the two disturbance classes (disturbed and undisturbed). Also, it's not clear exactly what lik and wA are (they look like the likelihood and some sort of weight, but it's not documented)

if (as.logical(block$disturbance)) {
load(block$dist.prior.file) # load disturbance prior file.
# prescribe mu0 for the disturbance-related reductions in AGB and LAI.
mu0 <- c(att$agb.bio.reduce[as.numeric(block$site.ids)], att$lai.bio.reduce[as.numeric(block$site.ids)])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I can't find where in the code att is being defined

##' @importFrom dplyr %>%
##' @export

disturbance_DA <- function (X, Y, R, H, aqq, bqq, disturbance, mu0=rep(80, 80), V0=diag(rep(0.25, 2))) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Default for mu0 don't make sense as these parameters have to be between 0 and 1.

new.biomass <- tobit(mvtnorm::rmvnorm(1,mu0*old.biomass,tcrossprod(old.biomass,old.biomass)*V0)) ## draw disturbed leaf and stem
check.max <- which(new.biomass > old.biomass)
if(length(check.max)>0) new.biomass[check.max] = old.biomass[check.max] # constrain new.biomass[i] <= old.biomass[i]
residual = sum(old.biomass-new.biomass)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not an intuitive name for biomass lost to disturbance (e.g., for fire this is the combustion). Also, you need to make sure you're explicitly tracking removals as they are important model outputs.

if(length(check.max)>0) new.biomass[check.max] = old.biomass[check.max] # constrain new.biomass[i] <= old.biomass[i]
residual = sum(old.biomass-new.biomass)
x[bio.idxs] <- new.biomass # update state with disturbed biomass
x[4] <- x[4] + residual*alloc.soil # some of the biomass gets transported to the soil

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Interesting that you're passing in the vegetative pools via a variable but you're hard coding that x[4] has to be soils. This seems a bit dangerous. More broadly, it's not safe to assume that the same fraction of leaf and stem go to the soil. Better solution and more general solution(not this PR) would be a full transition matrix.

residual = sum(old.biomass-new.biomass)
x[bio.idxs] <- new.biomass # update state with disturbed biomass
x[4] <- x[4] + residual*alloc.soil # some of the biomass gets transported to the soil
removal <- residual*(1-alloc.soil)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

similar to earlier comment, losses are being calculated but then never saved/reported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants