Skip to content
This repository has been archived by the owner on Sep 1, 2023. It is now read-only.

Independent Build Process #33

Closed
wants to merge 21 commits into from
Closed

Independent Build Process #33

wants to merge 21 commits into from

Conversation

david-ragazzi
Copy link
Contributor

This commit includes the compilation of the dynamic/static libraries
that compose nupic.core. They are generated by default on
nupic.core/build/release folder, however any user could change its
destination using cmake -DPROJECT_BUILD_RELEASE_DIR=… or cmake -DPROJECT_BUILD_TEMP_DIR=… command line.
Thought this option, Travis at nupic repo will compile the static
libraries and put them on /externalfolder.

The output of this library is a static library called nupic.core (I linked with HtmTest, TesteEverything, and all is ok.. :-) ). I'm also trying make CMake generate a dynamic library, but without success (problems with dynamic linking).

Note that neither HtmTest not TestEverything were included, they still
are hybrid and Python tests need be removed from them.

This PR replaces this:
#32

fixes #4

David Ragazzi and others added 4 commits March 1, 2014 21:37
This commit includes the compilation of the dynamic/static libraries
that compose nupic.core. They are generated by default on
nupic.core/build/release folder, however any user could change its
destination using "cmake -DPROJECT_BUILD_RELEASE_DIR=…" command line.
Thought this option, Travis at nupic repo will compile the static
libraries and put them on /external folder.
Note that neither HtmTest not TestEverything was included, they still
are hybrid and Python test need be removed from them.
@david-ragazzi
Copy link
Contributor Author

Hi Matt,

Actually CMake 2.8.8 have a new keyword called OBJECT which is used within add_library. It means that libraries are created virtually but doesn't generate physical files like *.lib or *.a.

We could remove OBJECT keyword (in other words, remove generate_object_library macro and continue using generate_static_library macro). But I would recommend update Travis-CI to using CMake 2.8.8 (if possible).

@david-ragazzi
Copy link
Contributor Author

Note the main ouput is nupic.core which is all in one static library that compile (again) the sources of all smaller libraries like libos, libengine, etc.. This is palliative. I will try linking it with these static libraries not reuse their source files. Ah.. and I say "palliative", because although this works fine, it's weird see several files not grouped in XCode project.. But this is not hinder we merge this PR..

@rhyolight
Copy link
Member

I created a ticket against Travis for this. A workaround would be to update the .travis.yml file to build and install cmake before the actual build starts, but I don't like that idea because of the extra build time. But for a stop-gap fix, we might have to do it.

@david-ragazzi
Copy link
Contributor Author

Hi Matt, regarding to Travis script, the only artifact that need be saved is the build/release folder. It contains nupic.core api.

@subutai
Copy link
Member

subutai commented Mar 6, 2014

@david-ragazzi Cool. I haven't been following the details lately, but does that directory also have the required header files?

@david-ragazzi
Copy link
Contributor Author

@subutai You're right. I simply forgot include header files once it is a library to be linked. My mistake. Well, we could write a code to get all header files and put them in /build/release/include folder.

@subutai
Copy link
Member

subutai commented Mar 6, 2014

OK, thanks. We don't have the formal API defined yet, so it's hard to know exactly what files are required. One issue is that the nupic files may refer to them using paths like #include <nta/algorithms/Cells4.hpp>

@david-ragazzi
Copy link
Contributor Author

One issue is that the nupic files may refer to them using paths like #include <nta/algorithms/Cells4.hpp>

Yeah, this way just we copy header's folder to /external/common/include/nupic.core at nupic. So we would have to replace this:

#include <nta/algorithms/Cells4.hpp>

to:

#include <nupic.core/algorithms/Cells4.hpp>

@david-ragazzi
Copy link
Contributor Author

Don't worry Subutai, I compiled nupic linking against nupic.core.a (of course, doing the replacements above) and everything worked fine!

@breznak
Copy link
Member

breznak commented Mar 6, 2014

On Thu, Mar 6, 2014 at 12:10 PM, david-ragazzi notifications@github.comwrote:

One issue is that the nupic files may refer to them using paths like
#include

Yeah, this way just we copy header's folder to
/external/common/include/nupic.core at nupic. So we would have to replace
this:

#include <nta/algorithms/Cells4.hpp>

to:

#include <nupic.core/algorithms/Cells4.hpp>

or we could call /external/common/include/nupic.core ->
/external/common/include/nta
instead

@david-ragazzi
Copy link
Contributor Author

or we could call /external/common/include/nupic.core -> /external/common/include/nta instead

Yeah, at first moment yes, but I would consider strongly use nupic.core instead nta, it's more intuitive.. However, in order to agilize this process, we could continue using nta.. But certainly renaming nta to nupic.core could be a future ticket (after everything is stable).. I even don't know what is the meaning of the nta acronym!

