Opened 14 years ago

Last modified 8 years ago

#1082 new defect

r.random cover= gets stray data in first and last rows

Reported by: hamish Owned by: grass-dev@…
Priority: normal Milestone: 6.4.6
Component: Raster Version: svn-develbranch6
Keywords: r.random Cc:
CPU: All Platform: All

Description

Hi,

spearfish:

g.region rast=elevation.dem
r.random in=elevation.dem cover=fields n=10000 rast=randout
WARNING: Only [6248] random points created

the resulting "randout" map is nicely filled with random dots but on the first and last row of the output raster map there are chunks of solid dots which are created no matter what you do.

a more obvious example:

r.random in=elevation.dem cover=fields n=5 rast=randout
Collecting Stats...
 100%
Writing raster map <randout> ...
WARNING: Only [3] random points created

# ok, so we should expect n=3 in the output map...
r.univar randout | grep -w 'n:'
n: 248

stray blocks of cells are in the same place regardless of n=.


also, for cover= it would be nice if random.c looped while(nt) so that it ran in multiple passes and you got as many points as you asked for, not less. (like v.random.cover addon does for points)

I thought about adding nt++ inside is_null_value(cover){...} in the column for-loop to counter-act the nt--, but I worry that may harm the overall randomness "density" of the rows for-loop. (just a hunch, I haven't proven that to myself)

thanks, Hamish

Change History (4)

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

CPU: x86-32All
Milestone: 6.4.06.4.1
Platform: LinuxAll

Replying to hamish:

the resulting "randout" map is nicely filled with random dots but on the first and last row of the output raster map there are chunks of solid dots which are created no matter what you do.

This caused by the loop "continue"ing on null, without setting the output to null. So if the input cell is null but the cover isn't null, it ends up unconditionally writing the cover value to the output.

This would be more obvious except that elevation.dem has so few nulls. (BTW, I think that base and cover are the wrong way around in the example. Either that or both r.random and r.statistics have them the wrong way around; I don't actually know which is correct).

Merging the null tests into the main conditional gets around this issue, although the nc-- needs to also be moved into the body of the test. This should be fixed with r43716 (trunk).

also, for cover= it would be nice if random.c looped while(nt) so that it ran in multiple passes and you got as many points as you asked for, not less.

The shortage of points is caused by it ignoring the number of nulls in the cover map when working out the probability of a given cell being chosen. The problem is, it can't work out the right answer because it counts the number of nulls individually for each of the two maps, regardless of whether they are disjoint or overlapping.

This one is a bit more problematic to fix, and if I start, I'm probably going to end up trying to fix everything else which is wrong with r.random (e.g. the code is far more complex than it has a right to be, due to trying to process the data in its native format rather than just using DCELL internally).

Also, the way that input= and cover= work is illogical; if you specify cover=, input= behaves as a base map; if you don't specify cover=, input= behaves as a cover map. It would make more sense for the options to be named base= and cover= or base= and input=.

comment:2 by hamish, 12 years ago

Milestone: 6.4.16.4.3

comment:3 by hamish, 11 years ago

Component: DefaultRaster

comment:4 by neteler, 8 years ago

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