femiguez / apsimx

R package for APSIM-X
https://femiguez.github.io/apsimx-docs/
45 stars 19 forks source link

edit_apsim_replace_soil_profile() gives error when checking for soilwat #147

Closed LenLon closed 5 months ago

LenLon commented 5 months ago

edit_apsim_replace_soil_profile() consistently gives the same error when trying to use it with soil profiles I downloaded from ISRIC using the get_isric_soil_profile() function. The soils themselves (stored as objects of class soil profile in a list or an .Rdata file) should be fine, I can use the same ones to replace the soil profile in an .apsimx file.

Here's the call, with the resulting output and error:

> apsimx::edit_apsim_replace_soil_profile(src.dir = paste0(wd_apsim, "\\Sim"),
+                                         file = "Maize_SudanoSahelian_AEZ.apsim",
+                                         soil.profile = soils[[1]], 
+                                         overwrite = TRUE, verbose = TRUE)
Edited .//Soil/Water/Thickness 
Edited parameter Thickness 
New values  50 100 150 300 400 1000 
Created  ./Maize_SudanoSahelian_AEZ-tmp.apsim 
Edited .//Soil/Water/SAT 
Edited parameter SAT 
New values  0.4049198 0.4078957 0.41563 0.4174553 0.418605 0.4149351 
Created  ./Maize_SudanoSahelian_AEZ-tmp.apsim 
(...)
Edited .//Soil/Water/AirDry 
Edited parameter AirDry 
New values  0.08599427 0.1785128 0.1967388 0.2002356 0.2031675 0.1978454 
Created  ./Maize_SudanoSahelian_AEZ-tmp.apsim 
Error in apsimx::edit_apsim_replace_soil_profile(src.dir = paste0(wd_apsim,  : 
  object should be of class 'soilwat_parms'

When I manually set the soilwat argument (is that mandatory now?), the function fails somewhere in edit_apsim():

> apsimx::edit_apsim_replace_soil_profile(src.dir = paste0(wd_apsim, "\\Sim"),
+                                         file = "Maize_SudanoSahelian_AEZ.apsim",
+                                         soil.profile = soils[[1]], 
+                                         soilwat = soils[[1]]$soilwat,
+                                         overwrite = TRUE, verbose = TRUE)
Edited .//Soil/Water/Thickness 
Edited parameter Thickness 
New values  50 100 150 300 400 1000 
Created  ./Maize_SudanoSahelian_AEZ-tmp.apsim 
(...)
Edited .//Soil/Water/AirDry 
Edited parameter AirDry 
New values  0.08599427 0.1785128 0.1967388 0.2002356 0.2031675 0.1978454 
Created  ./Maize_SudanoSahelian_AEZ-tmp.apsim 
Error in edit_apsim(file = new.file.path, src.dir = ".", wrt.dir = ".",  : 
  value vector of incorrect length

The same call used to work some months ago when I last used it to dynamically replace soil profiles in a loop, but now it doesn't - I've only observed this error after the recent update.
But there is a similar error with an older version of edit_apsim_replace_soil_profile():

Error in !missing(soilwat) || !is.na(soil.profile$soilwat) : 
  'length = 18' in coercion to 'logical(1)'

I've attached my .apsim file and the soil profiles .Rdata file.

Apsim_Maize_soil.zip

femiguez commented 5 months ago

@LenLon Thanks again for testing the package and the feedback. The RData file you sent me appears to be corrupted - at least I could not load it. Even though there is a new version of 'apsimx' in CRAN (2.6.2) there are some errors related to this there still. (I will need to submit an update). The current version in github has addressed these issues. Try this code:

library(apsimx)
packageVersion("apsimx") ## It should be 2.6.3
sp1 <- get_isric_soil_profile(c(-93, 42), fix = TRUE)
edit_apsim_replace_soil_profile("Maize_SudanoSahelian_AEZ.apsim",
                                soil.profile = sp1)

It is not my intention to have the initialwater as mandatory, but these changes are helping move things in the right direction I hope.

LenLon commented 5 months ago

Sorry, I meant to attach this one:

ISRIC_soilGrids_list_complete.zip

Interestingly, opening the file using R (just clicking on it) fails for me to, but importing it in RStudio using readRDS() works.

My current version comes out to 2.6.1 - though I did use the devtools way of updating it just yesterday?

femiguez commented 5 months ago

@LenLon To get the latest version use 'remotes::install_github("femiguez/apsimx"). I did not run the simulations (I do not have the weather data), but this works for me using 2.6.3

soils <- readRDS("ISRIC_soilGrids_list_complete")

fixed.soils <- vector("list", length = length(soils))

for(i in seq_along(soils)){
  fixed.soils[[i]] <- apsimx:::fix_apsimx_soil_profile(soils[[i]])  
}

## Looking at one soil
plot(fixed.soils[[1]], property = "water")
plot(fixed.soils[[1]], property = "Carbon")

## Editing the template simulation and creating separate ones
for(i in seq_along(soils)){
  edit_apsim_replace_soil_profile("Maize_SudanoSahelian_AEZ.apsim",
                                  edit.tag = paste0("_", i),
                                  soil.profile = fixed.soils[[i]],
                                  verbose = FALSE)
}
LenLon commented 5 months ago

Thanks, with another devtools call I now have version 2.6.3 :-)

Unfortunately, fix_apsimx_soil_profile() is giving me an error, and I can't find the function on Github to check where it's really coming from:


> soils <- readRDS(filename)
> fixed.soils <- vector("list", length = length(soils))
> for(i in seq_along(soils)){
+   fixed.soils[[i]] <- apsimx:::fix_apsimx_soil_profile(soils[[i]])  
+ }
Error in if (!is.na(x$initialwater)) { : argument is of length zero```
femiguez commented 5 months ago

I suppose 'filename' is different from what you shared with me. Here is the file you sent me after I 'fixed' the soils.

ISRIC_soilGrids_list_complete_fixed.rds.zip

LenLon commented 5 months ago

Nah, it is the same file:

filename <- paste(wd_apsim, "\\Soil\\ISRIC_soilGrids_list_complete.RData", sep = "")

Weird that it works for you, because looking at the function code if (!is.na(x$initialwater)) { works as expected: my soil data have no $initialwater, so the output of !is.na() is logical(0). I get the same error with your fixed version as well!

But your fixed version works for replacing the soil profiles in my sim, so thank you very much already! Now I'm wondering what was wrong with them in the first place..

femiguez commented 5 months ago

It could be that it is my fault because I have not yet pushed some changes to the 'fix' function, so you do not have the latest one. I just attempted to push the latest changes.

femiguez commented 5 months ago

I'm closing this. Please submit a new issue if needed. Also the SoilGrids rest api has been down for several days, so I haven't been able to test this further. Once it is up again, I will make several changes and improvements to 'get_isric_soil_profile'.