@david-ragazzi
Copy link
Contributor Author

@rhyolight Up to the moment, the following artifacts at nupic.core need be saved:

  • /build/release/include : which nupic must download its content to its /external/common/include/nta folder.
  • /build/release/lib : which nupic must be download its content to its /external/your_os/lib folder.

In other words, the entire /build/release need to be saved.

@subutai
Copy link
Member

subutai commented Mar 6, 2014

Yeah, at first moment yes, but I would consider strongly use nupic.core instead nta, it's more intuitive.

How about nupic? It's shorter but also intuitive. I don't think we have to be tied to the repository name. So include <nupic/algorithms/Cells4.hpp>

@david-ragazzi
Copy link
Contributor Author

How about nupic? It's shorter but also intuitive. I don't think we have to be tied to the repository name. So include <nupic/algorithms/Cells4.hpp>

Could be, but in this case, I still would prefer nupic.core, Nupic platform has several subprojects and so we have specific names would a good pratice (in my opinion, of course). This is good to differentiate it from nupic.cerebro, nupic.tools, etc..If a software use more than one of these Nupic subprojects, the headers directives could confuse developers! In other words, it would fine we have:

  • include <nupic.core/algorithms/Cells4.hpp>
  • include <nupic.cerebro/algorithms/XXXXX.hpp>

@rhyolight
Copy link
Member

Yes, I prefer nupic.core to nupic.

@subutai
Copy link
Member

subutai commented Mar 7, 2014

Yes, I prefer nupic.core to nupic.

OK.

@david-ragazzi
Copy link
Contributor Author

@rhyolight Have you managed update CMake at Travis to 2.8.8? Otherwise, I'll try remove object libraries to status is green.

@rhyolight
Copy link
Member

@david-ragazzi I won't be able to get it done today. It involves compiling a 64b binary of cmake in the proper environment, uploading it to S3, and downloading it with the build script to install it. I think it will be a lot less work to remote the object libraries for now.

@david-ragazzi
Copy link
Contributor Author

Here we go, glentemen.. Travis is green with my 2 last commits..

@breznak I added the same warning flags from your CMake file PR at Nupic

(Curiosly my fork is red because build is presenting error at GCC environment, at least CLang passed in all tests.. I hope that my Travis file is missing something...)

@david-ragazzi
Copy link
Contributor Author

Update:

This weekend I managed isolate pynode tests from htmtest and testeverything and now these tests are pure c++ tests (which is a good news). I moved them to nupic.core (although them also can be copied to nupic and called from it). Today I'll check what's is the problem with pynode. After all ok, they will be a small set within of nupic. In future, they will be moved to nupic.core.python as this repo is directly related to python binding from nupíc.core.

@david-ragazzi
Copy link
Contributor Author

@rhyolight

I ended working on my forks in order to avoid confusion.. After everything fine, I'll compare the repos indicated by you and pull a request..

The problem is that I had deleted my fork and forked them again which makes your permissions get lost.. I gave permissions again to you and now you can follow everything what is happening...

The branchs I'm working:
Nupic: https://github.com/david-ragazzi/nupic (master)
Nupic.Core: https://github.com/david-ragazzi/nupic.core (master)

@david-ragazzi
Copy link
Contributor Author

Yes, that was my suggestion a few days ago. We could have a version of the tests that don't run the pynode tests and those could be in nupic.core. We would have a second version in nupic that runs the pynode tests. The second set of tests still need to pass though when run from the nupic repo. Will that be a problem?

@subutai I'm working on this! After everything ok, it will be a very small set of tests within nupic. In future, a suggestion would be we move it to nupic.core.python (numenta/nupic-legacy#724) as these pynode tests are directly related to the python binding from nupic.core.

@subutai
Copy link
Member

subutai commented Mar 17, 2014

@david-ragazzi Thanks David. Your suggestion makes sense. Let us know if you need any help with debugging, etc.

@rhyolight
Copy link
Member

The problem is that I had deleted my fork and forked them again which makes your permissions get lost.. I gave permissions again to you and now you can follow everything what is happening...

The branchs I'm working:
Nupic: https://github.com/david-ragazzi/nupic (master)
Nupic.Core: https://github.com/david-ragazzi/nupic.core (master)

Ok, I think this has Travis-CI all confused, and it is not running builds against your new fork. Can you close this PR and create a new one from your master branch?

@rhyolight
Copy link
Member

@david-ragazzi The good thing is that Travis is building off your pushes to your master branch:

https://travis-ci.org/david-ragazzi/nupic.core

But this PR is not getting status updates from Travis from this, probably because what you mentioned earlier (deleting and recreating your fork).

@rhyolight
Copy link
Member

