Discussion:
[Community] Bug in Rtree
Christian Ledermann
2011-07-13 06:09:51 UTC
Permalink
from rtree import Rtree
tree = Rtree()
bounds = [0,0,1,1]
tree.add(1,bounds)
list(tree.intersection(bounds))
[1L]
tree.add(-1288508995,bounds)
list(tree.intersection(bounds))
[1L, 18446744072421042621L]
--
Best Regards,

Christian Ledermann

Nairobi - Kenya
Mobile : +254 702978914

<*)))>{

If you save the living environment, the biodiversity that we have left,
you will also automatically save the physical environment, too. But If
you only save the physical environment, you will ultimately lose both.

1) Don?t drive species to extinction

2) Don?t destroy a habitat that species rely on.

3) Don?t change the climate in ways that will result in the above.

}<(((*>
Sean Gillies
2011-07-13 15:34:10 UTC
Permalink
On Wed, Jul 13, 2011 at 12:09 AM, Christian Ledermann
Post by Christian Ledermann
from rtree import Rtree
tree = Rtree()
bounds = [0,0,1,1]
tree.add(1,bounds)
list(tree.intersection(bounds))
[1L]
tree.add(-1288508995,bounds)
list(tree.intersection(bounds))
[1L, 18446744072421042621L]
Hi Christian,

Libspatialindex uses unsigned longs and we've failed to document this
properly. If you are hashing objects to make integer ids, you'll need
to shift the values to be > 0.

In Vaytrou, I maintain a pair of OI and IO BTrees to map objects to a
positive integer id following the example of zope.intid.
--
Sean
Howard Butler
2011-07-13 15:42:42 UTC
Permalink
Post by Sean Gillies
On Wed, Jul 13, 2011 at 12:09 AM, Christian Ledermann
Post by Christian Ledermann
from rtree import Rtree
tree = Rtree()
bounds = [0,0,1,1]
tree.add(1,bounds)
list(tree.intersection(bounds))
[1L]
tree.add(-1288508995,bounds)
list(tree.intersection(bounds))
[1L, 18446744072421042621L]
Hi Christian,
Libspatialindex uses unsigned longs and we've failed to document this
properly. If you are hashing objects to make integer ids, you'll need
to shift the values to be > 0.
Sean,

This is my mistake in the libspatialindex API.

I see this in the SpatialIndex.h header:

typedef int64_t id_type;

but for some reason, I used uint64_t's for the ids and the compiler didn't warn us about that.

/me wonders how to/if to fix this gracefully.
Post by Sean Gillies
In Vaytrou, I maintain a pair of OI and IO BTrees to map objects to a
positive integer id following the example of zope.intid.
--
Sean
_______________________________________________
Community mailing list
Community at lists.gispython.org
http://lists.gispython.org/mailman/listinfo/community
Christian Ledermann
2011-07-14 08:58:38 UTC
Permalink
Post by Howard Butler
Post by Sean Gillies
Libspatialindex uses unsigned longs and we've failed to document this
properly. If you are hashing objects to make integer ids, you'll need
to shift the values to be > 0.
Sean,
This is my mistake in the libspatialindex API.
typedef int64_t id_type;
but for some reason, I used uint64_t's for the ids and the compiler didn't warn us about that.
/me wonders how to/if to fix this gracefully.
Post by Sean Gillies
In Vaytrou, I maintain a pair of OI and IO BTrees to map objects to a
positive integer id following the example of zope.intid.
I agree that this can be 'worked around' at the application level (for
the short term) but this is not how it should be.
Remember the olden days when bugs became features like the A20 gate? I
don't want to go up this road, I've been there before

I REALLY want to see this fixed upstream (in Midterm in Rtree and in
the long term in spatial index)


I agree with Howard (assuming i interpret him right) that an API
change of spatialindex should be delayed to the next major version
which has API changes.
--
Best Regards,

Christian Ledermann

Nairobi - Kenya
Mobile : +254 702978914

<*)))>{

If you save the living environment, the biodiversity that we have left,
you will also automatically save the physical environment, too. But If
you only save the physical environment, you will ultimately lose both.

1) Don?t drive species to extinction

2) Don?t destroy a habitat that species rely on.

3) Don?t change the climate in ways that will result in the above.

}<(((*>
Sean Gillies
2011-07-14 19:30:44 UTC
Permalink
Post by Howard Butler
Post by Sean Gillies
On Wed, Jul 13, 2011 at 12:09 AM, Christian Ledermann
Post by Christian Ledermann
from rtree import Rtree
tree = Rtree()
bounds = [0,0,1,1]
tree.add(1,bounds)
list(tree.intersection(bounds))
[1L]
tree.add(-1288508995,bounds)
list(tree.intersection(bounds))
[1L, 18446744072421042621L]
Hi Christian,
Libspatialindex uses unsigned longs and we've failed to document this
properly. If you are hashing objects to make integer ids, you'll need
to shift the values to be > 0.
Sean,
This is my mistake in the libspatialindex API.
typedef int64_t id_type;
but for some reason, I used uint64_t's for the ids and the compiler didn't warn us about that.
/me wonders how to/if to fix this gracefully.
Let's do fix this. Rtree users wouldn't notice the difference. How
many other users of the C API are there? I think I've seen more
questions about the C++ classes. Could we make a new C interface with
uint64_t shims for backwards compatibility, or is that too ugly?
--
Sean
Howard Butler
2011-07-14 20:59:13 UTC
Permalink
Post by Sean Gillies
Post by Howard Butler
Post by Sean Gillies
On Wed, Jul 13, 2011 at 12:09 AM, Christian Ledermann
Post by Christian Ledermann
from rtree import Rtree
tree = Rtree()
bounds = [0,0,1,1]
tree.add(1,bounds)
list(tree.intersection(bounds))
[1L]
tree.add(-1288508995,bounds)
list(tree.intersection(bounds))
[1L, 18446744072421042621L]
Hi Christian,
Libspatialindex uses unsigned longs and we've failed to document this
properly. If you are hashing objects to make integer ids, you'll need
to shift the values to be > 0.
Sean,
This is my mistake in the libspatialindex API.
typedef int64_t id_type;
but for some reason, I used uint64_t's for the ids and the compiler didn't warn us about that.
/me wonders how to/if to fix this gracefully.
Let's do fix this. Rtree users wouldn't notice the difference. How
many other users of the C API are there? I think I've seen more
questions about the C++ classes. Could we make a new C interface with
uint64_t shims for backwards compatibility, or is that too ugly?
We should rip the Bandaid off earlier rather than later. We should ask on the libspatialindex list if anyone is even using the C API other than us to hear what the impact might be. If no one speaks up, I'll just fix it.

Howard
Christian Ledermann
2011-07-15 08:37:12 UTC
Permalink
+1 :)
Post by Howard Butler
Post by Sean Gillies
Post by Howard Butler
Post by Sean Gillies
On Wed, Jul 13, 2011 at 12:09 AM, Christian Ledermann
Post by Christian Ledermann
from rtree import Rtree
tree = Rtree()
bounds = [0,0,1,1]
tree.add(1,bounds)
list(tree.intersection(bounds))
[1L]
tree.add(-1288508995,bounds)
list(tree.intersection(bounds))
[1L, 18446744072421042621L]
Hi Christian,
Libspatialindex uses unsigned longs and we've failed to document this
properly. If you are hashing objects to make integer ids, you'll need
to shift the values to be > 0.
Sean,
This is my mistake in the libspatialindex API.
typedef int64_t id_type;
but for some reason, I used uint64_t's for the ids and the compiler didn't warn us about that.
/me wonders how to/if to fix this gracefully.
Let's do fix this. Rtree users wouldn't notice the difference. How
many other users of the C API are there? I think I've seen more
questions about the C++ classes. Could we make a new C interface with
uint64_t shims for backwards compatibility, or is that too ugly?
We should rip the Bandaid off earlier rather than later. We should ask on the libspatialindex list if anyone is even using the C API other than us to hear what the impact might be. If no one speaks up, I'll just fix it.
Howard
_______________________________________________
Community mailing list
Community at lists.gispython.org
http://lists.gispython.org/mailman/listinfo/community
--
Best Regards,

Christian Ledermann

Nairobi - Kenya
Mobile : +254 702978914

<*)))>{

If you save the living environment, the biodiversity that we have left,
you will also automatically save the physical environment, too. But If
you only save the physical environment, you will ultimately lose both.

1) Don?t drive species to extinction

2) Don?t destroy a habitat that species rely on.

3) Don?t change the climate in ways that will result in the above.

}<(((*>
Sean Gillies
2011-07-15 23:15:34 UTC
Permalink
Post by Howard Butler
Post by Sean Gillies
Post by Howard Butler
Post by Sean Gillies
On Wed, Jul 13, 2011 at 12:09 AM, Christian Ledermann
Post by Christian Ledermann
from rtree import Rtree
tree = Rtree()
bounds = [0,0,1,1]
tree.add(1,bounds)
list(tree.intersection(bounds))
[1L]
tree.add(-1288508995,bounds)
list(tree.intersection(bounds))
[1L, 18446744072421042621L]
Hi Christian,
Libspatialindex uses unsigned longs and we've failed to document this
properly. If you are hashing objects to make integer ids, you'll need
to shift the values to be > 0.
Sean,
This is my mistake in the libspatialindex API.
typedef int64_t id_type;
but for some reason, I used uint64_t's for the ids and the compiler didn't warn us about that.
/me wonders how to/if to fix this gracefully.
Let's do fix this. Rtree users wouldn't notice the difference. How
many other users of the C API are there? I think I've seen more
questions about the C++ classes. Could we make a new C interface with
uint64_t shims for backwards compatibility, or is that too ugly?
We should rip the Bandaid off earlier rather than later. We should ask on the libspatialindex list if anyone is even using the C API other than us to hear what the impact might be. If no one speaks up, I'll just fix it.
Howard
I can confirm that with commits

https://github.com/Rtree/Rtree/commit/415ab33f26c1e2960d5c58b9471e0186dd102a4e
https://github.com/libspatialindex/libspatialindex/commit/787128ab5c7b00203efc10a08138ae0a39ce99ec
Post by Howard Butler
Post by Sean Gillies
from rtree import Rtree
Post by Howard Butler
tree = Rtree()
tree.add(-1, (0,0))
x3b2e68>
Post by Howard Butler
Post by Sean Gillies
Post by Howard Butler
list(tree.intersection((0, 0)))
[-1L]

Christian, can you try it out?
--
Sean
Christian Ledermann
2011-07-18 07:50:19 UTC
Permalink
trying to compile the current libspatialindex (git clone
https://github.com/libspatialindex/libspatialindex.git)

./autogen.sh
./configure
make

I get the errors in make

libtool: compile: g++ -DPACKAGE_NAME=\"spatialindex\"
-DPACKAGE_TARNAME=\"spatialindex-src\" -DPACKAGE_VERSION=\"1.6.1\"
"-DPACKAGE_STRING=\"spatialindex 1.6.1\""
-DPACKAGE_BUGREPORT=\"mhadji at gmail.com\" -DPACKAGE_URL=\"\"
-DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1
-DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1
-DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\"
-DPACKAGE=\"spatialindex-src\" -DVERSION=\"1.6.1\" -DHAVE_FCNTL_H=1
-DHAVE_UNISTD_H=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1
-DHAVE_PTHREAD_H=1 -DHAVE_SYS_RESOURCE_H=1 -DHAVE_SYS_TIME_H=1
-DHAVE_STDINT_H=1 -DHAVE_GETTIMEOFDAY=1 -DHAVE_BZERO=1 -DHAVE_MEMSET=1
-DHAVE_MEMCPY=1 -DHAVE_BCOPY=1 -I. -I../../include -Wall
-Wno-long-long -pedantic -std=c++98 -O2 -DNDEBUG -MT
RandomEvictionsBuffer.lo -MD -MP -MF .deps/RandomEvictionsBuffer.Tpo
-c RandomEvictionsBuffer.cc -fPIC -DPIC -o
.libs/RandomEvictionsBuffer.o
In file included from RandomEvictionsBuffer.cc:25:
../../include/tools/rand48.h:3: error: declaration of ?void
srand48(long int)? throws different exceptions
/usr/include/stdlib.h:409: error: from previous declaration ?void
srand48(long int) throw ()?
../../include/tools/rand48.h:4: error: declaration of ?short unsigned
int* seed48(short unsigned int*)? throws different exceptions
/usr/include/stdlib.h:410: error: from previous declaration ?short
unsigned int* seed48(short unsigned int*) throw ()?
../../include/tools/rand48.h:5: error: declaration of ?long int
nrand48(short unsigned int*)? throws different exceptions
/usr/include/stdlib.h:400: error: from previous declaration ?long int
nrand48(short unsigned int*) throw ()?
../../include/tools/rand48.h:6: error: declaration of ?long int
mrand48()? throws different exceptions
/usr/include/stdlib.h:404: error: from previous declaration ?long int
mrand48() throw ()?
../../include/tools/rand48.h:7: error: declaration of ?long int
lrand48()? throws different exceptions
/usr/include/stdlib.h:399: error: from previous declaration ?long int
lrand48() throw ()?
../../include/tools/rand48.h:8: error: declaration of ?void
lcong48(short unsigned int*)? throws different exceptions
/usr/include/stdlib.h:412: error: from previous declaration ?void
lcong48(short unsigned int*) throw ()?
../../include/tools/rand48.h:9: error: declaration of ?long int
jrand48(short unsigned int*)? throws different exceptions
/usr/include/stdlib.h:405: error: from previous declaration ?long int
jrand48(short unsigned int*) throw ()?
../../include/tools/rand48.h:10: error: declaration of ?double
erand48(short unsigned int*)? throws different exceptions
/usr/include/stdlib.h:396: error: from previous declaration ?double
erand48(short unsigned int*) throw ()?
../../include/tools/rand48.h:11: error: declaration of ?double
drand48()? throws different exceptions
/usr/include/stdlib.h:395: error: from previous declaration ?double
drand48() throw ()?
make[2]: *** [RandomEvictionsBuffer.lo] Error 1
make[2]: Leaving directory
`/home/ledermac/devel/spatialindex/libspatialindex/src/storagemanager'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory
`/home/ledermac/devel/spatialindex/libspatialindex/src'
make: *** [all-recursive] Error 1


Am i doing smthg wrong here (sorry I am new to git so maybe i am using
a wrong branch)?
Post by Sean Gillies
Post by Howard Butler
Post by Sean Gillies
Post by Howard Butler
Post by Sean Gillies
On Wed, Jul 13, 2011 at 12:09 AM, Christian Ledermann
Post by Christian Ledermann
from rtree import Rtree
tree = Rtree()
bounds = [0,0,1,1]
tree.add(1,bounds)
list(tree.intersection(bounds))
[1L]
tree.add(-1288508995,bounds)
list(tree.intersection(bounds))
[1L, 18446744072421042621L]
Hi Christian,
Libspatialindex uses unsigned longs and we've failed to document this
properly. If you are hashing objects to make integer ids, you'll need
to shift the values to be > 0.
Sean,
This is my mistake in the libspatialindex API.
typedef int64_t id_type;
but for some reason, I used uint64_t's for the ids and the compiler didn't warn us about that.
/me wonders how to/if to fix this gracefully.
Let's do fix this. Rtree users wouldn't notice the difference. How
many other users of the C API are there? I think I've seen more
questions about the C++ classes. Could we make a new C interface with
uint64_t shims for backwards compatibility, or is that too ugly?
We should rip the Bandaid off earlier rather than later. We should ask on the libspatialindex list if anyone is even using the C API other than us to hear what the impact might be. If no one speaks up, I'll just fix it.
Howard
I can confirm that with commits
https://github.com/Rtree/Rtree/commit/415ab33f26c1e2960d5c58b9471e0186dd102a4e
https://github.com/libspatialindex/libspatialindex/commit/787128ab5c7b00203efc10a08138ae0a39ce99ec
Post by Howard Butler
Post by Sean Gillies
from rtree import Rtree
Post by Howard Butler
tree = Rtree()
tree.add(-1, (0,0))
x3b2e68>
Post by Howard Butler
Post by Sean Gillies
Post by Howard Butler
list(tree.intersection((0, 0)))
[-1L]
Christian, can you try it out?
--
Sean
_______________________________________________
Community mailing list
Community at lists.gispython.org
http://lists.gispython.org/mailman/listinfo/community
--
Best Regards,

Christian Ledermann

Nairobi - Kenya
Mobile : +254 702978914

<*)))>{

If you save the living environment, the biodiversity that we have left,
you will also automatically save the physical environment, too. But If
you only save the physical environment, you will ultimately lose both.

1) Don?t drive species to extinction

2) Don?t destroy a habitat that species rely on.

3) Don?t change the climate in ways that will result in the above.

}<(((*>
Christian Ledermann
2011-07-18 08:25:19 UTC
Permalink
I can confirm Rtree works as expected :)
Post by Sean Gillies
Post by Howard Butler
import rtree
tree = rtree.Rtree()
tree.add(-1, (0,0))
list(tree.intersection((0, 0)))
[-1L]
Post by Sean Gillies
Post by Howard Butler
bounds = [0,0,1,1]
tree.add(-1288508995,bounds)
list(tree.intersection(bounds))
[-1L, -1288508995L]


I can also confirm that the new version or Rtree works fine with the
implementation of collective.geo.index
(all items are shown, the formerly 'missing' items too) without
rebuilding the catalog.

Also no incidents to report when i reindex the catalog- everything works fine :)
Post by Sean Gillies
Post by Howard Butler
Post by Howard Butler
Post by Sean Gillies
On Wed, Jul 13, 2011 at 12:09 AM, Christian Ledermann
Post by Christian Ledermann
from rtree import Rtree
tree = Rtree()
bounds = [0,0,1,1]
tree.add(1,bounds)
list(tree.intersection(bounds))
[1L]
tree.add(-1288508995,bounds)
list(tree.intersection(bounds))
[1L, 18446744072421042621L]
Hi Christian,
Libspatialindex uses unsigned longs and we've failed to document this
properly. If you are hashing objects to make integer ids, you'll need
to shift the values to be > 0.
Sean,
This is my mistake in the libspatialindex API.
typedef int64_t id_type;
but for some reason, I used uint64_t's for the ids and the compiler didn't warn us about that.
/me wonders how to/if to fix this gracefully.
Let's do fix this. Rtree users wouldn't notice the difference. How
many other users of the C API are there? I think I've seen more
questions about the C++ classes. Could we make a new C interface with
uint64_t shims for backwards compatibility, or is that too ugly?
We should rip the Bandaid off earlier rather than later. We should ask on the libspatialindex list if anyone is even using the C API other than us to hear what the impact might be. If no one speaks up, I'll just fix it.
Howard
I can confirm that with commits
https://github.com/Rtree/Rtree/commit/415ab33f26c1e2960d5c58b9471e0186dd102a4e
https://github.com/libspatialindex/libspatialindex/commit/787128ab5c7b00203efc10a08138ae0a39ce99ec
Post by Howard Butler
from rtree import Rtree
Post by Howard Butler
tree = Rtree()
tree.add(-1, (0,0))
x3b2e68>
Post by Howard Butler
Post by Howard Butler
list(tree.intersection((0, 0)))
[-1L]
Christian, can you try it out?
--
Sean
_______________________________________________
Community mailing list
Community at lists.gispython.org
http://lists.gispython.org/mailman/listinfo/community
--
Best Regards,

Christian Ledermann

Nairobi - Kenya
Mobile : +254 702978914

<*)))>{

If you save the living environment, the biodiversity that we have left,
you will also automatically save the physical environment, too. But If
you only save the physical environment, you will ultimately lose both.

1) Don?t drive species to extinction

2) Don?t destroy a habitat that species rely on.

3) Don?t change the climate in ways that will result in the above.

}<(((*>
Loading...