Opened 5 years ago

Last modified 4 years ago

#3695 new enhancement

r.terraflow should support large file dimensions by default

Reported by: mankoff Owned by: grass-dev@…
Priority: normal Milestone: 7.8.3
Component: Raster Version: svn-trunk
Keywords: r.terraflow Cc: mankoff@…
CPU: Unspecified Platform: Unspecified

Description

Currently r.terraflow is limited to rasters with width or height < SHRT_MAX (215, 32678). I imagine this default made sense years ago when this was coded, but now on even a decent laptop r.watershed works with rasters that large. I suggest r.terraflow remove this limit so that users with large rasters do not need to recompile it. Patch attached and inline below:

Index: raster/r.terraflow/types.h
===================================================================
--- raster/r.terraflow/types.h	(revision 73638)
+++ raster/r.terraflow/types.h	(working copy)
@@ -26,8 +26,8 @@
 /* input parameters type */
 /* ------------------------------------------------------------ */
 
-typedef short dimension_type; /* represent dimension of the grid */
-static const dimension_type dimension_type_max=SHRT_MAX;
+typedef long dimension_type; /* represent dimension of the grid */
+static const dimension_type dimension_type_max=LONG_MAX;
 
 typedef short direction_type;  /* represent the direction of a cell */
 static const direction_type DIRECTION_UNDEF=-1;

Attachments (1)

r.terraflow.patch (687 bytes ) - added by mankoff 5 years ago.
Replacing patch with INT instead of LONG

Download all attachments as: .zip

Change History (10)

comment:1 by mankoff, 5 years ago

Summary: r.terraflow default should work with large file dimensionsr.terraflow default should support large file dimensions by default

comment:2 by mankoff, 5 years ago

Summary: r.terraflow default should support large file dimensions by defaultr.terraflow should support large file dimensions by default

in reply to:  description comment:3 by mmetz, 5 years ago

Replying to mankoff:

Currently r.terraflow is limited to rasters with width or height < SHRT_MAX (32678). I imagine this default made sense years ago when this was coded, but now on even a decent laptop r.watershed works with rasters that large.

Makes sense. Note that the concept of r.terraflow is to avoid seek'ing on slow spinning harddrives. With SSD's, there is no measurable speed penalty when seek'ing, and r.terraflow looses this advantage over e.g. r.watershed.

I suggest r.terraflow remove this limit so that users with large rasters do not need to recompile it. Patch attached and inline below:

Index: raster/r.terraflow/types.h
===================================================================
--- raster/r.terraflow/types.h	(revision 73638)
+++ raster/r.terraflow/types.h	(working copy)
@@ -26,8 +26,8 @@
 /* input parameters type */
 /* ------------------------------------------------------------ */
 
-typedef short dimension_type; /* represent dimension of the grid */
-static const dimension_type dimension_type_max=SHRT_MAX;
+typedef long dimension_type; /* represent dimension of the grid */
+static const dimension_type dimension_type_max=LONG_MAX;
 
 typedef short direction_type;  /* represent the direction of a cell */
 static const direction_type DIRECTION_UNDEF=-1;

GRASS uses int as type for rows and columns which should be more than sufficient. long, if it is a 64 bit integer, is too much, and long is sometimes 32 bit, sometimes 64 bit, therefore I suggest to change short to int.

by mankoff, 5 years ago

Attachment: r.terraflow.patch added

Replacing patch with INT instead of LONG

comment:4 by mankoff, 5 years ago

When I run it with type int it crashes with:

------------------------------
COMPUTING FLOW DIRECTIONS
classifying nodata (inner & boundary)
finding nodata
r.terraflow: 3scan.h:140: void scan3line(FUN&, AMI_STREAM<T>*, AMI_STREAM<T>*, AMI_STREAM<T>*, BASETYPE, dimension_type) [with T = float; BASETYPE = float; FUN = detectEdgeNodata; dimension_type = int]: Assertion `ae == AMI_ERROR_END_OF_STREAM' failed.
Aborted (core dumped)

This does not occur if I use long.

comment:5 by mankoff, 5 years ago

Milestone: 7.8.0

Upgrading milestone so that this ticket is migrated to GitHub

comment:6 by neteler, 5 years ago

Milestone: 7.8.07.8.1

Ticket retargeted after milestone closed

comment:7 by neteler, 4 years ago

Milestone: 7.8.17.8.2

Ticket retargeted after milestone closed

comment:8 by neteler, 4 years ago

Milestone: 7.8.2

Ticket retargeted after milestone closed

comment:9 by neteler, 4 years ago

Milestone: 7.8.3
Note: See TracTickets for help on using tickets.