[GRASS5] r.slope.aspect crash - libes/gis/null_data.c

GRASS GRASS developer
1

Hi,

today we realized a nice crash of r.slope.aspect:

r.slope.aspect elev pc=z.rsa.pcurf
percent complete: Segmentation fault (core dumped)

If you try the spearfish data:
r.slope.aspect elevation.dem pc=z.rsa.pcurf
percent complete: Segmentation fault (core dumped)

The debugger tells us that there is something strange in
libes/gis/null_data.c
Program received signal SIGSEGV, Segmentation fault.
0x8056635 in G_set_f_null_value (fcellVals=0x0, numVals=1) at null_val.c:295
/ssi0/ssi/neteler/cvsgrass/src/libes/gis/null_val.c:295:9157:beg:0x8056635
(gdb)

Unfortunately this doesn't tell me anything? Probably the NULL
implemenation is weird in null_val.c?

To our surprise the aspect/slope calculation runs well.

It would be nice to have that fixed...

Thanks,

Markus

2

Markus

I used r.slope.aspect for curvatures quite a bit and did not have a problem
(except that it does not write the custom color table as the rst programs do
which is annoying)
- I will test it too, but I want to ask whether you had a mask set, because I
might not
have used it with a mask.

Helena

(If you are computing it to visualize the waves along contours, I am covering
that in the applications, so do not worry about that right now)

Neteler wrote:

Hi,

today we realized a nice crash of r.slope.aspect:

r.slope.aspect elev pc=z.rsa.pcurf
percent complete: Segmentation fault (core dumped)

If you try the spearfish data:
r.slope.aspect elevation.dem pc=z.rsa.pcurf
percent complete: Segmentation fault (core dumped)

The debugger tells us that there is something strange in
libes/gis/null_data.c
Program received signal SIGSEGV, Segmentation fault.
0x8056635 in G_set_f_null_value (fcellVals=0x0, numVals=1) at null_val.c:295
/ssi0/ssi/neteler/cvsgrass/src/libes/gis/null_val.c:295:9157:beg:0x8056635
(gdb)

Unfortunately this doesn't tell me anything? Probably the NULL
implemenation is weird in null_val.c?

To our surprise the aspect/slope calculation runs well.

It would be nice to have that fixed...

Thanks,

Markus
_______________________________________________
grass5 mailing list
grass5@grass.itc.it
http://grass.itc.it/mailman/listinfo/grass5

3

(cc RT)

On Fri, Jan 18, 2002 at 11:09:36AM -0500, Helena Mitasova wrote:

Markus

I used r.slope.aspect for curvatures quite a bit and did not have a problem
(except that it does not write the custom color table as the rst programs do
which is annoying)
- I will test it too, but I want to ask whether you had a mask set, because I
might not have used it with a mask.
Helena

...no, I didn't:

GRASS:~ >g.list rast |grep MASK
GRASS:~ >

It can be even replicated with the Spearfish data.

Markus

(If you are computing it to visualize the waves along contours, I am covering
that in the applications, so do not worry about that right now)

Neteler wrote:

> Hi,
>
> today we realized a nice crash of r.slope.aspect:
>
> r.slope.aspect elev pc=z.rsa.pcurf
> percent complete: Segmentation fault (core dumped)
>
> If you try the spearfish data:
> r.slope.aspect elevation.dem pc=z.rsa.pcurf
> percent complete: Segmentation fault (core dumped)
>
> The debugger tells us that there is something strange in
> libes/gis/null_data.c
> Program received signal SIGSEGV, Segmentation fault.
> 0x8056635 in G_set_f_null_value (fcellVals=0x0, numVals=1) at null_val.c:295
> /ssi0/ssi/neteler/cvsgrass/src/libes/gis/null_val.c:295:9157:beg:0x8056635
> (gdb)
>
> Unfortunately this doesn't tell me anything? Probably the NULL
> implemenation is weird in null_val.c?
>
> To our surprise the aspect/slope calculation runs well.
>
> It would be nice to have that fixed...
>
> Thanks,
>
> Markus

4

Markus

when you give it both curvatures it runs, when you give it only pc curvature
it crashes. So this looks as another unfinished enhancement and genuine bug. I do
not have time to look at
it untill Feb.1, but somebody else may be able to fix it (and add the colortable
from rst too).

Helena

GRASS:~ > r.slope.aspect bh02dec01.t800 slope=slp.test aspect=asp.test pcur=pc.test

percent complete: Segmentation fault (core dumped)
GRASS:~ > r.slope.aspect bh02dec01.t800 slope=slp.test aspect=asp.test pcur=pc.test
tcur=tc.test
percent complete: 100%
CREATING SUPPORT FILES
ELEVATION PRODUCTS for mapset [helena] in [jockey-state]
min computed aspect 0.0019 max computed aspect 359.9997
Color table for [asp.test] set to aspect
ASPECT [asp.test] COMPLETE
min computed slope 0.0000 max computed slope 9.9895
SLOPE [slp.test] COMPLETE
PROFILE CURVE [pc.test] COMPLETE
TANGENTIAL CURVE [tc.test] COMPLETE

--------------------------------------
Neteler wrote:

(cc RT)

On Fri, Jan 18, 2002 at 11:09:36AM -0500, Helena Mitasova wrote:
> Markus
>
> I used r.slope.aspect for curvatures quite a bit and did not have a problem
> (except that it does not write the custom color table as the rst programs do
> which is annoying)
> - I will test it too, but I want to ask whether you had a mask set, because I
> might not have used it with a mask.
> Helena

...no, I didn't:

GRASS:~ >g.list rast |grep MASK
GRASS:~ >

It can be even replicated with the Spearfish data.

Markus

>
> (If you are computing it to visualize the waves along contours, I am covering
> that in the applications, so do not worry about that right now)
>
> Neteler wrote:
>
> > Hi,
> >
> > today we realized a nice crash of r.slope.aspect:
> >
> > r.slope.aspect elev pc=z.rsa.pcurf
> > percent complete: Segmentation fault (core dumped)
> >
> > If you try the spearfish data:
> > r.slope.aspect elevation.dem pc=z.rsa.pcurf
> > percent complete: Segmentation fault (core dumped)
> >
> > The debugger tells us that there is something strange in
> > libes/gis/null_data.c
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x8056635 in G_set_f_null_value (fcellVals=0x0, numVals=1) at null_val.c:295
> > /ssi0/ssi/neteler/cvsgrass/src/libes/gis/null_val.c:295:9157:beg:0x8056635
> > (gdb)
> >
> > Unfortunately this doesn't tell me anything? Probably the NULL
> > implemenation is weird in null_val.c?
> >
> > To our surprise the aspect/slope calculation runs well.
> >
> > It would be nice to have that fixed...
> >
> > Thanks,
> >
> > Markus
_______________________________________________
grass5 mailing list
grass5@grass.itc.it
http://grass.itc.it/mailman/listinfo/grass5

5

Markus Neteler wrote:

today we realized a nice crash of r.slope.aspect:

r.slope.aspect elev pc=z.rsa.pcurf
percent complete: Segmentation fault (core dumped)

If you try the spearfish data:
r.slope.aspect elevation.dem pc=z.rsa.pcurf
percent complete: Segmentation fault (core dumped)

The debugger tells us that there is something strange in
libes/gis/null_data.c
Program received signal SIGSEGV, Segmentation fault.
0x8056635 in G_set_f_null_value (fcellVals=0x0, numVals=1) at null_val.c:295
/ssi0/ssi/neteler/cvsgrass/src/libes/gis/null_val.c:295:9157:beg:0x8056635
(gdb)

Unfortunately this doesn't tell me anything? Probably the NULL
implemenation is weird in null_val.c?

The bug is in r.slope.aspect, not the libraries. The problem is with
the section of code between lines 925 and 942 in main.c:

    if (pcurv_fd > 0) {
      if (out_type == CELL_TYPE)
        *((CELL *) pcurv_ptr) = (CELL) (scik1 * pcurv);
      else
        G_set_raster_value_d(pcurv_ptr, (DCELL) pcurv, data_type);
    }
* else
* G_set_null_value (pcurv_ptr, 1, data_type);
* pcurv_ptr= G_incr_void_ptr(pcurv_ptr, G_raster_size(data_type));
                if (tcurv_fd > 0) {
                  if (out_type == CELL_TYPE)
                    *((CELL *) tcurv_ptr) = (CELL) (scik1 * tcurv);
                  else
                    G_set_raster_value_d(tcurv_ptr, (DCELL) tcurv, data_type);
                }
* else
* G_set_null_value (tcurv_ptr, 1, data_type);
* tcurv_ptr= G_incr_void_ptr(tcurv_ptr, G_raster_size(data_type));

The [pt]curv_ptr values are only valid if the corresponding
[pt]curv_fd value is also valid; otherwise, they are NULL, hence the
segfault.

Note that r.slope.aspect will also crash if you use "tc=..." without
using "pc=...".

