nlm_planargradient bug when nrow != ncol

[GatesDupont]

On my machine, when simulating a planar gradient landscape using nlm_planargradient() , if nrow != ncol, I get a landscape where the values are shuffled, thereby producing an incorrect result:

# When nrow == ncol, it works perfectly
l = nlm_planargradient(ncol=10, nrow = 10)
show_landscape(l)

# When nrow != ncol, the values are shuffled
l2 = nlm_planargradient(ncol=10, nrow = 9)
show_landscape(l2)

I checked out the function and flipped the logic for the byrow parameter when creating col_index and row_index, and this seemed to solve the problem. Note the comments at the end of lines 12 and 13.

New function:

r.nlm_planargradient = function (ncol, nrow, resolution = 1, direction = NA, rescale = TRUE) 
{
  checkmate::assert_count(ncol, positive = TRUE)
  checkmate::assert_count(nrow, positive = TRUE)
  checkmate::assert_numeric(direction)
  checkmate::assert_logical(rescale)
  if (is.na(direction)) {
    direction <- stats::runif(1, 0, 360)
  }
  eastness <- sin((pi/180) * direction)
  southness <- cos((pi/180) * direction) * -1
  col_index <- matrix(0:(ncol - 1), nrow, ncol, byrow = TRUE) # This didn't have byrow (assume FALSE)
  row_index <- matrix(0:(nrow - 1), nrow, ncol, byrow = FALSE) # This was set to TRUE
  gradient_matrix <- (southness * row_index + eastness * col_index)
  gradient_raster <- raster::raster(gradient_matrix)
  raster::extent(gradient_raster) <- c(0, ncol(gradient_raster) * 
                                         resolution, 0, nrow(gradient_raster) * resolution)
  if (rescale == TRUE) {
    gradient_raster <- util_rescale(gradient_raster)
  }
  return(gradient_raster)
}

I used this new function here and it worked:

l3 = r.nlm_planargradient(ncol = 10, nrow = 9)
show_landscape(l3)

Also, it would be great if you could add user_seed like you have for some of the other functions.

Otherwise, thanks for a fantastic package!!

Session Info

```r R version 3.6.1 (2019-07-05) Platform: x86_64-w64-mingw32/x64 (64-bit) Running under: Windows >= 8 x64 (build 9200) Matrix products: default locale: [1] LC_COLLATE=English_United States.1252 LC_CTYPE=English_United States.1252 LC_MONETARY=English_United States.1252 [4] LC_NUMERIC=C LC_TIME=English_United States.1252 attached base packages: [1] stats graphics grDevices utils datasets methods base other attached packages: [1] viridis_0.5.1 viridisLite_0.3.0 landscapetools_0.5.0 raster_3.0-7 sp_1.3-1 [6] NLMR_0.4.2 dplyr_0.8.3 oSCR_0.42.0 loaded via a namespace (and not attached): [1] Rcpp_1.0.2 rstudioapi_0.10 magrittr_1.5 munsell_0.5.0 tidyselect_0.2.5 colorspace_1.4-1 lattice_0.20-38 [8] R6_2.4.0 rlang_0.4.0 tools_3.6.1 parallel_3.6.1 grid_3.6.1 gtable_0.3.0 checkmate_1.9.4 [15] digest_0.6.21 lazyeval_0.2.2 assertthat_0.2.1 tibble_2.1.3 crayon_1.3.4 gridExtra_2.3 ggplot2_3.2.1 [22] purrr_0.3.2 codetools_0.2-16 glue_1.3.1 labeling_0.3 compiler_3.6.1 pillar_1.4.2 scales_1.0.0 [29] backports_1.1.4 pkgconfig_2.0.3 ```

[bitbacchus]

@GatesDupont thanks for your fantastic bug report! I have adopted your fix. You can install the fixed version from my branch:

# install.packages("devtools")
devtools::install_github("ropensci/NLMR@bugfix#65")

I've opened a pull request and the fix should be on CRAN soon.

[bitbacchus]

@GatesDupont , about your suggestion regarding user_seed: Can't you just use the set.seed() function?

set.seed(1)
planar_gradient <- nlm_planargradient(ncol = 80, nrow = 100, direction = 180)
landscapetools::show_landscape(planar_gradient)

Actually, I can't see a use case for an additional user_seed at all. The only functions in the package with a seed parameter are the rcpp functions in the backend (AFAIK), which I should fix some time.

[marcosci]

Oopsie daisy! Thanks for finding the bug and also providing the fix 😄

Does the seed solution from @bitbacchus work for you?

[GatesDupont]

That works, I didn't know about the connection to rcpp. I just found the user_seed parameter in nlm_gaussianfield to be useful. Thanks all!