Skip to content

r.geomorphon: parallelize with OpenMP using memory-bounded banding - #7783

Draft
krcoder123 wants to merge 9 commits into
OSGeo:mainfrom
krcoder123:gsoc-rgeomorphon
Draft

r.geomorphon: parallelize with OpenMP using memory-bounded banding#7783
krcoder123 wants to merge 9 commits into
OSGeo:mainfrom
krcoder123:gsoc-rgeomorphon

Conversation

@krcoder123

@krcoder123 krcoder123 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

This PR parallelizes r.geomorphon with OpenMP while keeping memory usage within the amount the user allows.

The actual parallelization method is similar to r.param.scale parallelization in theory. The output is processed in bands, with the band height computed from a new memory option. For each band the input rows are read in parallel with per thread file descriptors into a shared strip, and each band's rows are then computed in parallel. Cells near the map edges see their search window positioned exactly the way the original code positions its row buffer there, which is what allows the output to be exactly the output serial produced.

Output is exactly the same as the current serial module at 1, 2, 4, and 8 threads, across all twelve output maps, on projected and lat-lon data, through the whole memory range, with masks, and on a 100 million cell map. On my Apple M3 machine with 8 cores, I got a speedup of about 1.9x at 2 threads, 3.3 to 3.7x at 4, and 4.2 to 5.1x at 8.

@github-actions github-actions Bot added raster Related to raster data processing Python Related code is in Python C Related code is in C module CMake labels Jul 27, 2026
@krcoder123

Copy link
Copy Markdown
Contributor Author

I attached a benchmarking script in this module as well and I will post my benchmark graphs once the full run finishes on my end. I'll make the pytest for this module in a separate PR this week.

@krcoder123

Copy link
Copy Markdown
Contributor Author

Here are the scaling graphs from my machine (8 core Apple M3, 4 performance and 4 efficiency cores).

The speedups and the scalings look pretty good.

One note, I had to use a Mac specific benchmark wrapper with cooldowns and pmset because my Mac was throttling a lot using the universal benchmark that's in this PR. So the below images of my graphs are with that setup, but if you would like to also see the throttling graphs done with the universal benchmark I can attach those too.

image image image image image image

@github-actions github-actions Bot added docs markdown Related to markdown, markdown files labels Jul 28, 2026
The output is processed in bands sized from a new memory option, with the
input strip loaded through per thread descriptors and each band's rows
computed in parallel. Output is byte identical to the serial module at
every thread count across all twelve outputs, both test projections, the
memory range, and the option matrix. A new nprocs option matches the other
parallel modules.
…mentation

Also removes the stale one-off region-size warning; the one-off path now reads only the window rows.

/* Compute one cell's pattern and landform, shared by the raster and one-off
* paths. */
static void compute_forms(FCELL **rows, int cur_row, int row, int col,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would be better to move this function below main() and only keep its prototype here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider restructuring this with one or more structures.

@krcoder123 krcoder123 Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I moved compute_forms below main now and kept the prototype at the top. I restructured the parameters into a config struct and a result struct.

@krcoder123

krcoder123 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

The parallel pytest cases are written and I tested it locally against this branch. But, it needs the conftest.py file which is already in #7785. Committing with a duplicate here doesn't make sense so I'll wait for that PR to merge first. I'll then rebase and add them here, so the test files don't get duplicated across the two PRs.


/* Compute one cell's pattern and landform, shared by the raster and one-off
* paths. */
static void compute_forms(FCELL **rows, int cur_row, int row, int col,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider restructuring this with one or more structures.

Comment thread raster/r.geomorphon/main.c Outdated
memory = atoi(par_memory->answer);
nprocs = G_set_omp_num_threads(par_nprocs);
nprocs = Rast_disable_omp_on_mask(nprocs);
if (nprocs < 1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is dead, nprocs won't be < 1 after G_set_omp_num_threads

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed it now

strip_rows = strip_hi - strip_lo + 1;
for (k = 0; k < strip_rows; ++k)
strip_ptr[k] = strip_block + (size_t)k * ncols;
load_strip(elevation.fd, elevation.raster_type, tmp_buf, strip_ptr,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

elevation.fd is opened by open_map and then unused in the raster path, which opens nprocs more descriptors. Reuse it as fd_thread[0].

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, fd_thread[0] now reuses elevation.fd. This also fixed the descriptor never being closed on the raster path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C Related code is in C CMake docs markdown Related to markdown, markdown files module Python Related code is in Python raster Related to raster data processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants