Opened 15 years ago

Last modified 8 years ago

#763 new defect

r.statistics method=distribution: no percentages for negative basemap values

Reported by: peifer Owned by: grass-dev@…
Priority: normal Milestone: 6.4.6
Component: Raster Version: 6.4.0 RCs
Keywords: r.statistics Cc:
CPU: x86-32 Platform: Unspecified

Description

Here some sampe results:

   basemap covermap percentage

     -70       -7      inf
     -70       63      inf
     -50       -7      inf
     -50       45      inf

As far as I can see this is the relevant line in r.statistics/o_distrib.c (I am not a programmer, though)

68 	        if (catb != basecat && basecat > 0)

Change History (8)

comment:1 by neteler, 14 years ago

Could it be the lack of sufficient NULL detection?

See source:grass/branches/releasebranch_6_4/raster/r.statistics/o_distrib.c#L68

Or, for percentage, conditionalize and use absolute values as input?

Markus

in reply to:  1 ; comment:2 by glynn, 14 years ago

Replying to neteler:

Could it be the lack of sufficient NULL detection?

See source:grass/branches/releasebranch_6_4/raster/r.statistics/o_distrib.c#L68

I suspect that "basecat > 0" should be either "basecat != 0" or absent, depending upon whether category zero causes problems for some other component (e.g. r.stats).

in reply to:  2 ; comment:3 by peifer, 14 years ago

This is how I fixed it locally, for me:

--- o_distrib.c.save    2008-12-19 21:28:36.000000000 +0100
+++ o_distrib.c 2009-09-27 15:14:58.000000000 +0200
@@ -65,7 +65,8 @@
     total_count = 0;

     while (fscanf(fd1, "%ld %ld %ld", &basecat, &covercat, &area) == 3) {
-       if (catb != basecat && basecat > 0) {
+       if (catb != basecat) {
            if (fscanf(fd2, "%ld %ld", &cat, &total_count) != 2)
                return (1);
            catb = basecat;

A question, perhaps for Markus:
Why should cat=0 cause problems for r.stats or r.statistics? It is a legal value, by the end of the day. I also vaguely remember that some r.stats/r.statistics/r.report output doesn't show the counts for cat=0. At the time, my workaround was to reclassify 0 to 99999 or some such, in order to get correct statistcs.

in reply to:  3 ; comment:4 by glynn, 14 years ago

Replying to peifer:

Why should cat=0 cause problems for r.stats or r.statistics? It is a legal value, by the end of the day.

Prior to 5.x, category zero was often used as a no-data value. 5.x added support for a distinguished null value (as well as floating-point), but some code still follows the old zero-is-null convention. In particular:

  • If a raster map doesn't have a null bitmap, any zeros are converted to nulls upon read.
  • The functions G_get_map_row() and G_get_map_row_nomask() convert any nulls to zeros upon read, so any module which uses these functions cannot distinguish between zero and null.

Given the large number of occurrences of G_get_map_row[_nomask]() in 6.4, I can't help wondering whether all of the modules which were converted to use Rast_get_c_row() in 7.0 were actually converted correctly, or whether the function call was changed without any effort made to handle nulls.

in reply to:  4 comment:5 by hamish, 14 years ago

Keywords: r.statistics added

Replying to glynn:

Prior to 5.x, category zero was often used as a no-data value. 5.x added support for a distinguished null value (as well as floating-point), but some code still follows the old zero-is-null convention.

...

Given the large number of occurrences of G_get_map_row[_nomask]() in 6.4, I can't help wondering whether all of the modules which were converted to use Rast_get_c_row() in 7.0 were actually converted correctly, or whether the function call was changed without any effort made to handle nulls.

as a test point, r.surf.contour in 6.4 still treats 0 as NULL (and is not FP-aware), so if there are problems in the transition, they'll probably show up there.

in reply to:  4 comment:6 by peifer, 14 years ago

Replying to glynn:

...but some code still follows the old zero-is-null convention.


This is what I observed, from a simple end-user perspective. But actually, I didn't want to believe it and I never dared to create a ticket whenever I came across such an observation.

As stated: my current practice is to reclassify my maps so that all legitimate 0's (and negative values) are replaced by some more or less meaningful positive integer. Can you confirm that this continues to make sense, if one wants to be on the safe side?

comment:7 by neteler, 12 years ago

Milestone: 6.4.06.4.4

comment:8 by neteler, 8 years ago

Milestone: 6.4.46.4.6
Note: See TracTickets for help on using tickets.