[GRASS5] r.contour crashes if step=0.5

Hi developers,

r.contour crashes if step=0.5... Floating point exception.
I actually cannot see the problem. The offending line is a macro:

  dmax = (max->answer) ? atof (max->answer) : zmax - (int)zmax%(int)dstep;

dstep contains 0.5, zmax is determined my the program zmax=7.1231 here).

Thanks in advance

Markus

----------------------------------------
If you want to unsubscribe from GRASS Development Team mailing list write to:
minordomo@geog.uni-hannover.de with
subject 'unsubscribe grass5'

Roger,

[cc to grass5]
On Sat, Jan 27, 2001 at 12:48:44PM +0100, Roger Bivand wrote:

On Sat, 27 Jan 2001, Markus Neteler wrote:

> Hi developers,
>
> r.contour crashes if step=0.5... Floating point exception.
> I actually cannot see the problem. The offending line is a macro:
>
> dmax = (max->answer) ? atof (max->answer) : zmax - (int)zmax%(int)dstep;
>
> dstep contains 0.5, zmax is determined my the program zmax=7.1231 here).
>

It's the (int) cast on dstep, which becomes integer 0, and which gives a
NaN when used for division. For this code to work, dstep must be >= 1. It
looks a bit cludgy too.

I see. But in my case I have a map with pH values ranging from 5.5 to
7.3. So setting "step" to 1 doesn't produce exciting contours.
Even step is a double. Se we might need to change this line (and the next
in main.c of r.contours)?

Thanks for your quick reply

Markus

----------------------------------------
If you want to unsubscribe from GRASS Development Team mailing list write to:
minordomo@geog.uni-hannover.de with
subject 'unsubscribe grass5'

On Sat, 27 Jan 2001, Markus Neteler wrote:

Roger,

[cc to grass5]
On Sat, Jan 27, 2001 at 12:48:44PM +0100, Roger Bivand wrote:
> On Sat, 27 Jan 2001, Markus Neteler wrote:
>
> > Hi developers,
> >
> > r.contour crashes if step=0.5... Floating point exception.
> > I actually cannot see the problem. The offending line is a macro:
> >
> > dmax = (max->answer) ? atof (max->answer) : zmax - (int)zmax%(int)dstep;
> >
> > dstep contains 0.5, zmax is determined my the program zmax=7.1231 here).
> >
>
> It's the (int) cast on dstep, which becomes integer 0, and which gives a
> NaN when used for division. For this code to work, dstep must be >= 1. It
> looks a bit cludgy too.

I see. But in my case I have a map with pH values ranging from 5.5 to
7.3. So setting "step" to 1 doesn't produce exciting contours.
Even step is a double. Se we might need to change this line (and the next
in main.c of r.contours)?

Sorry to "fake" this in R, but it's the integer casts that cause the
problem:

dstep <- 0.5
max <- NULL
min <- NULL
zmax <- 7.3
zmin <- 5.5
zmax %% dstep

[1] 0.3

zmin %% dstep

[1] 0

dmax <- ifelse(!is.null(max), as.numeric(max),

zmax - (as.integer(zmax) %% as.integer(dstep)))

dmax

[1] NA

dmax <- ifelse(!is.null(max), as.numeric(max),

zmax - (zmax %% dstep))

dmax

[1] 7

dmin <- ifelse(!is.null(min), as.numeric(min),

ifelse(as.integer(zmin) %% as.integer(dstep),
zmin - (as.integer(zmin) %% as.integer(dstep)), zmin))

dmin

[1] NA

dmin <- ifelse(!is.null(min), as.numeric(min),

ifelse(zmin %% dstep, zmin - (zmin %% dstep), zmin))

dmin

[1] 5.5

If you set max and min for your contours, you wouldn't hit the bug, nor if
you set the levels manually. Whether removing the casts is OK later on is
another question, but it looks as though there are no more (int) casts,
so I'd take out the casts in the assignments to dmax and dmin.

Roger

--
Roger Bivand
Economic Geography Section, Department of Economics, Norwegian School of
Economics and Business Administration, Breiviksveien 40, N-5045 Bergen,
Norway. voice: +47 55 95 93 55; fax +47 55 95 93 93
e-mail: Roger.Bivand@nhh.no
and: Department of Geography and Regional Development, University of
Gdansk, al. Mar. J. Pilsudskiego 46, PL-81 378 Gdynia, Poland.

