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.

Sign up for GitHub

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

Jump to bottom

r.series: implement parallelization with OpenMP #1776

Merged
merged 14 commits into from
Apr 26, 2022
Merged

r.series: implement parallelization with OpenMP #1776

merged 14 commits into from
Apr 26, 2022

Conversation

aaronsms
Copy link
Contributor

@aaronsms aaronsms commented Aug 4, 2021 •
edited by wenzeslaus
Loading

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)
  • 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 tests without OpenMP (runs in CI)
  • 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

aaronsms commented Aug 4, 2021

This is a draft awaiting PR #1772

@wenzeslaus wenzeslaus added enhancement New feature or request gsoc Reserved for Google Summer of Code student(s) labels Aug 5, 2021
@HuidaeCho HuidaeCho added the raster Related to raster data processing label Aug 6, 2021
@aaronsms aaronsms force-pushed the parallel-r.series branch 2 times, most recently from 4dfbbce to 5a6cb0d Compare August 20, 2021 13:17
@wenzeslaus wenzeslaus added this to the 8.2.0 milestone Aug 24, 2021
@petrasovaa
Copy link
Contributor

Benchmark with 16 billion cells, sum and median of 4 rasters, just one run:
results

@aaronsms
Copy link
Contributor Author

For this, parallelization is possible for lazy (opening of raster files), but the performance will not scale really well with more nprocs.

@aaronsms
Copy link
Contributor Author

For the failed test cases, I will spend more time investigating it.

wenzeslaus
wenzeslaus approved these changes Apr 24, 2022
View reviewed changes
Copy link
Member

@wenzeslaus wenzeslaus left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Code looks good, just minor doc updates needed.

raster/r.series/r.series.html Show resolved Hide resolved
raster/r.series/main.c Show resolved Hide resolved
petrasovaa
petrasovaa reviewed Apr 25, 2022
View reviewed changes
raster/r.series/main.c Outdated Show resolved Hide resolved
petrasovaa
petrasovaa reviewed Apr 25, 2022
View reviewed changes
raster/r.series/main.c Outdated Show resolved Hide resolved
@petrasovaa petrasovaa merged commit 5190da3 into OSGeo:main Apr 26, 2022
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
@aaronsms @ninsbl
r.series: implement parallelization with OpenMP (OSGeo#1776)
3ec400e
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
@aaronsms @ninsbl
r.series: implement parallelization with OpenMP (OSGeo#1776)
20b4ce8
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
@aaronsms @neteler
r.series: implement parallelization with OpenMP (OSGeo#1776)
f3b861b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HuidaeCho HuidaeCho HuidaeCho left review comments

@petrasovaa petrasovaa petrasovaa left review comments

@wenzeslaus wenzeslaus wenzeslaus approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request gsoc Reserved for Google Summer of Code student(s) raster Related to raster data processing
Projects
None yet
Milestone
8.2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@aaronsms @petrasovaa @wenzeslaus @HuidaeCho