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.flowaccumulation: Handle null values properly #957

Merged
merged 1 commit into from
Oct 17, 2023
Merged

r.flowaccumulation: Handle null values properly #957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/raster/r.flowaccumulation/accumulate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include <stdlib.h>
#include "global.h"

#define DIR_NULL 0x80000000
#define ACCUM(row, col) accum_map->cells[(size_t)(row)*ncols + (col)]
#define FIND_UP(row, col) \
((row > 0 ? (col > 0 && DIR(row - 1, col - 1) == SE ? NW : 0) | \
Expand Down Expand Up @@ -41,7 +40,7 @@ void accumulate(struct raster_map *dir_map, struct raster_map *accum_map)
#pragma omp parallel for schedule(dynamic) private(col)
for (row = 0; row < nrows; row++) {
for (col = 0; col < ncols; col++)
if (DIR(row, col) != DIR_NULL)
if (DIR(row, col) != cell_null)
UP(row, col) = FIND_UP(row, col);
}
#endif
Expand All @@ -51,7 +50,7 @@ void accumulate(struct raster_map *dir_map, struct raster_map *accum_map)
for (col = 0; col < ncols; col++)
/* if the current cell is not null and has no upstream cells, start
* tracing down */
if (DIR(row, col) != DIR_NULL && !UP(row, col))
if (DIR(row, col) != cell_null && !UP(row, col))
trace_down(dir_map, accum_map, row, col, 1);
}

Expand Down Expand Up @@ -103,7 +102,7 @@ static void trace_down(struct raster_map *dir_map, struct raster_map *accum_map,
/* if the downstream cell is null or any upstream cells of the downstream
* cell have never been visited, stop tracing down */
if (row < 0 || row >= nrows || col < 0 || col >= ncols ||
DIR(row, col) == DIR_NULL || !(up = UP(row, col)) ||
DIR(row, col) == cell_null || !(up = UP(row, col)) ||
!(accum_up = sum_up(accum_map, row, col, up)))
return;

Expand Down
8 changes: 8 additions & 0 deletions src/raster/r.flowaccumulation/global.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,12 @@ long long timeval_diff(struct timeval *, struct timeval *, struct timeval *);
/* accumulate.c */
void accumulate(struct raster_map *, struct raster_map *);

#ifdef _MAIN_C_
#define GLOBAL
#else
#define GLOBAL extern
#endif

GLOBAL CELL cell_null;
Comment on lines +35 to +41
Copy link
Member

Choose a reason for hiding this comment

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

Using Rast_is_c_null_value would avoid the need for an award global variable. Would Rast_is_c_null_value work here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, Rast_is_c_null_value should also work. That global was my attempt for micro-optimization. Just did a benchmark test against Rast_is_c_null_value. Well.. not much difference... Maybe, GCC can optimize out this function call as inline. Yeah.. not worth it.

Copy link
Member

Choose a reason for hiding this comment

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

Benchmark! Nice! The function is in fact a macro, so it should be always inlined.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah! I missed #ifndef Rast_is_c_null_value!

#ifndef Rast_is_c_null_value
int Rast_is_c_null_value(const CELL *cellVal)
{
    /* Check if the CELL value matches the null pattern */
    return *cellVal == (CELL)0x80000000;
}
#endif


#endif
11 changes: 8 additions & 3 deletions src/raster/r.flowaccumulation/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ int main(int argc, char *argv[])
nrows = Rast_window_rows();
ncols = Rast_window_cols();

Rast_set_c_null_value(&cell_null, 1);

dir_map = G_malloc(sizeof *dir_map);
dir_map->nrows = nrows;
dir_map->ncols = ncols;
Expand All @@ -186,15 +188,18 @@ int main(int argc, char *argv[])
Rast_get_c_row(dir_fd, dir_map->cells + ncols * row, row);
if (dir_format == DIR_DEG) {
for (col = 0; col < ncols; col++)
DIR(row, col) = pow(2, abs(DIR(row, col) / 45.));
if (DIR(row, col) != cell_null)
DIR(row, col) = pow(2, abs(DIR(row, col) / 45.));
}
else if (dir_format == DIR_DEG45) {
for (col = 0; col < ncols; col++)
DIR(row, col) = pow(2, 8 - abs(DIR(row, col)));
if (DIR(row, col) != cell_null)
DIR(row, col) = pow(2, 8 - abs(DIR(row, col)));
}
else {
for (col = 0; col < ncols; col++)
DIR(row, col) = abs(DIR(row, col));
if (DIR(row, col) != cell_null)
DIR(row, col) = abs(DIR(row, col));
}
}
G_percent(1, 1, 1);
Expand Down
Loading