FYI: Travis has gotten confused, and won't update the Github status for any PRs associated with @david-ragazzi's fork. So ignore the status on this PR and watch Travis here: https://travis-ci.org/david-ragazzi/nupic.core

@rhyolight
Copy link
Member

So @david-ragazzi, it looks like you are 1/2 there. Travis build is building and running tests in clang, but not gcc:

screen shot 2014-03-17 at 12 01 09 pm

@david-ragazzi
Copy link
Contributor Author

@rhyolight

So @david-ragazzi it looks like you are 1/2 there. Travis build is building and running tests in clang, but not gcc:

I had told this in other message.. Curiosily it passed in other branchs...

@david-ragazzi
Copy link
Contributor Author

Update:

A bad news:

I still didn't fix pynode issues.

An excellent news:

Yesterday I was able to compile nupic.core as all-in-one static library. Among benefits:

  • One that wish consume nupic.core API, doesn't need other external libraries like libapr.a, libyaml.a, etc, just the nupic.core.a is enough to be linked against executables or other dyn libraries.
  • Many files at external folder at nupic can be deleted (actually they were moved to nupic.core), as nupic.core already encapsulated all static libraries located in its own external folder.

In other words, now nupic.core is stand-alone indeed.

@sjmackenzie
Copy link

Congratulations!

@rhyolight
Copy link
Member

@david-ragazzi That's great! I guess you have these changes locally, right? Because I don't see any new builds running on travis today. Can you push your changes to your remote so we can see what happens with Travis?

@david-ragazzi
Copy link
Contributor Author

Well, I finally found what was problem with PyNode.. I had said that nupic.core passed on all tests when was compiled directly by nupic cmake file, but not when was compiled by nupic.core cmake file. The question is that I had removed all python flags (NTA_SUPPORT_PYTHON, for example) from nupic.core cmake file. Unfortunatelly, some code at nupic STILL has python support (RegionImplFactory.cpp, for example) and use these Python flags at runtime.

In other words, a few python flags should be passed to nupic.core from nupic cmake file untill all python support be definitily removed (I don't feel skilled enough to remove this support because this integration still is a little confuse to me). Anyway, this doesn't hinder those programmers that want consume a pure C++ API without have python installed on their computers, but this is a "white elephant" within nupic.core..

@david-ragazzi
Copy link
Contributor Author

Can you push your changes to your remote so we can see what happens with Travis?

Hi @rhyolight , ASAP I have a more stable build process I push them.. I feel that the current state could hinder any conclusions.. :-(

@rhyolight
Copy link
Member

@david-ragazzi Well ok. But no one can help you if you don't push. 😕

@david-ragazzi
Copy link
Contributor Author

@rhyolight You're right.. I just push last changes to nupic and nupic.core :

Nupic: https://github.com/david-ragazzi/nupic (master)
Nupic.Core: https://github.com/david-ragazzi/nupic.core (master)

@david-ragazzi
Copy link
Contributor Author

Ok, when I passed some py flags to nupic.core CMake (extra_cxxflags and extra_linkflags)
"Matching Python module for py.TestNode not found."

But I now I get:


Davids-iMac:~ davidragazzi$ ~/desktop/nupic/build/release/bin/htmtest
Creating network...
Region count is 0
Adding a PyNode region...
ERROR:  Could not get valid spec for Region: py.TestNode [/Volumes/Data/Users/davidragazzi/Desktop/nupic/nta/src/main/engine/RegionImplFactory.cpp line 446]
Exception: Could not get valid spec for Region: py.TestNode at: /Volumes/Data/Users/davidragazzi/Desktop/nupic/nta/src/main/engine/RegionImplFactory.cpp:446

Some idea??

@david-ragazzi
Copy link
Contributor Author

Ok, PyNode tests finally passed. I had to create a CMake file again from CMake at nupic being careful on not excluding any compiler flags.. And it works! Now I will commit from scratch to ease the review work (if I pull alll now, github won't show changes individually, and you will have dificulty to know that was changed).

@rhyolight
Copy link
Member

@david-ragazzi That's great, David. Sorry I haven't been keeping up with your progress this week. Been busy with other tasks. Let us know what we can do to help you.

@david-ragazzi
Copy link
Contributor Author

@rhyolight . Yes, it seems the things are working now! Anyway, I just would ask you (team) review and merge the PRs as soon as possible where these get done (I'll communicate when this happen). The problem is that I'm restructuring directories, and this will cause conflicts with existent PRs (bad reference to a file), otherwise we will have rework..

@rhyolight
Copy link
Member

@david-ragazzi Just to be totally clear, the PRs you want us to review have not been created yet, is that right?

@david-ragazzi
Copy link
Contributor Author

@david-ragazzi Just to be totally clear, the PRs you want us to review have not been created yet, is that right?

Yes, I created just now! #47

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Establish independent build process
5 participants