[GRASS-dev] r.neighbors bug in cvs

I have found an error in the CVS version of the average for r.neighbors.

I've tested this with source compiles of the latest cvs snapshot on both a 32 bit and 64 bit machine.

Basically, the 6.2 version of r.neighbors gives the correct value. The cvs version reports inflated values for DCELL input layers.

For example if the input values in a 3x3 window are:

0.8747536354 0.9208617206 1.0010881339
1.0152474088 1.0720150743 1.0436036351
1.0319341389 1.0412245464 1.0209956859

the value for the center pixel should be: 1.0024137755

the cvs version reports 1.518590477 for a regular square window and 1.5024137755 using a circular window.

T
--
Trevor Wiens
twiens@interbaun.com

The significant problems that we face cannot be solved at the same
level of thinking we were at when we created them.
(Albert Einstein)

On Tue, 2007-09-18 at 21:23 -0600, Trevor Wiens wrote:

I have found an error in the CVS version of the average for r.neighbors.

I've tested this with source compiles of the latest cvs snapshot on both a 32 bit and 64 bit machine.

Basically, the 6.2 version of r.neighbors gives the correct value. The cvs version reports inflated values for DCELL input layers.

For example if the input values in a 3x3 window are:

0.8747536354 0.9208617206 1.0010881339
1.0152474088 1.0720150743 1.0436036351
1.0319341389 1.0412245464 1.0209956859

the value for the center pixel should be: 1.0024137755

the cvs version reports 1.518590477 for a regular square window and 1.5024137755 using a circular window.

There is no significant difference between the two versions. They are
nearly identical, which unfortunately tends to point toward lib/gis.

What is the exact command you are using, so we can try to replicate?

--
73, de Brad KB8UYR/6 <rez touchofmadness com>

Trevor Wiens wrote:

I have found an error in the CVS version of the average for r.neighbors.

I'm glad that someone is actually checking their results; this bug has
been present for over a year, and you're the first person to notice.

I've tested this with source compiles of the latest cvs snapshot on both a 32 bit and 64 bit machine.

Basically, the 6.2 version of r.neighbors gives the correct value. The cvs version reports inflated values for DCELL input layers.

For example if the input values in a 3x3 window are:

0.8747536354 0.9208617206 1.0010881339
1.0152474088 1.0720150743 1.0436036351
1.0319341389 1.0412245464 1.0209956859

the value for the center pixel should be: 1.0024137755

the cvs version reports 1.518590477 for a regular square window and 1.5024137755 using a circular window.

I think that you have those numbers the wrong way around. I get
1.5185904770400001 with -c and 1.5024137754777778 without it.

Note that the square window case is off by exactly 0.5 (the -c switch
isn't present in 6.2.x).

When I replaced the aggregates with those in lib/stats, I moved the
rounding out of the individual aggregates and into the main program.
The 0.5 should only be added when writing integer maps, but that part
got missed.

Fixed in CVS, with:

--- raster/r.neighbors/main.c 11 Sep 2007 17:53:52 -0000 2.16
+++ raster/r.neighbors/main.c 19 Sep 2007 08:04:31 -0000
@@ -216,7 +216,7 @@
     exit(EXIT_FAILURE);
   }

- half = menu[method].half;
+ half = (map_type == CELL_TYPE) ? menu[method].half : 0;

   /* establish the newvalue routine */
   newvalue = menu[method].method;

--
Glynn Clements <glynn@gclements.plus.com>

On Wed, 19 Sep 2007 09:19:07 +0100
Glynn Clements <glynn@gclements.plus.com> wrote:

Fixed in CVS, with:

Confirmed. Downloaded and retested this morning. Thanks

T
--
Trevor Wiens
twiens@interbaun.com

The significant problems that we face cannot be solved at the same
level of thinking we were at when we created them.
(Albert Einstein)

On Wednesday 19 September 2007, Trevor Wiens wrote:

On Wed, 19 Sep 2007 09:19:07 +0100

Glynn Clements <glynn@gclements.plus.com> wrote:
> Fixed in CVS, with:

Confirmed. Downloaded and retested this morning. Thanks

T

Are there any mechanisms in place such that core modules like this can be
tested on a regular basis for correct results ?

Dylan

--
Dylan Beaudette
Soils and Biogeochemistry Graduate Group
University of California at Davis
530.754.7341

On Wed, 19 Sep 2007 11:44:52 -0700
Dylan Beaudette <dylan.beaudette@gmail.com> wrote:

On Wednesday 19 September 2007, Trevor Wiens wrote:
> On Wed, 19 Sep 2007 09:19:07 +0100
>
> Glynn Clements <glynn@gclements.plus.com> wrote:
> > Fixed in CVS, with:
>
> Confirmed. Downloaded and retested this morning. Thanks
>
> T

Are there any mechanisms in place such that core modules like this can be
tested on a regular basis for correct results ?

I don't know what would be ideal, but one possiblity would be to start building a testing map set for the Spearfish data set and some simple scripts to go with it.

In the case of r.neighbors for example I would think as simple script would be ideal.
something like
begin loop through neighbors operations
  r.neighbors over limited area
  r.what to extract values from specific location
  results match
    report OK
  results don't match
    report problem
end loop

I would think that if this was slowly built up for each core module, then future developers could simply use this as a testing mechanism which should speed development.

Also, prior to release a master script could run each of these in turn against the rc versions to make sure that the core functionality was still working.

I realize this is labour intensive but have no suggestions as to a quicker but still reliable method.

For myself, I doubt I will have little time to this, as I still have v.buffer waiting to be fixed on my to do list.

T
--
Trevor Wiens
twiens@interbaun.com

The significant problems that we face cannot be solved at the same
level of thinking we were at when we created them.
(Albert Einstein)

Trevor Wiens wrote:

> Are there any mechanisms in place such that core modules like this can be
> tested on a regular basis for correct results ?

I don't know what would be ideal, but one possiblity would be to
start building a testing map set for the Spearfish data set and some
simple scripts to go with it.

In the case of r.neighbors for example I would think as simple
script would be ideal.
something like
begin loop through neighbors operations
  r.neighbors over limited area
  r.what to extract values from specific location

I'd suggest using r.out.ascii rather than r.what.

  results match
    report OK
  results don't match
    report problem
end loop

I would think that if this was slowly built up for each core module,
then future developers could simply use this as a testing mechanism
which should speed development.

Also, prior to release a master script could run each of these in
turn against the rc versions to make sure that the core
functionality was still working.

I realize this is labour intensive but have no suggestions as to a
quicker but still reliable method.

You could reduce the effort a bit by adopting a common protocol for
test scripts. E.g. a sequence of commands, including some commands
which dump information to text files (e.g. r.out.ascii, v.out.ascii,
r.info > textfile, etc), where the text files have standardised names
(e.g. test-output-<num>.txt).

Testing changes to a module would involve running the script on the
previous version of the module, moving the output files to a
directory, running the script on the new version, moving the output
files to a different directory, running "diff -r" on the directories,
and reporting any differences.

There's still a fair amount of work involved in constructing useful
commands (i.e. selecting combinations of options, selecting input maps
which are appropriate for the command). This part probably can't
realistically be automated, as it requires some knowledge of
"sensible" option values and combinations.

Unfortunately, getting people to work for free on tedious jobs like
this is a lot harder than getting them to work for free on more
interesting tasks.

--
Glynn Clements <glynn@gclements.plus.com>

Dylan Beaudette-2 wrote:

On Wednesday 19 September 2007, Trevor Wiens wrote:

On Wed, 19 Sep 2007 09:19:07 +0100

Glynn Clements <glynn@gclements.plus.com> wrote:
> Fixed in CVS, with:

Confirmed. Downloaded and retested this morning. Thanks

T

Are there any mechanisms in place such that core modules like this can be
tested on a regular basis for correct results ?

Yes, but currently unused (!):
testsuite/raster/
testsuite/vector/

You can put new scripts there and then we could run
checksum tests and so forth, like Soeren did for a while:

http://www-pool.math.tu-berlin.de/~soeren/grass/GRASS_TestSuite/
http://www-pool.math.tu-berlin.de/~soeren/grass/GRASS_TestSuite/html/

We "just" have to use out tools :slight_smile:

Markus
--
View this message in context: http://www.nabble.com/r.neighbors-bug-in-cvs-tf4478584.html#a12786314
Sent from the Grass - Dev mailing list archive at Nabble.com.

On Wednesday 19 September 2007, Trevor Wiens wrote:

On Wed, 19 Sep 2007 11:44:52 -0700

Dylan Beaudette <dylan.beaudette@gmail.com> wrote:
> On Wednesday 19 September 2007, Trevor Wiens wrote:
> > On Wed, 19 Sep 2007 09:19:07 +0100
> >
> > Glynn Clements <glynn@gclements.plus.com> wrote:
> > > Fixed in CVS, with:
> >
> > Confirmed. Downloaded and retested this morning. Thanks
> >
> > T
>
> Are there any mechanisms in place such that core modules like this can be
> tested on a regular basis for correct results ?

I don't know what would be ideal, but one possiblity would be to start
building a testing map set for the Spearfish data set and some simple
scripts to go with it.

In the case of r.neighbors for example I would think as simple script would
be ideal. something like
begin loop through neighbors operations
  r.neighbors over limited area
  r.what to extract values from specific location
  results match
    report OK
  results don't match
    report problem
end loop

I would think that if this was slowly built up for each core module, then
future developers could simply use this as a testing mechanism which should
speed development.

Also, prior to release a master script could run each of these in turn
against the rc versions to make sure that the core functionality was still
working.

I realize this is labour intensive but have no suggestions as to a quicker
but still reliable method.

For myself, I doubt I will have little time to this, as I still have
v.buffer waiting to be fixed on my to do list.

T

Good ideas. It looks like Soren did some work on an excellent testsuite for
GRASS... but not sure how to use / extend it. Perhaps I can spend some time
on this after by thesis is done...

cheers,

Dylan

--
Dylan Beaudette
Soils and Biogeochemistry Graduate Group
University of California at Davis
530.754.7341