[GRASS-dev] Re: [GRASS GIS] #390: r.viewshed precision bug

[GRASS GIS]

#390: r.viewshed precision bug
------------------------+---------------------------------------------------
 Reporter:  neteler     |       Owner:  grass-dev@…              
     Type:  defect      |      Status:  new                      
 Priority:  major       |   Milestone:  6.4.2                    
Component:  Raster      |     Version:  svn-develbranch6         
 Keywords:  r.viewshed  |    Platform:  All                      
      Cpu:  All         |  
------------------------+---------------------------------------------------

Comment(by mmetz):

 Replying to [comment:7 hamish]:
 > note compiler warning: (x3)
 {{{
 > include/grass/iostream/replacementHeapBlock.h:146: warning: 'str' may be
 used uninitialized in this function
 }}}

 Hmm, not here, although I usually get warnings about uninitialized
 variables.

 [...]
 > seems to keep -1 as invisibile not NULL ?????
 fixed in r46201, came from original code
 {{{
 > [...]
 >
 > # 180deg is just @ viewing cell, so for good colors we also need:
 (PITA, so maybe we could build that into the module)
 > [...]
 }}}
 Same as r.los, the viewing cell is defined to be straight below the
 viewpoint = 180 deg.

 >
 >
 >
 > coord, -r desc:  lat/lon -> easting/northing
 >   (seems to use current e,n LOCATION coords already..)
 > drop the -r flag now that debugging stage is ending?

 Sure, can be dropped.
 >
 > tgt_elev=:  how is that used if a target coord is never defined?
 >
 Assume you want to identify the cells where the top of a hypothetical
 structure (person, building, signal receiver, ...) is visible from the
 observation point, and it suffices if the top is visible, the base need
 not be visible.
 >
 >
 > timings on my trusty old P4:
 [...]
 >
 > indeed it seems to scale a bit better than r.los ... :-)
 >
 For small regions, you would mostly test the speed of reading the input
 map and writing the output map, but not the speed needed for LOS
 computation...
 >
 >
 > > For GRASS 7, we are free to change module options, for GRASS 6, this
 patt_map
 > > option could be added for compatibility reasons, but the only effect
 would
 > > then be to print out a warning that a MASK should be used instead of
 > > patt_map=mask_map?
 >
 >
 > I wasn't thinking to make this as r.los2; as it is a totally diff't code
 > base I'd add it named as r.viewshed, and drop r.los in grass7.
 Makes sense...
 > but maybe it is prefered to keep the r.los name? if so then at least in
 > GRASS 6.x it would be fine to have the option as "This does nothing and
 is
 > kept for backwards compatibility reasons." and the warning as you
 describe.
 > ?  since r.los is so useless at modern region array sizes, maybe we
 should..
 > alternately, we could add r.viewshed in 6.x with a wrapper script for
 the old r.los name, like r.cats -> r.category.
 >
 In GRASS 7, other names changed too, e.g. v.db.addcol -> v.db.addcolumn

 A quick glimpse into the manual with the list of raster modules should
 reveal the name of module that can do LOS analysis, a new name should be
 ok considering the vastly different code base.

 Markus M

-- 
Ticket URL: <http://trac.osgeo.org/grass/ticket/390#comment:9>
GRASS GIS <http://grass.osgeo.org>