lohedges / aabbcc

Dynamic AABB trees in C++ with support for periodic systems.
Other
321 stars 60 forks source link

Python Wrapper Build Fail on OSX 10.12.2 #2

Closed joel-simon closed 7 years ago

joel-simon commented 7 years ago
$ make PYTHON=3.4 python
--> Building Python wrapper
../src/AABB.cc:172:41: error: no viable overloaded '='
        else                periodicity = {false, false, false};
                            ~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/4.2.1/bits/stl_bvector.h:516:5: note: candidate function not
      viable: cannot convert initializer list argument to 'const std::vector<bool,
      std::allocator<bool> >'
    operator=(const vector& __x)
    ^
../src/AABB.cc:1105:13: warning: unused variable 'height' [-Wunused-variable]
        int height = 1 + std::max(height1, height2);
            ^
1 warning and 1 error generated.
error: command '/usr/bin/clang' failed with exit status 1
make: *** [python] Error 1

Let me know if I can provide anything else. Thanks.

lohedges commented 7 years ago

Hi there,

Note that while the Makefile is set to use g++ by default, Apple actually have the cheek to alias g++ to clang++. This means that you'll be using LLVM rather than GNU, which comes with a different version of the standard library. To see this try running:

g++ --version

Recent versions of OS X now use libc++ rather than libstdc++; the former is maintained by LLVM, the latter by GNU. There are some slight inconsistencies in the way in which they handle C++11 features, see e.g.

Apparently the version of libstdc++ that Apple ship isn't very recent, so it doesn't fully support C++11.

If you have gcc installed alongside clang (perhaps through MacPorts), could you try the following and report back as to what works?

(Try running make clobber before each command to ensure that you are rebuilding everything from scratch.)

make PYTHON=3.4 CXX=clang++ OPTFLAGS="-stdlib=libc++" devel
make PYTHON=3.4 CXX=clang++ OPTFLAGS="-stdlib=libstdc++" devel
make PYTHON=3.4 CXX=g++ devel

I am using Linux, but have both clang and libc++ installed alongside gcc. All of the above work for me. (I have pretty much the latest versions for all packages.)

If it still doesn't work, you might need to add -stdlib=libc++ or -stdlib=libstdc++ to the extra_compile_args array in the python setup file, python/setup.py.

Cheers.

lohedges commented 7 years ago

Hi again,

I was just doing a quick web search and found this. If you're still having problems, could you possibly try replacing the contents of python/aabb.i with the following:

%module aabb

%{
#include "../src/AABB.h"
%}

%include "../src/AABB.h"
%include "std_vector.i"

%inline %{
typedef bool Bool;
typedef double Double;
typedef unsigned int UnsignedInt;
%}

%template(BoolVector) std::vector<Bool>;
%template(DoubleVector) std::vector<Double>;
%template(UnsignedIntVector) std::vector<unsigned int>;

This still works fine for me on Linux, but might fix the issue with compiling the Swig generated CXX on OS X.

joel-simon commented 7 years ago

Hi lohedges,

Thank you for the fast and thorough response!

Unfortunately I tried all combinations of aabb.i file, extra_compile_args and command line option to no success. This is out of my c++ expertise but I'll try playing around with other options as well and let you know if anything else works. I'm hoping to compare this to a cython spatial hash for large number of segment-segments intersection (about 300k) (similar sized)

Thanks again!

lohedges commented 7 years ago

Ah, bummer!

Another thought is that distutils is always using clang, regardless of whether gcc is specified in the Makefile, or on the command line. You can override this in python/setup.py by doing something like the following:

import os
os.environ["CXX"] = "g++-4.9"

(Using whichever version of g++ you have, assuming that it is recent enough for c++11 support.)

I'll see if I can source an OS X machine at some point so I can have a play around myself.

Cheers.

lohedges commented 7 years ago

For simplicity, we can just remove all instances of initializer lists. You should be able to replace lines 169-172 of src/AABB.cc with:

// Initialise the periodicity vector.
periodicity.resize(dimension);
std::fill(periodicity.begin(), periodicity.end(), false);

Let me know if that fixes things and I'll update the code.

joel-simon commented 7 years ago

Hi lohedges, The os.environ did not work but replacing lines 169 -172 did! Thank you very much.

I have now had a chance to play around with it. For my purpose it was slower than the spatial hash but my situation is perhaps better suited for spatial hashes anyway (large number of all similar sized bounding boxes in close proximity). Although the overhead of creating the vector and aabb objects in cython may be part of the performance issue.

Anyway, I had some questions about the interface. Is there a reason insert returns a treenode that is not accessed anywhere else?

Using particle index for other methods doesn't makes sense to me because of removals. It seems to give it an ID equal to the current number of particles. It would probably be better to accept a key like the interface for r* tree http://toblerity.org/rtree/tutorial.html#creating-an-index

cheers!

lohedges commented 7 years ago

Great, glad it's finally working!

For my purpose it was slower than the spatial hash but my situation is perhaps better suited for spatial hashes anyway (large number of all similar sized bounding boxes in close proximity).

Yes, that's probably likely. It really comes into its own for sparse or inhomogeneous systems, or when there's a large size disparity between objects.

Have you looked into "cell lists" that are often used in molecular simulation? These are similar to a spatial hash, but there are some fancy tricks for checking overlaps quickly, and for maintaining the data structures when objects move or are inserted or deleted. There's an example in the source code of my vmmc repository if you're interested.

Thanks for your suggestions regarding the interface. Some comments below:

Is there a reason insert returns a treenode that is not accessed anywhere else?

This is something that I used for debugging in an early version of the code. As you say, it is never used. I've updated the code to remove the return value.

Using particle index for other methods doesn't makes sense to me because of removals. It seems to give it an ID equal to the current number of particles. It would probably be better to accept a key.

I originally wrote the code for fixed particle number simulations and only thought about removals late on. I agree that the particle index doesn't work as it should in this situation. I've updated to the code so that the particle index (its key) is now used to create a map between particles and nodes. Any time a particle is removed from the system, the corresponding key-value pair is removed from the map. You can now use any key for the particle (as long as it's an unsigned int) rather than it being assumed that particles are always indexed in a sequential ascending order.

Cheers.

joel-simon commented 7 years ago

Glad to hear my suggestions were a somewhat helpful.

I agree about best use cases, I'll probably have an application that needs it soon.

Thank you for the suggestion into cell lists and sample code, I wasn't familiar with them but look forward to reading into them. I have a system of deformable polygons than cannot self/other intersect so I need to do lots of segment-segments intersection tests.

very useful repo, cheers! joel