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

Generate raster runif #55

Merged
merged 8 commits into from
Apr 26, 2021
Merged

Generate raster runif #55

merged 8 commits into from
Apr 26, 2021

Conversation

ToonHub
Copy link
Contributor

@ToonHub ToonHub commented Apr 21, 2021

I created a raster with a random number between 0 and 1 for each 32m cell of grts master based on the uniform distribution. This random number can be used to select a equal probability sample for terrestrial habitats based on the phab values.

@ToonHub ToonHub requested a review from florisvdh April 21, 2021 13:13
Copy link
Member

@florisvdh florisvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great that you did this @ToonHub 👍 !! I believe your PR addresses issue #46. raster_runif seems fine as a name for this datasource!

I didn't yet check all code and the datasource (will try to do that in due time); I just had a quick look at the PR in GitHub, so here is some first feedback already:

Thanks!!

src/generate_raster_runif/10_raster_runif.Rmd Outdated Show resolved Hide resolved
@ToonHub
Copy link
Contributor Author

ToonHub commented Apr 21, 2021

Below the renderded bookdown:

generating_raster_runif.zip

Copy link
Member

@florisvdh florisvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for providing the html! As said, I will look deeper into it (and maybe run the code myself), just adding some more comments in the meantime.

My previous comment:

a (2 km?) buffer around GRTSmaster_habitats

needs a bit further clarification (also in the light of #55 (comment)): here I mean buffering the non-NA part (Flanders + Brussels), not the rectangle as such.

  • in index.Rmd, you can add options(rgdal_show_exportToProj4_warnings = "none") to get rid of PROJ.4 warnings (they don't matter in this case).

src/generate_raster_runif/10_raster_runif.Rmd Outdated Show resolved Hide resolved
ToonHub added 2 commits April 22, 2021 11:12
add option rgdal not to show warning related to rproj
add parameter to avoid overwriting result
remove unnecessary libraries
increase extent of raster_runif
mask raster_unif outside buffer around Flanders
add checks for result
write result to 20_processed folder
checksums of result
@ToonHub
Copy link
Contributor Author

ToonHub commented Apr 22, 2021

Copy link
Member

@florisvdh florisvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @ToonHub ! Some minor comments.

  • missing files in src/generate_raster_runif which should have been created by renv: .Rprofile, renv/.gitignore
  • raster_runif has a hole compared to GRTSmaster_habitats, namely the Brussels Capital Region. A quick solution might be to fill the hole in the sf object flanders_buffer. I saw a possible approach in Filling polygon holes based on area r-spatial/sf#609 (comment)

I think we best discuss on the pattern of random numbers:

Compare:

> grts_master %>% crop(extent(5e4,52e3,18e4,182e3)) %>% spplot(scales = list(draw = TRUE))

image

with:

> raster_runif_buffer_mask %>% crop(extent(5e4,52e3,18e4,182e3)) %>% spplot(scales = list(draw = TRUE))

image

The simple random sampling results in more clumped patterns. More ideally I believe the random numbers could be spread in a spatially balanced way. Don't know how easy it can be accomplished, it may be feasible with inbo/grtsdb. I did a short experiment, but not with grtsdb.

src/generate_raster_runif/renv.lock Outdated Show resolved Hide resolved
src/generate_raster_runif/10_raster_runif.Rmd Outdated Show resolved Hide resolved
ToonHub added 3 commits April 26, 2021 15:06
install n2khab dev_0.5.0
use functions xxh64, md5sum, sha256sum
@ToonHub
Copy link
Contributor Author

ToonHub commented Apr 26, 2021

I added the Brussels region to the raster. As agreed with @florisvdh, we will use simple random sampling to generate the numbers.

@ToonHub ToonHub merged commit aa3fa47 into master Apr 26, 2021
@ToonHub ToonHub deleted the generate_raster_runif branch April 26, 2021 14:44
@@ -0,0 +1,271 @@
# Generate random number based on uniform distribution for each GRTS master cell

+ We start from the `GRTSmh_base4frac` data source
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw this. I don't think this is true. Also applies to next paragraphs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what exactly is not true?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, in the code you actually work with GRTSmaster_habitats, not with GRTSmh_base4frac which you state here (and for which you verify checksums in the beginning).

See:

```{r}
grts_master <- read_GRTSmh()
```

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.

Comment on lines +233 to +236
dir.create(file.path(path, "20_processed/raster_runif"), recursive = TRUE)

filepath <- file.path(fileman_up("n2khab_data"), "20_processed/raster_runif/raster_runif.tif")
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it should go to 10_raw. Although you use GRTSmh for extent and resolution, the values are completely new data, so I'd still put it in 10_raw (all those data sources are typically never 'made from scratch', so it's to be seen relative).

I suggested that before, in #55 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I tought that I changed this already. But not...

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

Successfully merging this pull request may close these issues.

2 participants