[GRASS-dev] Re: [GRASS GIS] #392: backport G_is_c_null_value() to devbr6

[GRASS GIS]

#392: backport G_is_c_null_value() to devbr6
----------------------+-----------------------------------------------------
  Reporter:  hamish   |       Owner:  grass-dev at lists.osgeo.org
      Type:  task     |      Status:  new                      
  Priority:  minor    |   Milestone:  6.5.0                    
 Component:  default  |     Version:  svn-develbranch6         
Resolution:           |    Keywords:  NULL                     
  Platform:  All      |         Cpu:  All                      
----------------------+-----------------------------------------------------
Changes (by hamish):

  * priority:  blocker => minor
  * keywords:  => NULL

Comment:

 um, nevermind about the recent noise re. broken NULL support in GRASS 6.5.
 It was only due to local modifications at my end partially implementing
 this patch.

 develbranch_6 and releasebranch_6_4 both still use the old way of testing
 for a NULL cell. Which takes the conversation back to:

 Glynn:
 > I suggest completely replacing null_val.c with the 7.0 version,
 > and editing gisdefs.h to match. Either that, or leave it as-is.

 Hamish:
 > In that case, for the rc1 release I'd say leave it as is. It's
 > too core a fn to mess with so close to release time.
 >
 > For rc2, to completely replace null_val.c or not? I am still a
 > bit unsure- is there actually a bug in the current devbr6
 > version or is the idea to keep the methods in sync to ease
 > future maintenance?

 Glynn:
 > It's just clean-up. The existing code is just unnecessarily
 > complex:
 ...
 > So you can use:
 {{{
 svn merge -c 34445,34446,r34747
 https://svn.osgeo.org/grass/grass/trunk/lib/gis/null_val.c
 lib/gis/null_val.c
 }}}


 overly complex to the point of significantly slowing everything down? If
 there is some opportunity to speed up raster processing by some tangible
 amount it may be worth it, otherwise I'd play it safe and do nothing.
 Probably would have to do some time trials to answer that:

 {{{
 ### grass 6.5 ###
 # spearfish
 g.region rast=fields res=1 -p
   rows:       14000
   cols:       19000

 time r.univar fields
   total null and non-null cells: 266000000
   total null cells: 101220000
 -1-
 real    0m41.798s
 user    0m40.347s
 sys     0m0.360s
 -2-
 real    0m42.245s
 user    0m40.179s
 sys     0m0.340s
 -3-
 real    0m42.601s
 user    0m40.359s
 sys     0m0.268s
 }}}

 {{{
 ### grass 7.0 ###
 # spearfish
 g.region rast=fields res=1 -p
 time r.univar fields
 -1-
 real    0m36.288s
 user    0m35.106s
 sys     0m0.268s
 -2-
 real    0m36.079s
 user    0m34.926s
 sys     0m0.272s
 -3-
 real    0m36.274s
 user    0m35.154s
 sys     0m0.300s
 }}}


 So about '''15% faster in GRASS 7'''. I don't know if all of that can be
 attributed to this change, but if likely then it would be very useful to
 backport it, at least to develbranch_6.


 Hamish

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