Opened 5 years ago

Last modified 5 months ago

#2296 new enhancement

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

Reported by: hellik Owned by: grass-dev@…
Priority: normal Milestone:
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 (22)

comment:1 in reply to:  description ; Changed 5 years ago by 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).

comment:2 in reply to:  1 ; Changed 5 years ago by hellik

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?

comment:3 in reply to:  2 Changed 5 years ago by hellik

Replying to hellik:

worth to open a new ticket?

opened a new ticket, see #2301

comment:4 Changed 5 years ago by hellik

Milestone: 7.0.07.1.0

comment:5 in reply to:  description Changed 5 years ago by neteler

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 Changed 5 years ago by neteler

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 Changed 5 years ago by neteler

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 Changed 4 years ago by neteler

Milestone: 7.0.17.0.2

Ticket retargeted after 7.0.1 milestone closed

comment:9 Changed 4 years ago by neteler

Milestone: 7.0.27.0.3

Ticket retargeted after milestone closed

comment:10 Changed 4 years ago by neteler

Milestone: 7.0.3

Ticket retargeted after milestone closed

comment:11 Changed 4 years ago by neteler

Milestone: 7.0.4

Ticket retargeted after 7.0.3 milestone closed

comment:12 Changed 4 years ago by mlennert

Can this be closed ?

comment:13 in reply to:  12 Changed 4 years ago by hellik

Replying to mlennert:

Can this be closed ?

AFAIK not completed yet.

comment:14 Changed 3 years ago by martinl

Milestone: 7.0.47.0.5

comment:15 Changed 3 years ago by martinl

Milestone: 7.0.57.3.0
Priority: majornormal

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

comment:16 Changed 3 years ago by martinl

Milestone: 7.3.07.4.0

Milestone renamed

comment:17 Changed 20 months ago by neteler

Milestone: 7.4.07.4.1

Ticket retargeted after milestone closed

comment:18 Changed 15 months ago by neteler

Milestone: 7.4.17.4.2

comment:19 Changed 12 months ago by martinl

Milestone: 7.4.27.6.0

All enhancement tickets should be assigned to 7.6 milestone.

comment:20 Changed 8 months ago by martinl

Milestone: 7.6.07.6.1

Ticket retargeted after milestone closed

comment:21 Changed 6 months ago by martinl

Milestone: 7.6.17.6.2

Ticket retargeted after milestone closed

comment:22 Changed 5 months ago by martinl

Milestone: 7.6.2

Remove Milestone from Addons bugreports.

Note: See TracTickets for help on using tickets.