Simply removing the "else" clauses will eliminate the crash (the
G_incr_void_ptr() calls should also be moved inside the conditional).
But I don't know if that's the "right" fix, or whether there should be
some additional logic (i.e. whether there's some case where
G_set_null_value() is supposed to be called).

--
Glynn Clements <glynn.clements@virgin.net>

6

[CC to GRASS5]

Helena wrote:

> The bug is in r.slope.aspect, not the libraries.

you are right

> The problem is with
> the section of code between lines 925 and 942 in main.c:
>
> if (pcurv_fd > 0) {
> if (out_type == CELL_TYPE)
> *((CELL *) pcurv_ptr) = (CELL) (scik1 * pcurv);

the above scik1*pcurv is a leftover from integer GRASS (that is how curvatures
were handled in s.surf.tps because they are usually between -1,0,1).
I believe that it can be removed (same for tcurv)

I'm not so sure; a quick test using elevation.dem from spearfish
indicates that the resulting values are likely to be much smaller than
one (e.g. +/- 0.03 for elevation.dem) so, without the scale factor, a
CELL map would be all zeroes.

This also applies to the dx/dy/dxx/dxy/dyy maps; they are all zero if
you use "prec=int". Only slope and aspect produce reasonable CELL
maps.

I am just curious why is there CELL type, when would it be CELL, shouldn't it be
always DCELL? (I hope that this question isn't too stupid)

The output type is an option:

    parm.out_precision = G_define_option() ;
    parm.out_precision->key = "prec";
    parm.out_precision->type = TYPE_STRING ;
    parm.out_precision->required = NO ;
    parm.out_precision->answer = "float";
    parm.out_precision->options = "default,double,float,int";
    parm.out_precision->description= "type of output aspect and slope maps" ;

Contrary to the description, this is used for all output maps.

So, should the scik1 scalar be used for dx/dy/dxx/dxy/dyy, or should
maps other than slope/aspect be forced to use FCELL/DCELL?

> Simply removing the "else" clauses will eliminate the crash (the
> G_incr_void_ptr() calls should also be moved inside the conditional).
> But I don't know if that's the "right" fix, or whether there should be
> some additional logic (i.e. whether there's some case where
> G_set_null_value() is supposed to be called).

I'm presuming that the code should match the various derivative maps,
so I'll fix it on that basis.

--
Glynn Clements <glynn.clements@virgin.net>

7

Glynn Clements wrote:

[CC to GRASS5]

Helena wrote:

> > The bug is in r.slope.aspect, not the libraries.
>
> you are right
>
> > The problem is with
> > the section of code between lines 925 and 942 in main.c:
> >
> > if (pcurv_fd > 0) {
> > if (out_type == CELL_TYPE)
> > *((CELL *) pcurv_ptr) = (CELL) (scik1 * pcurv);
>
> the above scik1*pcurv is a leftover from integer GRASS (that is how curvatures
> were handled in s.surf.tps because they are usually between -1,0,1).
> I believe that it can be removed (same for tcurv)

I'm not so sure; a quick test using elevation.dem from spearfish
indicates that the resulting values are likely to be much smaller than
one (e.g. +/- 0.03 for elevation.dem) so, without the scale factor, a
CELL map would be all zeroes.

that is why the output should be floats

This also applies to the dx/dy/dxx/dxy/dyy maps; they are all zero if
you use "prec=int". Only slope and aspect produce reasonable CELL
maps.

> I am just curious why is there CELL type, when would it be CELL, shouldn't it be
> always DCELL? (I hope that this question isn't too stupid)

The output type is an option:

    parm.out_precision = G_define_option() ;
    parm.out_precision->key = "prec";
    parm.out_precision->type = TYPE_STRING ;
    parm.out_precision->required = NO ;
    parm.out_precision->answer = "float";
    parm.out_precision->options = "default,double,float,int";
    parm.out_precision->description= "type of output aspect and slope maps" ;

Contrary to the description, this is used for all output maps.

So, should the scik1 scalar be used for dx/dy/dxx/dxy/dyy, or should
maps other than slope/aspect be forced to use FCELL/DCELL?

Oh, I did not know about that option as I always use command line with
r.slope.aspect).
Definitelly, everything other than slope and aspect should be float (including the
first order derivatives) maybe with option for double. That multiplication by scik1
was a pain because you could always found surfaces where you would get just zeroes
and then it also messes-up the units so you need to look into the manual
what scik1 is (if it is there at all.) to figure out your units.

Helena

> > Simply removing the "else" clauses will eliminate the crash (the
> > G_incr_void_ptr() calls should also be moved inside the conditional).
> > But I don't know if that's the "right" fix, or whether there should be
> > some additional logic (i.e. whether there's some case where
> > G_set_null_value() is supposed to be called).

I'm presuming that the code should match the various derivative maps,
so I'll fix it on that basis.

--
Glynn Clements <glynn.clements@virgin.net>

8

[CC to GRASS5]

Helena wrote:

in case that you would have time to add it,
this is that missing color table, it is used for both pcurv and tcurv
- currently the output for curvatures will just give you
blue map, so it appears that the output is incorrect.
c1min is minimum value for profile curvature and c2min is minimum for tangential -
c1max and c2max is the same for maximums.
This colortable could be used also for partial derivatives output but I assume
that whoever runs it with that option knows what is he doing and can chenge the
colortable
himself.

At present, the only colour table management performed by
r.slope.aspect is this:

        sprintf(buf, "r.colors map='%s' c=aspect",
    G_fully_qualified_name (aspect_name, G_mapset()));
  system(buf);

If there is supposed to be a "standard" colour table for gradient
and/or curvature maps, it should probably go into r.colors rather than
individual programs.

NB: the colour table in question is the following, from
src/libes/rst_gmsl/interp_float/output2d.c:

  MIN 127 0 255
  -0.01 0 0 255
  -0.001 0 127 255
  -0.00001 0 255 255
  0.0 200 255 200
  0.00001 255 255 0
  0.001 255 127 0
  0.01 255 0 0
  MAX 255 0 200

Also, might it be better to map the extremes to some fixed value?

It might be a good idea to have at least one standard colour table
which *doesn't* adjust to the range of the data, to facilitate a
common colouring scheme across multiple maps.

Such a colour table would have to conceptually range between +/-
infinity (although a finite range could be useful for DEMs); r.colors
would extract a suitable subrange by interpolating arctangents.

--
Glynn Clements <glynn.clements@virgin.net>

9

Glynn Clements wrote:

[CC to GRASS5]

Helena wrote:

> in case that you would have time to add it,
> this is that missing color table, it is used for both pcurv and tcurv
> - currently the output for curvatures will just give you
> blue map, so it appears that the output is incorrect.
> c1min is minimum value for profile curvature and c2min is minimum for tangential -
> c1max and c2max is the same for maximums.
> This colortable could be used also for partial derivatives output but I assume
> that whoever runs it with that option knows what is he doing and can chenge the
> colortable
> himself.

At present, the only colour table management performed by
r.slope.aspect is this:

        sprintf(buf, "r.colors map='%s' c=aspect",
                G_fully_qualified_name (aspect_name, G_mapset()));
        system(buf);

If there is supposed to be a "standard" colour table for gradient
and/or curvature maps, it should probably go into r.colors rather than
individual programs.

it would be very nice to have a greater selection of color tables in r.colors,
especially some histogram equalized colors and standardized color maps
for some often used maps (e.g. upslope area, which is generated by r.flow).

However, it is also good to have the color tables in the individual programs. Having
them
in the rst programs and r.flow has saved me already a lot of time and you
get a nice map right away. Some users may be confused by the output when
the standard color table is used (just try to assign it to curvatures or flowline
density or look at the accumulation map from r.watershed - all you get is blue
map and the user may not know that he is supposed to use r.color to get something
meaningful (nobody reads manuals as we all know).

NB: the colour table in question is the following, from
src/libes/rst_gmsl/interp_float/output2d.c:

        MIN 127 0 255
        -0.01 0 0 255
        -0.001 0 127 255
        -0.00001 0 255 255
        0.0 200 255 200
        0.00001 255 255 0
        0.001 255 127 0
        0.01 255 0 0
        MAX 255 0 200

Also, might it be better to map the extremes to some fixed value?

Originally I had it like that, but I changed it because it used to get out of range
easily - the curvatures can get pretty high, often just around a single point.

It might be a good idea to have at least one standard colour table
which *doesn't* adjust to the range of the data, to facilitate a
common colouring scheme across multiple maps.

I use r.colors mapnew rast=mapstandard for that (and slope and aspect
color maps in rst programs are designed like that). But such color tables
would be great for r.colors.
I wonder - we already have a quite few color tables in r.color, how many
more could we add before it starts to be "too many". I can think of some
that it would be nice to have there.

Such a colour table would have to conceptually range between +/-
infinity (although a finite range could be useful for DEMs); r.colors
would extract a suitable subrange by interpolating arctangents.

yes, there are maps which have known limited range (DEM, slope, aspect,
precipitation) and those where it is hard to predict so you may need to assume
inifinity to make it general enough.

Helena

--
Glynn Clements <glynn.clements@virgin.net>