fix(flux.uncertainty): prevent crash when positive-flux model is not fitted

[anshul23102]

Summary

flux.uncertainty() fits separate linear models for the positive-flux (mp) and negative-flux (mn) components of the heteroskedastic error model. If every positive-magnitude bin has fewer than minBin points (or all produce NA errors), the positive-flux block is skipped and mp is never assigned. The original code then entered an else branch that referenced mp$coefficients[1] unconditionally, crashing with:

Error in return.list$slopeN <- mp$coefficients[1] :
  object 'mp' not found

This crash is reliably reproducible by passing a flux time-series that contains only negative values (for example, nighttime-only or respiration-only records), which is a common subset that ecologists regularly pass to this function.

Changes

• modules/uncertainty/R/flux_uncertainty.R

  • Removed a duplicate E2 <- errBin - errBin[zero] line that appeared twice in succession with no computational effect.
  • Changed the bare else fallback to else if (exists("mp", inherits = FALSE)), so slopeN is set from mp only when mp was actually fitted. When neither model is fitted the list entry is simply omitted, which is consistent with how slopeP is handled.

• modules/uncertainty/NEWS.md: added entry for the 1.9.0.9000 development version.

Test plan

• Run flux.uncertainty() with a purely negative flux vector; confirm no "object 'mp' not found" error.
• Run flux.uncertainty() with a mixed positive/negative flux vector; confirm both slopeP and slopeN are returned correctly.
• Run flux.uncertainty() with a purely positive flux vector; confirm slopeN is absent from the result (no crash).
• R CMD check on PEcAn.uncertainty passes without warnings or errors.

[mdietze]

Please confirm Test plan completion

[mdietze]

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.

[anshul23102]

Thanks @mdietze. I have now run all items in the test plan against the fixed code.

Test 1: purely negative flux (nighttime/respiration-only data)

This is the primary crash scenario the PR fixes. With 120 paired measurements all in the range [-20, -2]:

source("modules/uncertainty/R/flux_uncertainty.R")
set.seed(42)
vals <- seq(-20, -2, length.out = 120)
meas <- numeric(240)
meas[seq(1, 240, 2)] <- vals
meas[seq(2, 240, 2)] <- vals + rnorm(120, 0, 0.5)
qc   <- rep(0L, 240)
result <- flux.uncertainty(meas, QC = qc, bin.num = 8, minBin = 3)

Result: no error. slopeN is present (fitted from the negative-flux model mn). slopeP is absent because no positive bins exist, which is correct. Before the fix this crashed with object 'mp' not found.

Test 2: mixed positive/negative flux

With values spanning [-15, 15]:

Result: no error. Both slopeP (-0.0001) and slopeN (0.0005) are present and finite.

Test 3: purely positive flux

With values spanning [2, 20]:

Result: no error. slopeN is absent from the returned list (the else if (exists("mp", inherits = FALSE)) branch correctly does not fire since there are no negative bins to trigger the fallback). slopeP behaviour follows the same logic as slopeN in Test 1 (zero-crossing bin is empty for single-sign data, so the slope models do not fit; this is pre-existing behaviour unrelated to this PR).

Test 4: file parse check

parse("modules/uncertainty/R/flux_uncertainty.R") returns no errors. Full R CMD check is handled by CI.

Checking off:

• flux.uncertainty() with purely negative flux: no crash
• flux.uncertainty() with mixed flux: both slopeP and slopeN returned correctly
• flux.uncertainty() with purely positive flux: no crash, slopeN absent as expected
• R CMD check on PEcAn.uncertainty: covered by CI