Opened 5 years ago

Closed 15 months ago

Last modified 15 months ago

#2496 closed defect (fixed)

r.random.surface error from RAN C library

Reported by: cmbarton Owned by: grass-dev@…
Priority: normal Milestone: 7.0.7
Component: Raster Version: svn-trunk
Keywords: r.random.surface Cc:
CPU: Unspecified Platform: MacOSX

Description

We just encountered a weird bug in r.random.surface. So far, it seems to showing up on the Mac intermittently but not on Linux. The message is...

ERROR: RAN1: j==-67 shouldn't happen

It was generated from a Python script that called r.random.surface in the following way:

grass.run_command("r.random.surface", quiet = "True", output = tempimpactg, distance = grazespatial, exponent = grazepatchy, high = maxgrazeimpact)

I'm running on an oldish (ca. 5 years) iMac but with the latest OS X 10.9,x (no upgrade to 10.10 yet). This happens on both GRASS 7.0 beta3 and 7.1, both compiled 11 August 2014.

Attachments (1)

r.random.surface.patch (1.3 KB) - added by glynn 5 years ago.
ran1() replacement

Download all attachments as: .zip

Change History (23)

comment:1 Changed 5 years ago by annakrat

It comes from r.random.surface/random.c ran1 function, perhaps the new random functions could be used instead?

http://trac.osgeo.org/grass/browser/grass/trunk/lib/gis/lrand48.c

This applies also to r.random.cells. See also discussion here.

comment:2 Changed 5 years ago by cmbarton

I don't know. But this bug causes our modeling script to crash so that we get no results.

comment:3 Changed 5 years ago by glynn

That appears to be a conversion of this Fortran function.

That's supposed to generate uniform random numbers in the [0,1) interval, and so can be replaced by G_drand48().

comment:4 Changed 5 years ago by cmbarton

If this is an easy fix, can someone let us know when it gets fixed? We need this for landscape modeling experiments we are trying to run.

thanks Michael

comment:5 in reply to:  4 Changed 5 years ago by annakrat

Replying to cmbarton:

If this is an easy fix, can someone let us know when it gets fixed? We need this for landscape modeling experiments we are trying to run.

It would be most welcome if somebody from your group would submit a patch which can be reviewed and eventually submitted.

comment:6 Changed 5 years ago by cmbarton

Unfortunately, no one here is a C programmer. We do Python, Java, and other things. But not C. :-(

Otherwise we might have figured it out and fixed it already.

Michael

Changed 5 years ago by glynn

Attachment: r.random.surface.patch added

ran1() replacement

comment:7 in reply to:  3 Changed 5 years ago by glynn

Replying to glynn:

so can be replaced by G_drand48().

Can someone test attachment:r.random.surface.patch ?

comment:8 Changed 5 years ago by cmbarton

Is it updated in trunk or 7.1 dev? I can recompile and test.

Michael

comment:9 in reply to:  8 Changed 5 years ago by cmbarton

Replying to cmbarton:

Is it updated in trunk or 7.1 dev? I can recompile and test.

Michael

Sorry. I see it is a patch. I will try with 7.0 and let you know.

Michael

comment:10 Changed 5 years ago by cmbarton

This patch seems to fix the problem. I'll know for sure after our script repeated enough times. Most of the times, r.random.surface failed on the first or second time run. Once it did not fail until the 62nd iteration. I'm up to 65 iterations with no errors yet. So it looks good.

Michael

comment:11 Changed 5 years ago by neteler

Will you submit the patch?

comment:12 Changed 5 years ago by cmbarton

I tested this with trunk. While I can do the mechanics of submitting this later this week for 7.0 and trunk, I'm a little hesitant since I'm not familiar with C and don't know if this should be applied to other r.random.x modules or not.

Michael

comment:13 Changed 5 years ago by neteler

Indeed, these files are 99% cloned:

  • r.random.cells/gasdev.c
  • r.random.surface/gasdev.c

and

  • r.random.cells/random.c
  • r.random.surface/random.c

comment:14 Changed 5 years ago by cmbarton

So multiple patches are needed for different modules? Different for GRASS 7.0 and 7.1 or the same?

Michael

comment:15 in reply to:  14 Changed 5 years ago by neteler

Replying to cmbarton:

So multiple patches are needed for different modules? Different for GRASS 7.0 and 7.1 or the same?

The same patch will work in 7.0 and 7.1.

comment:16 Changed 5 years ago by cmbarton

So just a patch for each module that uses this then.

comment:17 Changed 4 years ago by martinl

Milestone: 7.0.07.0.5

comment:18 Changed 3 years ago by neteler

Milestone: 7.0.57.0.6

comment:19 Changed 21 months ago by neteler

Milestone: 7.0.67.0.7

comment:20 Changed 15 months ago by cmbarton

We hit this unfixed bug again and while an error trap would help, we really just need to change the documentation it seems.

r.random.surface output=surf generates the error r.random.surface output=surf seed=0 (or seed=ANYTHING) does not generate an error, though seed<0 generates a warning r.random.surface output=surf seed=- does NOT generate an error

comment:21 Changed 15 months ago by mmetz

Resolution: fixed
Status: newclosed

In 73234:

r.random.cells, r.random.surface: use G_drand48(), fixes #2496

comment:22 Changed 15 months ago by mmetz

In 73235:

r.random.cells, r.random.surface: use G_drand48(), fixes #2496 (backport trunk r73234)

Note: See TracTickets for help on using tickets.