[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 hamish):

 note compiler warning: (x3)
 {{{
 include/grass/iostream/replacementHeapBlock.h:146: warning: 'str' may be
 used uninitialized in this function
 }}}

 spearfish test:
 {{{
 g.region -d
 r.surf.volcano output=gauss method=gaussian sigma=1
 d.where
 r.viewshed in=gauss out=gauss.shed coord=597900,4920840 max_dist=7500 --o

 ###
 d.histogram gauss.shed
 }}}
 seems to keep -1 as invisibile not NULL ?????  fix:
 {{{
 r.null gauss.shed setnull=-1
 r.colors gauss.shed color=bcyr
 d.redraw

 # 180deg is just @ viewing cell, so for good colors we also need:  (PITA,
 so maybe we could build that into the module)
 r.null gauss.shed setnull=180
 r.colors gauss.shed color=bcyr
 d.redraw
 ###

 echo "597900,4920840" | v.in.ascii out=view_pt fs=,
 nviz gauss color=gauss.shed points=view_pt
 }}}



 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?

 tgt_elev=:  how is that used if a target coord is never defined?


 any interesting coverage/patch map functionality from r.cva which would
 be interesting to clean-room reimpliement?




 timings on my trusty old P4:
 {{{
 G:65> time r.viewshed in=elevation.dem out=elev.shed coord=597900,4920840
 max_dist=7500 --o
 real    0m3.898s
 user    0m3.768s
 sys     0m0.084s

 G65> time r.los in=elevation.dem out=elev.shed coord=597900,4920840
 max_dist=7500
 real    0m4.156s
 user    0m4.004s
 sys     0m0.108s


 G65> g.region rast=elevation.10m
 G65> time r.viewshed in=elevation.10m out=elev.shed coord=597900,4920840
 max_dist=7500 --o
 real    0m42.330s
 user    0m40.791s
 sys     0m0.532s

 G65> time r.los in=elevation.10m out=elev.shed coord=597900,4920840
 max_dist=7500 --o
 real    (finsihed in about 20 minutes)
 }}}

 indeed it seems to scale a bit better than r.los ... :-)



 > 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.
 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.



 Hamish

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