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

r.neighbors: implement parallelization with OpenMP #1724

Merged
merged 16 commits into from
Apr 26, 2022

Conversation

aaronsms
Copy link
Contributor

@aaronsms aaronsms commented Jul 14, 2021

This is a rework upon the revision to the first implementation of r.neighbors (#1654). It avoids using temporary file buffer to reduce unnecessary disk I/O which is not ideal in some systems. Instead, it allows the users to specify a parameter "memory" to indicate the amount of memory to be used for the output buffer. Furthermore, it exposes a "nprocs" parameter to allow users the amount of processing units to be used for computation. A benchmark script is also included under r.neighbors/benchmark to showcase how the performance scales with "nprocs", using 4 generated raster maps of size 50M, 100M, 200M and 400M respectively.

Checklists before merging:

  • code review
  • CI passes
  • performance section in documentation
  • confirm values in test are from the old version (run new tests with old code)
  • run tests without OpenMP (runs in CI)
  • visual check of results with custom data ("looks good" with non NC SPM dataset)
  • check that it works with really large data (16B cells)
  • run multi-core benchmark (no degraded performance with many threads)
  • run one core benchmark on many resolutions or many cell
  • rebase to main

@aaronsms
Copy link
Contributor Author

Here's the benchmark given from the script.

neighbors-size-7-after

@wenzeslaus wenzeslaus added the gsoc Reserved for Google Summer of Code student(s) label Jul 16, 2021
@wenzeslaus
Copy link
Member

I was running the following test in NC SPM sample location (2,025,000 cells):

g.region raster=elevation@PERMANENT res=$RES -a
r.surf.fractal output=fractals
r.neighbors input="fractals" output="neighbors" method="median" size=31 [memory=10000]

I repeated each run 30 times and used different machines with up to 32 cores (on HPC). Here are tests for 8 and 32 cores (results were the same or similar for all the machines and max nprocs I used):

compare_seg_file_mem_8_cores
compare_seg_file_mem_32_cores

Memory is the version in this PR. Segment and File are the two previous versions of parallel r.neighbors using the segment library and r.mfilter-like temporary files, respectively.

We discussed these results and the advantages and disadvantages of each approach during our GSoC meetings, so this just for the record.

@dnewcomb
Copy link
Contributor

Benchmark running on a Ryzen 7 1800 with 8 cores 16 threads. and 56 GB RAM. The data was reading and writing from a 1 TB spinning drive. 2 core jump in time confuses me.

test3_Figure_1

@HuidaeCho HuidaeCho added enhancement New feature or request raster Related to raster data processing labels Aug 1, 2021
@aaronsms aaronsms changed the title feat(raster): implement r.neighbors parallelization with OpenMP r.neighbors: implement parallelization with OpenMP Aug 4, 2021
@petrasovaa
Copy link
Contributor

Different benchmark on 16 billion cells with the label r.neighbors_[window size]_[memory]:
results1

It shows computations with larger window size can gain a lot of speed up. Apparently a lot of memory doesn't really help.

Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

Good. Just some smaller issues.

for (row = start - ncb.dist; row < start + ncb.dist; row++) at 512 could be for (row = 0; row < 2 * ncb.dist; row++), but not needed.

raster/r.neighbors/local_proto.h Outdated Show resolved Hide resolved
raster/r.neighbors/main.c Show resolved Hide resolved
raster/r.neighbors/main.c Outdated Show resolved Hide resolved
raster/r.neighbors/main.c Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request gsoc Reserved for Google Summer of Code student(s) raster Related to raster data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants