Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions modules/uncertainty/NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# PEcAn.uncertainty 1.9.0.9000

## Fixed

* `flux.uncertainty()`: removed a duplicate assignment of `E2` (the same line appeared twice in sequence with no effect except wasted computation).
* `flux.uncertainty()`: fixed a crash that occurred when all positive-flux magnitude bins contained `NA` error estimates. In that case the positive-slope model `mp` was never assigned, but the `else` branch for the negative-slope fallback unconditionally referenced `mp$coefficients[1]`, producing an "object 'mp' not found" error. The fallback now uses `else if (exists("mp", inherits = FALSE))` so it only runs when `mp` was actually fitted.

* Multiple bugfixes in `input.ens.gen()` handling of parent ids (#3783):
- No longer skips inputs that have a parent but no sampling method.
- Argument `parent_ids` now accepts integer vectors even if not wrapped in a list.
Expand Down
19 changes: 9 additions & 10 deletions modules/uncertainty/R/flux_uncertainty.R
Original file line number Diff line number Diff line change
Expand Up @@ -105,23 +105,22 @@ flux.uncertainty <- function(measurement, QC = 0, flags = TRUE, bin.num = 10,
## would be better to fit a two line model with a common intercept, but this
## is quicker to implement for the time being
E2 <- errBin - errBin[zero]
E2 <- errBin - errBin[zero]
intercept <- errBin[zero]
return.list <- list(mag = magBin,
err = errBin,
bias = biasBin,

return.list <- list(mag = magBin,
err = errBin,
bias = biasBin,
n = nBin,
intercept = intercept)
if(!all(is.na(E2[pos]))){

if (!all(is.na(E2[pos]))) {
mp <- stats::lm(E2[pos] ~ magBin[pos] - 1)
return.list$slopeP <- mp$coefficients[1]
}
if(!all(is.na(E2[neg]))){
}
if (!all(is.na(E2[neg]))) {
mn <- stats::lm(E2[neg] ~ magBin[neg] - 1)
return.list$slopeN <- mn$coefficients[1]
}else{
} else if (exists("mp", inherits = FALSE)) {
return.list$slopeN <- mp$coefficients[1]
}

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.

updated code is going to leave slopeN undefined if the new if is false. Doing so silently is going to cause issues downstream. Seems like there should be an additional else that sets slopeN to something, as well as a similar set of cases for the earlier slopeP. One option would be a sensible default (e.g., mean value from analyzing many site-years of data), another would be a NA or something like that. If we do set the parameter to NA, it would be good to check what existing PEcAn code calls this function to see what additional checks would be useful there.


Expand Down
Loading