Opened 7 years ago

Closed 7 years ago

#2020 closed defect (fixed)

r.volume gives wrong results on G7

Reported by: madi Owned by: grass-dev@…
Priority: normal Milestone: 7.0.0
Component: Raster Version: svn-trunk
Keywords: r.volume Cc:
CPU: x86-64 Platform: Linux

Description

simple test with r.volume. Small testing dataset here:

http://ubuntuone.com/2Odq6BDeQDdQhIJX2D8sHn

On Grass 6.4 {{{r.volume --overwrite data=slope clump=basin centroids=centroid1

Cat Average Data # Cells Centroid Total Number in clump Total in clump Easting Northing Volume

1 2.92 229610 78593 635195.00 220715.00 22961000.00

Total Volume = 22961000.00}}}

On Grass 7.0

{{{r.volume --overwrite input=slope clump=basin centroids=centroid1

Cat Average Data # Cells Centroid Total

Number in clump Total in clump Easting Northing Volume

1-57817810.36-4544075169558 78593 635195.00 220715.00

-454407516955800.00}}}

Change History (9)

comment:1 Changed 7 years ago by Nikos Alexandris

In grass64 I get,

Cat    Average   Data   # Cells        Centroid             Total
Number  in clump  Total  in clump   Easting   Northing       Volume

    1      3.41    268147   78593   635195.00   220715.00      26814700.00
                                            Total Volume =    26814700.00

in grass70,

 Cat    Average   Data   # Cells        Centroid             Total
Number  in clump  Total  in clump   Easting   Northing       Volume

    1-57817809.87-4544075131021   78593   635195.00   220715.00 -454407513102100.00

comment:2 Changed 7 years ago by madi

Nikos, thank you for testing. I had made several try, I must have exported a couple of maps that don't match with the result that I reported, but still the results between G6 and G7 are different. Thanks, madi

comment:3 Changed 7 years ago by hamish

Hi,

The problem is with the handling NULL cells in the input map, or rather not handling them. It's this line in main.c: sum[i] += data_buf[col]; Every now and then the value which is added is -2147483648 instead of in the range of ~ 0-36. That happens when the clump map exists but the input map does not. So for your test data the slope map is 1 cell smaller than the basins map around the edges of the area, and those cells which are non-NULL in the basins map but NULL in the slope map return corrupted values.

fwiw between devbr6 and trunk there don't seem to be any module changes beyond the conversion of G_() to Rast_() in the function names.

I notice even in grass7 it's still trying to make an old grass5 sites_list points map, and also that the input map is always opened and read as a CELL map, even when it is floating point, so the results will be.. less correct than they might otherwise be due to rounding/quantization errors. For 0.0-1.0 normalized data that might be fatal. (conversion of the input to int(map*1000) with r.mapcalc gives the same error for NULLs in 'sum' though)

Hamish

comment:4 in reply to:  3 Changed 7 years ago by mmetz

Replying to hamish:

The problem is with the handling NULL cells in the input map, or rather not handling them. It's this line in main.c: sum[i] += data_buf[col]; Every now and then the value which is added is -2147483648 instead of in the range of ~ 0-36. That happens when the clump map exists but the input map does not. So for your test data the slope map is 1 cell smaller than the basins map around the edges of the area, and those cells which are non-NULL in the basins map but NULL in the slope map return corrupted values.

fwiw between devbr6 and trunk there don't seem to be any module changes beyond the conversion of G_() to Rast_() in the function names.

I notice even in grass7 it's still trying to make an old grass5 sites_list points map, and also that the input map is always opened and read as a CELL map, even when it is floating point, so the results will be.. less correct than they might otherwise be due to rounding/quantization errors. For 0.0-1.0 normalized data that might be fatal. (conversion of the input to int(map*1000) with r.mapcalc gives the same error for NULLs in 'sum' though)

NULL handling fixed and fp support added to r.volume in trunk r56984.

Markus M

comment:5 Changed 7 years ago by madi

Hello Markus,

Thank you for that! Actually I still find that the results produced by G7 are different from the ones produced by G6. For the dataset attached, (setting the region to match slope), i get:

From G7:

Volume report on data from slope using clumps on basin map

 Cat    Average   Data   # Cells        Centroid             Total
Number  in clump  Total  in clump   Easting   Northing       Volume

    1      3.41    268000   78593   635195.00   220715.00      26799969.96
                                            Total Volume =    26799969.96

and G6

Volume report on data from slope using clumps on basin map

 Cat    Average   Data   # Cells        Centroid             Total
Number  in clump  Total  in clump   Easting   Northing       Volume

    1      3.43     29854    8715   635175.00   220725.00      26868600.00
                                            Total Volume =    26868600.00

Thanks, madi

comment:6 in reply to:  5 Changed 7 years ago by mmetz

Replying to madi:

Hello Markus,

Thank you for that! Actually I still find that the results produced by G7 are different from the ones produced by G6.

This is because r.volume in G7 has now floating point support and the slope map in your sample data is single precision floating point, thus correctly read only in G7. You can now file a bug for r.volume in G6.

comment:7 Changed 7 years ago by hamish

You can now file a bug for r.volume in G6.

we may as well continue with this ticket since it isn't too complicated. Is it ok to open a CELL map as DCELL in grass6, or do we need a switch statement to use the correct read/isnull commands?

if skipping adding to sum because of a null, don't we need to 'count--' too? or else for the average sum/n has too big an n. ?

thanks, Hamish

comment:8 in reply to:  7 Changed 7 years ago by mmetz

Replying to hamish:

You can now file a bug for r.volume in G6.

we may as well continue with this ticket since it isn't too complicated. Is it ok to open a CELL map as DCELL in grass6, or do we need a switch statement to use the correct read/isnull commands?

I think it is ok to open CELL or FCELL maps as DCELL in grass6, IIRC various modules do so already.

if skipping adding to sum because of a null, don't we need to 'count--' too? or else for the average sum/n has too big an n. ?

Right, fixed in r56987.

comment:9 Changed 7 years ago by madi

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.