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

[Helena]

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 at virgin.net>