Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue in maintrailextract() #66

Open
sylvainschmitt opened this issue May 22, 2023 · 6 comments
Open

Issue in maintrailextract() #66

sylvainschmitt opened this issue May 22, 2023 · 6 comments

Comments

@sylvainschmitt
Copy link
Collaborator

@thomasgaquiere , what is the purpose off the following lines:

preMainTrails <- DTMExtended > -Inf
preMainTrails<- rasterToPolygons(preMainTrails, dissolve=T)

Using a dummy raster of 20m height, this provoke a bug in my last install because of multiple polygons:

dem <- raster::raster(mask, nrow = 200, ncol = 200)
values(dem) <- rep(20, length(values(dem)))
maintrailextract(dem)
# Erreur dans st_cast.sfc(st_geometry(x), to, group_or_split = do_split) : 
# use smaller steps for st_cast; first cast to MULTILINESTRING or POLYGON?

This is because the multiple polygons can't be casted into one linestring here:

MainTrails <- preMainTrails %>% st_as_sf() %>% st_cast(to = 'LINESTRING', warn= FALSE)
@sylvainschmitt
Copy link
Collaborator Author

And @VincyaneBadouard , why do you expect a layer field in MainTrails:

  if(is.null(MainTrails) | MainTrails$layer == 0 | is.null(MainTrails$geometry)){
    stop("The main trails could not be calculated. There is probably a problem with the inputs: 'topography'")}

This results in a bug if main trail as no layer attribute!

@sylvainschmitt
Copy link
Collaborator Author

A possible fix for the bug:

    MainTrails <- preMainTrails %>% st_as_sf() %>% st_buffer(0.01) %>% 
      st_union() %>%  st_sf() %>% 
      st_cast(to = 'LINESTRING', warn= FALSE,
              group_or_split = FALSE, do_split = FALSE)
    MainTrails$layer <- 1 # useless and the check should be removed later

@VincyaneBadouard
Copy link
Owner

And @VincyaneBadouard , why do you expect a layer field in MainTrails:

  if(is.null(MainTrails) | MainTrails$layer == 0 | is.null(MainTrails$geometry)){
    stop("The main trails could not be calculated. There is probably a problem with the inputs: 'topography'")}

This results in a bug if main trail as no layer attribute!

I'm not sure that it's me who write this part. If the examples always work withour this part, we can remove it.

@thomasgaquiere
Copy link
Collaborator

thomasgaquiere commented May 23, 2023

@thomasgaquiere , what is the purpose off the following lines:

preMainTrails <- DTMExtended > -Inf
preMainTrails<- rasterToPolygons(preMainTrails, dissolve=T)

Using a dummy raster of 20m height, this provoke a bug in my last install because of multiple polygons:

dem <- raster::raster(mask, nrow = 200, ncol = 200)
values(dem) <- rep(20, length(values(dem)))
maintrailextract(dem)
# Erreur dans st_cast.sfc(st_geometry(x), to, group_or_split = do_split) : 
# use smaller steps for st_cast; first cast to MULTILINESTRING or POLYGON?

This is because the multiple polygons can't be casted into one linestring here:

MainTrails <- preMainTrails %>% st_as_sf() %>% st_cast(to = 'LINESTRING', warn= FALSE)

@sylvainschmitt, I tried to reproduce your issue and found that when :

dem <- raster::raster(mask, nrow = 200, ncol = 200) # what is your 'mask' object by the way (data(PlotMask)?, a spatial object with extent(200,200)?)?
values(dem) <- rep(20, length(values(dem)))

The resolution of the dummy dem is not "1,1", so :
fact <- floor(advancedloggingparameters$SlopeDistance/res(topography)[1])

(at the beginning of LoggingLab::maintrailextract())

fact equals to 1 in your case (3 when res 1,1), raster::focal() cannot fill the (empty) boundaries of the extended DEM (fill.boundaries function) with matrix(1,fact,fact),

At best, MainTrails produced are therefore inside the plot, which is not the expected result of the maintrailextract() function,

moreover, when creating a dummy raster without specifying the crs (I guess your 'mask' object had a crs in UTM), raster assigns WGS84 by default.

Should we add a check on the resolution of the topography argument in maintrailextract?

@sylvainschmitt
Copy link
Collaborator Author

Indeed the issue came from the "1,1" resolution, weirdly 200x200 is giving something near but not equal.

@thomasgaquiere
Copy link
Collaborator

values(dem) <- rep(20, length(values(dem))) may also produce weird results.

In fact, with maintrailextract(), we 'extend' the DEM (which should be clipped by a mask of the plot, so raster values outside the plot are NAs(NAs of 2 types : 1. present before the extension if the plot is not a square or a rectangle and not facing north and 2. created by the extension), then we fill the empty boundaries, we transform the extended DEM to a polygon and then a linestring.

If you create a raster and fill every cell of it before passing it to maintrailextract(), produced maintrails may not be at the edge and outside of the plot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants