-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A single CMake file with all build information #490
Conversation
A new and improved CMake file. Improvements: - Replacement of all CMake files to a single file in order to ease understanding and maintenance (this avoids several files scattered in the repository). - Environment variables (PATH, PYTHONPATH, NTA_ROOTDIR, etc) are set in own CMake file. - Now it use the own Swig executable and dependencies located on External folder. Before this, I used 'find_packages' which could set an incompatible version for Nupic. - Now it use the system python, i.e. the python package called from shell natively (similar to what Autotool version does). Before this, I used 'find_packages' which could set wrong python interpreter or libraries. Build result: SUCCESS (including Swig modules!) Tests results: HtmTest: SUCCESS in all tests!! TestEverything: 99.9107 %, only 1 (out of 2238) test failed: get spec of the dynamic library (i.e. libcpp_region). I have worked hard to find out what is causing this, but without success.. :-( When I ignore TRY statement, it states that is not possible get a symbol from swig wrapped library: dlopen(/Volumes/Data/Users/davidragazzi/nta/eng/lib/python2.7/site-packages/nupic/bindings/_math.so, 2): Symbol not found: __ZN14PySparseTensor3setERK13PyTensorIndexP7_object Referenced from: /Volumes/Data/Users/davidragazzi/nta/eng/lib/python2.7/site-packages/nupic/bindings/_math.so Expected in: flat namespace in /Volumes/Data/Users/davidragazzi/nta/eng/lib/python2.7/site-packages/nupic/bindings/_math.so If some numenta expert could debug this, it would be great!
@breznak Now I believe we have a (99,9%) working version to use. Please test it and let me know any questions. It would be interesting if you or other numenta expert could debug the issue mentioned. I've checked all compiler, linker flags, includes, but the error remains. Maybe you find out a missing piece that I'm not realising it.. :-( Best, David |
Hi @david-ragazzi - thanks for doing this! I am intrigued by CMake - it looks like a very clean way to do things. With regards to the SparseTensor errors - it's very hard to tell. I do know that file uses a lot of complex template stuff so that might be getting in the way. However I'm pretty sure we don't use SparseTensor anywhere anymore. Could someone do a scan to see where we might be using it? If it is not being used I am ok removing it entirely from NuPIC. Maybe @breznak or someone else could create a separate PR to remove that, and that should resolve this issue too. |
Hi @david-ragazzi , the cmake solution looks really pretty clear! :) I've spotted two (hope minor) problems: 1/ compiler identification: (py27)[marek@beruska cmake_build]$ cmake .. I have both gcc and clang avaliable, the C/CXX checks have found gcc, then the clang check has found clang. I haven't specified $CXX, but I think it would be good to default to gcc on linux. When I run make, it's clang that gets used. 2/ libpython problem: from the configure: Python 2.7.6 The file Which is causing the compilation error later: Otherwise it seems to be running fine, and even the swig seems compiling, that's 👍 Thank you very much, Mark |
Namaste @subutai ! My intuition says that LD_LIBRARY_PATH could not be well configured (i.e. pointing to lib/python2.*/site_packages/bindings folder), so TestEverything executable could be not able to load dynamically these swig libraries. The problem is that set environment variables on OSX is very tricky because there are several scopes where they can or not be visible (a single 'export' on shell script cannot be enough to a env variable be visible on any xcode solution). As Windows has a simplified form of manage env variables, I'll test this behavior on it. |
Hi @breznak ! 1/ compiler identification: 2/ libpython problem: |
OH yeah!!! I finally found the error! I simply forgot add PySparseTensor.cpp as extra source file to _math swig library. Now all tests are 100%!!! After much work, we finally can have a clean and simple build process. @subutai and @breznak: |
@david-ragazzi That's great news! Way to go! |
Thanks @rhyolight ! I have thought in some files that could be removed in a full clean-up:
The source folder will be perfectly clean after this clean-up, and new members will understand the build process only reading this file. Once that comunity members do not relate issues with CMake build, we will feel confident in to do this clean-up. |
Kudos to you David! |
Thanks @sjmackenzie ! |
Woooo! --Please excuse brevity, sent from phone.--
|
Thanks @iandanforth ! @rhyolight : (...) Currently supported platforms:
Dependencies:
The dependencies are included in platform-specific repositories for convenience:
Complete set of python requirements are documented in requirements.txt, compatible with pip: pip install -r external/common/requirements.txt Build and test NuPIC: Command line (using 'make'): Generate build files: Build the generated files: Run the C++ tests: Graphical interface: Generate an IDE solution:
Build the solution:
Run the C++ tests:
(...) Please let me know if is ok... Best, David PS: |
@david-ragazzi Thanks! Would you mind putting the README updates into a separate PR we can look at after this one is merged? I will probably want to help out with that. |
@rhyolight Of course, I will create a PR for this. Update: |
Very nice @david-ragazzi ! |
Thanks @subutai ! |
Hi @david-ragazzi , I tested the file you sent me (which is slightly different to this one) and my above error is fixed 👍 thanks! There's still some issue during compilation: Linking CXX shared module /home/marek/nta/eng/lib/python2.7/site-packages/nupic/bindings/_algorithms.so ...looks like OS-specific flags to the linker (?) PS: sorry to always bring problems :) Cheers, breznak! |
# Second, for multi-config builds (DEBUG or RELEASE, for example). | ||
# | ||
set(CMAKE_SWIG_OUTDIR "${PYTHON_SITE_PACKAGES_DIR}/nupic/bindings") | ||
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_INSTALL_PREFIX}/bin) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the bin/, lib/ and build/ directories should then be added to .gitignore
file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely.
@breznak I'll will check this issue. |
@david-ragazzi the new file you sent me runs just FINE!! 👍 👍 Thank you a lot. Notes: |
👍 Ok, so it's time to ask you more a favor! hehe I changed default directories structure according to our @subutai conversation. Basically the CMake script configure $NUPIC and $NTA with default locations to the same repository dir BUT in case of the user already have these variables set on his system, it simply re-use them. This increases flexibility. A sample of the code: .# ... .# .. Note that 'release' will be created in the parent directory of source dir. In soon, we would have to move some directories (in official nupic repository) to a new directory called 'source'. But this only can be done, after autotools process is replaced by CMake process. I still dont test this version (I'm not on my computer now), so if you can help me, I would be thankful. :-) I'll send it by email.. |
New version of CMake file (now working on Linux): Update: |
Is this the version with structure changes as Subutai suggested? I'll test it, so I'm supposted to create new folders source/, release/ and move everything in current $NUPIC to source? |
Yes!
Yes, would be great! Please move everything (except README.md, LICENSE.txt, and other files that you consider pertinent) to /source and set $NUPIC env variable to reference it. Also if you have find the /docs folder mentioned by Subutai please move it to /repository root. Very thanks!! |
Suggestions applied and created new version of this file in #514 |
A new and improved CMake file.
Improvements:
Build result: SUCCESS (including Swig modules!)
Tests results:
HtmTest: SUCCESS in all tests!!
TestEverything: 99.9107 %, only 1 (out of 2238) test failed: get spec of the dynamic library (i.e. libcpp_region).
I have worked hard to find out what is causing this, but without success.. :-( When I ignore TRY statement, it states that is not possible get a symbol from swig wrapped library:
dlopen(/Volumes/Data/Users/davidragazzi/nta/eng/lib/python2.7/site-packages/nupic/bindings/_math.so, 2): Symbol not found: __ZN14PySparseTensor3setERK13PyTensorIndexP7_object
Referenced from: /Volumes/Data/Users/davidragazzi/nta/eng/lib/python2.7/site-packages/nupic/bindings/_math.so
Expected in: flat namespace
in /Volumes/Data/Users/davidragazzi/nta/eng/lib/python2.7/site-packages/nupic/bindings/_math.so
If some numenta expert could debug this, it would be great!