Opened 8 years ago

Last modified 2 years ago

#2296 new enhancement

r.stream.* - unify some functions (avoid code duplication)

Reported by: hellik Owned by: grass-dev@…
Priority: normal Milestone: 7.8.3
Component: Addons Version: svn-trunk
Keywords: r.stream.* Cc:
CPU: All Platform: All

Description

the r.stream.*-modules were recently added to trunk.

to avoid code duplication, some functions should be unified over these modules in a lib(?) because following files are the same over all r.stream.*-modules:

http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.channel/io.c
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.distance/io.c
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.order/io.c
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.segment/io.c
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.snap/io.c
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.stats/io.c
http://trac.osgeo.org/grass/browser/grass-addons/grass7/raster/r.stream.basins/io.c
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.channel/io.h
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.distance/io.h
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.order/io.h
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.segment/io.h
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.snap/io.h
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.stats/io.h
http://trac.osgeo.org/grass/browser/grass-addons/grass7/raster/r.stream.basins/io.h

Change History (23)

in reply to:  description ; comment:1 by glynn, 8 years ago

Replying to hellik:

to avoid code duplication, some functions should be unified over these modules in a lib(?)

While we're at it, maybe we should look into unifying all of the different "segment" libraries.

They all do essentially the same thing: provide a 2-dimensional array which may be too large to fit into RAM (or, more accurately, into the process' address space; if RAM was the issue, mmap() etc would suffice), and which can be accessed (more or less) randomly.

Apart from the "official" segment library (lib/segment), r.proj has its own, r.stream.* each have their own, r.grow.distance has something simpler (the temporary file is read row-by-row but in reverse).

in reply to:  1 ; comment:2 by hellik, 8 years ago

Replying to glynn:

Replying to hellik:

to avoid code duplication, some functions should be unified over these modules in a lib(?)

While we're at it, maybe we should look into unifying all of the different "segment" libraries.

They all do essentially the same thing: provide a 2-dimensional array which may be too large to fit into RAM (or, more accurately, into the process' address space; if RAM was the issue, mmap() etc would suffice), and which can be accessed (more or less) randomly.

Apart from the "official" segment library (lib/segment), r.proj has its own, r.stream.* each have their own, r.grow.distance has something simpler (the temporary file is read row-by-row but in reverse).

http://trac.osgeo.org/grass/browser/grass/trunk/lib/segment
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.grow.distance
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.proj
http://trac.osgeo.org/grass/browser/grass/trunk/raster/r.stream.extract

unifying all of the different "segment" libraries may be a good choice.

worth to open a new ticket?

in reply to:  2 comment:3 by hellik, 8 years ago

Replying to hellik:

worth to open a new ticket?

opened a new ticket, see #2301

comment:4 by hellik, 8 years ago

Milestone: 7.0.07.1.0

in reply to:  description comment:5 by neteler, 8 years ago

Milestone: 7.1.07.0.0

Replying to hellik:

the r.stream.*-modules were recently added to trunk.

to avoid code duplication, some functions should be unified over these modules in a lib(?) because following files are the same over all r.stream.*-modules

The io.c and files I had sync'ed in r60127. Not sure if it is worth to move them into an own library.

comment:6 by neteler, 8 years ago

Component: RasterAddons

TODO for all r.stream.* modules (reported by mmetz in grass-dev):

Most importantly it is the NULL handling that needs to be fixed, and synced between the ram and disk modes. More generally, the ram and disk modes need to be synced and validated. For ease of maintenance, the ram mode could be removed.

comment:7 by neteler, 7 years ago

Milestone: 7.0.07.0.1

For the record:

  • r64502 mmetz r.stream.*: sync ram and seg mode
  • r64374 mmetz r-stream: fix NULL handling step 1

comment:8 by neteler, 7 years ago

Milestone: 7.0.17.0.2

Ticket retargeted after 7.0.1 milestone closed

comment:9 by neteler, 6 years ago

Milestone: 7.0.27.0.3

Ticket retargeted after milestone closed

comment:10 by neteler, 6 years ago

Milestone: 7.0.3

Ticket retargeted after milestone closed

comment:11 by neteler, 6 years ago

Milestone: 7.0.4

Ticket retargeted after 7.0.3 milestone closed

comment:12 by mlennert, 6 years ago

Can this be closed ?

in reply to:  12 comment:13 by hellik, 6 years ago

Replying to mlennert:

Can this be closed ?

AFAIK not completed yet.

comment:14 by martinl, 6 years ago

Milestone: 7.0.47.0.5

comment:15 by martinl, 6 years ago

Milestone: 7.0.57.3.0
Priority: majornormal

The modules are back in addons. Downgrading priority and changing milestone.

comment:16 by martinl, 6 years ago

Milestone: 7.3.07.4.0

Milestone renamed

comment:17 by neteler, 4 years ago

Milestone: 7.4.07.4.1

Ticket retargeted after milestone closed

comment:18 by neteler, 4 years ago

Milestone: 7.4.17.4.2

comment:19 by martinl, 4 years ago

Milestone: 7.4.27.6.0

All enhancement tickets should be assigned to 7.6 milestone.

comment:20 by martinl, 3 years ago

Milestone: 7.6.07.6.1

Ticket retargeted after milestone closed

comment:21 by martinl, 3 years ago

Milestone: 7.6.17.6.2

Ticket retargeted after milestone closed

comment:22 by martinl, 3 years ago

Milestone: 7.6.2

Remove Milestone from Addons bugreports.

comment:23 by neteler, 2 years ago

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