----------------------------------------
If you want to unsubscribe from GRASS Development Team mailing list write to:
minordomo@geog.uni-hannover.de with
subject 'unsubscribe grass5'

Roger,

On Sat, Jan 27, 2001 at 01:38:02PM +0100, Roger Bivand wrote:

On Sat, 27 Jan 2001, Markus Neteler wrote:
> On Sat, Jan 27, 2001 at 12:48:44PM +0100, Roger Bivand wrote:
> > On Sat, 27 Jan 2001, Markus Neteler wrote:
> >
> > > Hi developers,
> > >
> > > r.contour crashes if step=0.5... Floating point exception.
> > > I actually cannot see the problem. The offending line is a macro:
> > >
> > > dmax = (max->answer) ? atof (max->answer) : zmax - (int)zmax%(int)dstep;
> > >
> > > dstep contains 0.5, zmax is determined my the program zmax=7.1231 here).
> >
> > It's the (int) cast on dstep, which becomes integer 0, and which gives a
> > NaN when used for division. For this code to work, dstep must be >= 1. It
> > looks a bit cludgy too.
>
> I see. But in my case I have a map with pH values ranging from 5.5 to
> 7.3. So setting "step" to 1 doesn't produce exciting contours.
> Even step is a double. Se we might need to change this line (and the next
> in main.c of r.contours)?

Sorry to "fake" this in R, but it's the integer casts that cause the
problem:

> dstep <- 0.5
> max <- NULL
> min <- NULL
> zmax <- 7.3
> zmin <- 5.5
> zmax %% dstep
[1] 0.3
> zmin %% dstep
[1] 0
> dmax <- ifelse(!is.null(max), as.numeric(max),
zmax - (as.integer(zmax) %% as.integer(dstep)))
> dmax
[1] NA
> dmax <- ifelse(!is.null(max), as.numeric(max),
zmax - (zmax %% dstep))
> dmax
[1] 7
> dmin <- ifelse(!is.null(min), as.numeric(min),
ifelse(as.integer(zmin) %% as.integer(dstep),
zmin - (as.integer(zmin) %% as.integer(dstep)), zmin))
> dmin
[1] NA
> dmin <- ifelse(!is.null(min), as.numeric(min),
ifelse(zmin %% dstep, zmin - (zmin %% dstep), zmin))
> dmin
[1] 5.5

If you set max and min for your contours, you wouldn't hit the bug, nor if
you set the levels manually. Whether removing the casts is OK later on is
another question, but it looks as though there are no more (int) casts,
so I'd take out the casts in the assignments to dmax and dmin.

It seems that C behaves differently than "R".
The compiler doesn't accept modulo with doubles:

gcc -g -O2 -I/home/neteler/ggg/src/include -c main.c -o
OBJ.i686-pc-linux-gnu/main.o
main.c: In function 'getlevels':
main.c:233: invalid operands to binary %
main.c:234: invalid operands to binary %
main.c:235: invalid operands to binary %

You are right, if I add max= and min= the problem of course disappears.
Maybe we can get rid of this macro? To be honest, I just don't get
the idea of it. Do we need it (like that)?

Thanks for looking into this,

Markus

----------------------------------------
If you want to unsubscribe from GRASS Development Team mailing list write to:
minordomo@geog.uni-hannover.de with
subject 'unsubscribe grass5'

On Sat, 27 Jan 2001, Markus Neteler wrote:

Roger,

On Sat, Jan 27, 2001 at 01:38:02PM +0100, Roger Bivand wrote:
> On Sat, 27 Jan 2001, Markus Neteler wrote:
> > On Sat, Jan 27, 2001 at 12:48:44PM +0100, Roger Bivand wrote:
> > > On Sat, 27 Jan 2001, Markus Neteler wrote:
> > >
> > > > Hi developers,
> > > >
> > > > r.contour crashes if step=0.5... Floating point exception.
> > > > I actually cannot see the problem. The offending line is a macro:
> > > >
> > > > dmax = (max->answer) ? atof (max->answer) : zmax - (int)zmax%(int)dstep;
> > > >
> > > > dstep contains 0.5, zmax is determined my the program zmax=7.1231 here).
> > >
> > > It's the (int) cast on dstep, which becomes integer 0, and which gives a
> > > NaN when used for division. For this code to work, dstep must be >= 1. It
> > > looks a bit cludgy too.
> >
It seems that C behaves differently than "R".
The compiler doesn't accept modulo with doubles:

