Dear Pieter-Jan,
Many thanks for spotting these issues with libcsironn. I have committed
fixes to SVN. See below for comments.
On Sun, Oct 19, 2008 at 08:55:43PM +0200, Pieter-Jan Busschaert wrote:
> Hello,
>
>
> I have recently been testing libcsironn (the version from plplot).
> I have come across 2 errors :
>
> 1) in trunk/lib/nn/delaunay.c : delaunay_circles_find()
>
> line 679 : if (tid < 0 || tid == nn) {
>
> should be changed to
>
> line 679 : if (tid < 0 || i == nn) {
>
>
> This bug leads to "random" results, ie: the result of the interpolation
> depended on the search just before.
This is a clear bug. I've commited your fix. To see the results, look at
example 21 before and after. At the bottom of the 3rd subplot (top
right) on page 2 you can see some differences. With this fix the odd
"blobs" have been removed and the results look more like the other
interpolation methods. They may be other differences - these were the
ones I spotted.
>
> 2) in trunk/lib/nn/nncommon.c : circle_contains()
>
> line 90 : return hypot(c->x - p->x, c->y - p->y) <= c->r;
>
> should be changed to
>
> line 90 : return hypot(c->x - p->x, c->y - p->y) < c->r + 0.000001;
>
>
> This is just a standard floating point equality checking bug. It surfaces
> when asking to interpolate on one of the given x,y coordinates. It gives the
> result of delaunay_xytoi returning a certain triangle for the point, but
> then the circle_contains() function says this point is no part of the
> circumcircle.
This is not quite a "standard" floating point equality check since we're
looking at <= not ==. Whether you consider this a bug depends on how
important it is that the == case is included, or that points ever so
slightly outside are excluded. Anyway, I have committed a slightly
modified version of your fix which checks against <= c->r*(1.0*EPSILON)
where EPSILON is 1.0e-8. This looks at the relative error since we don't
know what the magnitude of the radius will be. This makes no difference
(checked with diff) for example 21. Can you give an example where this
is actually important? Of course it will be architecture / compiler
dependent... The comments above this line suggest that quite a bit of
work went into finding this formulation to minimise such errors.
> I don't know if there is any "upstream" version of this library left that
> maintains this code ?
See Alan's answer.
Thanks
Andrew
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel
