Skip to content

Commit 8a77ccd

Browse files
authored
Merge pull request #3506 from divine7022/fix/soilgrids-extraction-errors
Fix "external pointer is not valid" error and improve output handling in soilgrids_soilC_extract()
2 parents 493fc73 + 2a07c41 commit 8a77ccd

3 files changed

Lines changed: 70 additions & 20 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ section for the next release.
4040
- PEcAn.SIPNET now accepts relative paths in its input XML (#3418). Previously all files referenced in the autogenerated `job.sh` needed to be specified as absolute paths.
4141
- R version 4.4 installs Python 3.12 which wants to leverage os managed packages instead, install python3-pika using apt.
4242
- Fixed a bugs and BADM now process both single-site and multi-site settings, detecting the input structure and processing each site independently to generate the correct number of ensemble members per site.
43+
- Fixed "external pointer is not valid" error and addressed key bugs in `soilgrids_soilC_extract()` function (#3506)
4344

4445
### Changed
4546

modules/data.land/NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
# PEcAn.data.land 1.8.2
77
- Removed unused parameter `machine` from put_veg_module()
8+
- Fixed "external pointer is not valid" error and addressed key bugs in `soilgrids_soilC_extract()` function (#3506)
89

910

1011
# PEcAn.data.land 1.8.1

modules/data.land/R/soilgrids_soc_extraction.R

Lines changed: 68 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,18 @@ soilgrids_soilC_extract <- function (site_info, outdir=NULL, verbose=TRUE) {
6666

6767
# prepare site info for extraction
6868
internal_site_info <- site_info[, c("site_id", "site_name", "lat", "lon")]
69+
70+
# Early return if no valid sites (after processing internal_site_info)
71+
if (nrow(internal_site_info) == 0) {
72+
if (verbose) {
73+
PEcAn.logger::logger.severe(
74+
"No valid sites remaining after NA check. ",
75+
"All sites had missing SoilGrids data for the first depth layer."
76+
)
77+
}
78+
return(NULL)
79+
}
80+
6981
#create a variable to store mean and quantile of organic carbon density (ocd) for each soil depth
7082
ocdquant <- matrix(NA, nrow = 6, ncol = length(internal_site_info$lon) * 4) #row represents soil depth, col represents mean, 5%, 50% and 95%-quantile of ocd for all sites
7183
lonlat <- cbind(internal_site_info$lon, internal_site_info$lat)
@@ -78,17 +90,27 @@ soilgrids_soilC_extract <- function (site_info, outdir=NULL, verbose=TRUE) {
7890
p <- terra::vect(lonlat, crs = "+proj=longlat +datum=WGS84") # Users need to provide lon/lat
7991
newcrs <- "+proj=igh +datum=WGS84 +no_defs +towgs84=0,0,0"
8092
p_reproj <- terra::project(p, newcrs) # Transform the point vector to data with Homolosine projection
93+
94+
# Extract coordinates for safe parallel transfer
95+
p_coords <- terra::crds(p_reproj)
96+
8197
data_tag <- c("_mean.vrt", "_Q0.05.vrt", "_Q0.5.vrt", "_Q0.95.vrt")
8298
name_tag <- expand.grid(depths, data_tag, stringsAsFactors = F)#find the combinations between data and depth tags.
8399
L <- split(as.data.frame(name_tag), seq(nrow(as.data.frame(name_tag))))#convert tags into lists.
84100

85101
get_layer <- function(l) {
86102
ocd_url <- paste0(base_data_url, l[[1]], l[[2]])
87-
ocd_map <- terra::extract(terra::rast(ocd_url), p_reproj)
88-
unlist(ocd_map[, -1]) / 10
103+
tryCatch({
104+
# Create temporary vector inside worker
105+
p_temp <- terra::vect(p_coords, crs = newcrs)
106+
vals <- terra::extract(terra::rast(ocd_url), p_temp)
107+
unlist(vals[, -1]) / 10
108+
}, error = function(e) {
109+
rep(NA, nrow(p_coords))
110+
})
89111
}
90112

91-
ocd_real <- try(furrr::future_map(L, get_layer, .progress = TRUE))
113+
ocd_real <- try(furrr::future_map(L, get_layer, .options = furrr::furrr_options(seed = TRUE), .progress = TRUE))
92114
if ("try-error" %in% class(ocd_real)) {
93115
ocd_real <- vector("list", length = length(L))
94116
pb <- utils::txtProgressBar(min = 0, max = length(L), style = 3)
@@ -116,6 +138,19 @@ soilgrids_soilC_extract <- function (site_info, outdir=NULL, verbose=TRUE) {
116138
ocd_df$Value<-as.numeric(ocd_df$Value)
117139
f1<-factor(ocd_df$Siteid,levels=unique(ocd_df$Siteid))
118140
f2<-factor(ocd_df$Depth,levels=unique(ocd_df$Depth))
141+
142+
# Skip if not enough quantiles (before gamma fitting)
143+
if (length(unique(ocd_df$Quantile)) < 2) {
144+
if (verbose) {
145+
PEcAn.logger::logger.warn(
146+
"Insufficient quantiles (", length(unique(ocd_df$Quantile)), ") ",
147+
"available for gamma distribution fitting at some sites. ",
148+
"Require at least 2 different quantiles to fit parameters."
149+
)
150+
}
151+
return(NULL)
152+
}
153+
119154
#split data by groups of sites and soil depth, while keeping the original order of each group
120155
dat <- split(ocd_df, list(f1, f2))
121156

@@ -132,22 +167,29 @@ soilgrids_soilC_extract <- function (site_info, outdir=NULL, verbose=TRUE) {
132167
}
133168

134169
fitQ <- function(x) {
135-
val = x$Value
136-
stat = as.character(x$Quantile)
137-
theta = c(10, 10)
138-
fit <-
139-
list(Gamma = stats::optim(theta, cgamma, val = val, stat = stat))
140-
SS <- sapply(fit, function(f) {
141-
f$value
142-
})
143-
par <- sapply(fit, function(f) {
144-
f$par
145-
})
146-
return(list(par = par, SS = SS))
170+
val <- x$Value
171+
stat <- as.character(x$Quantile)
172+
# Skip fitting if all values are NA or not numeric
173+
if (all(is.na(val)) || length(val) == 0) {
174+
return(list(par = c(NA, NA), SS = NA))
175+
}
176+
theta <- c(10, 10)
177+
fit <- tryCatch(
178+
stats::optim(theta, cgamma, val = val, stat = stat),
179+
error = function(e) NULL
180+
)
181+
if (is.null(fit)) {
182+
return(list(par = c(NA, NA), SS = NA))
183+
}
184+
return(list(par = fit$par, SS = fit$value))
147185
}
148186

149187
score <- suppressWarnings(lapply(dat, fitQ))
150188
bestPar <- sapply(score, function(f) { f$par })
189+
# Ensure bestPar is a 2-row matrix even when invalid sites are present
190+
if (is.null(dim(bestPar)) || nrow(bestPar) != 2) {
191+
bestPar <- matrix(bestPar, nrow = 2, byrow = TRUE)
192+
}
151193
mean <- bestPar[1,] / bestPar[2,]
152194
std <- sqrt(bestPar[1,] / bestPar[2,] ^ 2)
153195
mean_site <- matrix(mean, length(internal_site_info$lon), 6)
@@ -184,11 +226,17 @@ soilgrids_soilC_extract <- function (site_info, outdir=NULL, verbose=TRUE) {
184226
rownames(soilgrids_soilC_data) <- NULL
185227

186228
if (!is.null(outdir)) {
187-
PEcAn.logger::logger.info(paste0("Storing results in: ",file.path(outdir,"soilgrids_soilC_data.csv")))
188-
utils::write.csv(soilgrids_soilC_data,file=file.path(outdir,"soilgrids_soilC_data.csv"),row.names = FALSE)
189-
}
190-
else {
191-
PEcAn.logger::logger.error("No output directory found.")
229+
# Ensure the directory exists; create if not
230+
if (!dir.exists(outdir)) {
231+
dir.create(outdir, recursive = TRUE)
232+
PEcAn.logger::logger.info(paste0("Created output directory: ", outdir))
233+
}
234+
PEcAn.logger::logger.info(paste0("Storing results in: ", file.path(outdir, "soilgrids_soilC_data.csv")))
235+
utils::write.csv(soilgrids_soilC_data,
236+
file = file.path(outdir, "soilgrids_soilC_data.csv"),
237+
row.names = FALSE)
238+
} else {
239+
PEcAn.logger::logger.warn("No output directory found. Results are only returned to R environment.")
192240
}
193241
# return the results to the terminal as well
194242
return(soilgrids_soilC_data)

0 commit comments

Comments
 (0)