gcc -g -O2 -I/home/neteler/ggg/src/include -c main.c -o
OBJ.i686-pc-linux-gnu/main.o
main.c: In function 'getlevels':
main.c:233: invalid operands to binary %
main.c:234: invalid operands to binary %
main.c:235: invalid operands to binary %

You are right, if I add max= and min= the problem of course disappears.
Maybe we can get rid of this macro? To be honest, I just don't get
the idea of it. Do we need it (like that)?

I think we need the fmod(x,y) function (K&R2: floating point remainder of
x/y, same sign as x; y=0 needs to be trapped, p. 251 in my edition). So:

dmax = (max->answer) ? atof (max->answer) : zmax - (int)zmax%(int)dstep;

could be

dmax = (max->answer) ? atof (max->answer) : dstep == 0 ?
        G_error(***) : zmax - fmod(zmax, dstep);

though I haven't tried it.

Roger

--
Roger Bivand
Economic Geography Section, Department of Economics, Norwegian School of
Economics and Business Administration, Breiviksveien 40, N-5045 Bergen,
Norway. voice: +47 55 95 93 55; fax +47 55 95 93 93
e-mail: Roger.Bivand@nhh.no
and: Department of Geography and Regional Development, University of
Gdansk, al. Mar. J. Pilsudskiego 46, PL-81 378 Gdynia, Poland.

----------------------------------------
If you want to unsubscribe from GRASS Development Team mailing list write to:
minordomo@geog.uni-hannover.de with
subject 'unsubscribe grass5'

Roger,

great!

On Sat, Jan 27, 2001 at 02:26:32PM +0100, Roger Bivand wrote:

On Sat, 27 Jan 2001, Markus Neteler wrote:
> On Sat, Jan 27, 2001 at 01:38:02PM +0100, Roger Bivand wrote:
> > On Sat, 27 Jan 2001, Markus Neteler wrote:
> > > On Sat, Jan 27, 2001 at 12:48:44PM +0100, Roger Bivand wrote:
> > > > On Sat, 27 Jan 2001, Markus Neteler wrote:
> > > > >
> > > > > r.contour crashes if step=0.5... Floating point exception.
> > > > > I actually cannot see the problem. The offending line is a macro:
> > > > >
> > > > > dmax = (max->answer) ? atof (max->answer) : zmax - (int)zmax%(int)dstep;
> > > > >
> > > > > dstep contains 0.5, zmax is determined my the program zmax=7.1231 here).
> > > >

[...]

I think we need the fmod(x,y) function (K&R2: floating point remainder of
x/y, same sign as x; y=0 needs to be trapped, p. 251 in my edition). So:

dmax = (max->answer) ? atof (max->answer) : zmax - (int)zmax%(int)dstep;

could be

dmax = (max->answer) ? atof (max->answer) : dstep == 0 ?
        G_error(***) : zmax - fmod(zmax, dstep);

though I haven't tried it.

I have rewritten the other macro accordingly:
Old:
dmin = (min->answer) ? atof (min->answer) : (int)zmin%(int)dstep?
                              zmin - (int)zmin%(int)dstep +dstep: zmin;

New:
dmin = (min->answer) ? atof (min->answer) : dstep == 0 ?
                   G_fatal_error("This step value is not allowed.") :
                   fmod(zmin,dstep) ? zmin - fmod(zmin,dstep) +dstep: zmin;

And it seems to produce reasonable results:

Old macro with fp-exception if min/max not specified:
r.contour in=ph_upper out=ph_upper step=0.1 min=5.64 max=7.19
Reading data.
Percent complete: 100%
FPRange of data: min = 5.647517 max = 7.195973
Minimum level will be 5.740000
Maximum level will be 7.190000
Continue?(y/n) [y]

New version:
New:
r.contour in=ph_upper out=ph_upper step=0.1
Reading data.
Percent complete: 100%
FPRange of data: min = 5.647517 max = 7.195973
Minimum level will be 5.700000
Maximum level will be 7.100000
Continue?(y/n) [y]

New:
r.contour in=ph_upper out=ph_upper step=0.1 min=5.64 max=7.19
Reading data.
Percent complete: 100%
FPRange of data: min = 5.647517 max = 7.195973
Minimum level will be 5.740000
Maximum level will be 7.190000
Continue?(y/n) [y]

r.contour in=ph_upper out=ph_upper step=0.1 min=5.647 max=7.196
Reading data.
Percent complete: 100%
FPRange of data: min = 5.647517 max = 7.195973
Minimum level will be 5.747000
Maximum level will be 7.096000
Continue?(y/n) [y]
Total levels: 15 Current level:
Finished.

It's in CVS now.
Thanks Roger, many users like will me enjoy this fix.

Markus

----------------------------------------
If you want to unsubscribe from GRASS Development Team mailing list write to:
minordomo@geog.uni-hannover.de with
subject 'unsubscribe grass5'

On Sat, 27 Jan 2001, Markus Neteler wrote:

Roger,

great!

On Sat, Jan 27, 2001 at 02:26:32PM +0100, Roger Bivand wrote:
[ .... ]

> I think we need the fmod(x,y) function (K&R2: floating point remainder of
> x/y, same sign as x; y=0 needs to be trapped, p. 251 in my edition). So:
>
> dmax = (max->answer) ? atof (max->answer) : zmax - (int)zmax%(int)dstep;
>
> could be
>
> dmax = (max->answer) ? atof (max->answer) : dstep == 0 ?
> G_error(***) : zmax - fmod(zmax, dstep);
>
> though I haven't tried it.

I have rewritten the other macro accordingly:
Old:
dmin = (min->answer) ? atof (min->answer) : (int)zmin%(int)dstep?
                              zmin - (int)zmin%(int)dstep +dstep: zmin;

New:
dmin = (min->answer) ? atof (min->answer) : dstep == 0 ?
                   G_fatal_error("This step value is not allowed.") :
                   fmod(zmin,dstep) ? zmin - fmod(zmin,dstep) +dstep: zmin;

And it seems to produce reasonable results:

Old macro with fp-exception if min/max not specified:
r.contour in=ph_upper out=ph_upper step=0.1 min=5.64 max=7.19
Reading data.
Percent complete: 100%
FPRange of data: min = 5.647517 max = 7.195973
Minimum level will be 5.740000
Maximum level will be 7.190000
Continue?(y/n) [y]

New version:
New:
r.contour in=ph_upper out=ph_upper step=0.1
Reading data.
Percent complete: 100%
FPRange of data: min = 5.647517 max = 7.195973
Minimum level will be 5.700000
Maximum level will be 7.100000
Continue?(y/n) [y]

OK!

New:
r.contour in=ph_upper out=ph_upper step=0.1 min=5.64 max=7.19
Reading data.
Percent complete: 100%
FPRange of data: min = 5.647517 max = 7.195973
Minimum level will be 5.740000
Maximum level will be 7.190000
Continue?(y/n) [y]

Also OK!

r.contour in=ph_upper out=ph_upper step=0.1 min=5.647 max=7.196
Reading data.
Percent complete: 100%
FPRange of data: min = 5.647517 max = 7.195973
Minimum level will be 5.747000
Maximum level will be 7.096000
Continue?(y/n) [y]
Total levels: 15 Current level:
Finished.

This is a bit odd, though. It's being post-processed by the subsequent
code to bring the max & min within the FPRange - ideally, the levels
should be "pretty" numbers, say for a 0.1 step breaking on 0.1, 0.2, ...,
or 0.15, 0.25, .... But that's what's in the code ...

Roger

--
Roger Bivand
Economic Geography Section, Department of Economics, Norwegian School of
Economics and Business Administration, Breiviksveien 40, N-5045 Bergen,
Norway. voice: +47 55 95 93 55; fax +47 55 95 93 93
e-mail: Roger.Bivand@nhh.no
and: Department of Geography and Regional Development, University of
Gdansk, al. Mar. J. Pilsudskiego 46, PL-81 378 Gdynia, Poland.

----------------------------------------
If you want to unsubscribe from GRASS Development Team mailing list write to:
minordomo@geog.uni-hannover.de with
subject 'unsubscribe grass5'