numenta / nupic-legacy

Numenta Platform for Intelligent Computing is an implementation of Hierarchical Temporal Memory (HTM), a theory of intelligence based strictly on the neuroscience of the neocortex.
http://numenta.org/
GNU Affero General Public License v3.0
6.34k stars 1.56k forks source link

Implement new directory structure for nupic #624

Closed rhyolight closed 10 years ago

rhyolight commented 10 years ago

As discussed in https://github.com/numenta/nupic/issues/591.

david-ragazzi commented 10 years ago

As many folders at nupic.core repo will be moved from root nupic.core/nta to nupic.core/nta/src subfolder, will we have problems with any file that references the source files at nupic.core/nta?

Thinking of this, maybe the @sjmackenzie suggestion makes sense, i.e. first check if nupic repo runs ok with CMake and then we remove the Autotools stuff. My fear is changing the structure at nupic.core but the building of support software get problems due to dependency of Makefiles that references /nta and not /nta/src...

sjmackenzie commented 10 years ago

Well, there is no nupic.core/nta/src it is only nupic.core/src.

As many folders at nupic.core repo will be moved from root nupic.core/nta to nupic.core/nta/src subfolder, will we have problems with any file that references the source files at nupic.core/nta?

subutai commented 10 years ago

@david-ragazzi Did you mean nupic/nta/src? Yeah, probably there will be issues. The python bindings code references the code in nupic/nta Also the htmtest and testeverything apps probably do as well. There may be others. Maybe as you guys suggest it might be easier to first move nupic to CMake??

subutai commented 10 years ago

Or is there another way to have nupic/nta point to nupic.core/src?

sjmackenzie commented 10 years ago

Well I'm under the impression an ambitious change is under way. Way too ambitious for me, so I retreated from the conversation after giving my warnings.

I understand this to be happening: change core dir structure and getting it to compile into a static/dynamic lib via CMake, then getting nupic to talk to this lib.

If you're feeling brave go for it! Though someone pointed out that nupic is a python project that should be built by python conventions. I can't fault this, but I would transition to python default building approaches after core has been separated.

Personally I'd keep it simple, make sure nupic has moved over to CMake. Completely remove autotools, in this time I'd make light cosmetic changes ie remove nta. Now simply doing this you allows nupic and nupic.core to build independently of each other AND on all platforms. THEN change the folder structure (ie src, tests, doc, etc). Then separate core from nupic (this should now be painless). Now at this stage all that is needed is to change the way nupic looks for the core lib once you completely separate it out. One could, download the git folder via cmake and build it, or search for an already installed lib on the path. At this stage there are many ways of doing it. This bin isn't an issue.

The above would be my preferred way of doing it. Though there might be better ways! I am unable to share my opinion of the current direction as I don't understand it. So here is an opportunity to learn as I'm always aiming to improve myself by learning from others. In other words Subutai I can't answer your question as I cannot see the starting and finishing point of this work unit.

On February 12, 2014 2:31:50 AM GMT+08:00, Subutai Ahmad notifications@github.com wrote:

Or is there another way to have nupic/nta point to nupic.core/src?


Reply to this email directly or view it on GitHub: https://github.com/numenta/nupic/issues/624#issuecomment-34788081

Kind regards Stewart

Please excuse my typos and brevity

david-ragazzi commented 10 years ago

@subutai said:

Maybe as you guys suggest it might be easier to first move nupic to CMake??

Yes, after consider these issues, yes.

The reasons pointed by @sjmackenzie were the same I had when I faced these issues. So my opinion is continue the test with CMake located at nupic repo and split it into 2 (nupic and nupic.core) later. It's easier update a single file like CMake file than transverse an entire repo looking for build files spread over it.

Well, I think we have 2 options:

The truth is that current CMake file at nupic repo already works and it was created for replace both autotools and shell scripts. nupic.core extraction happened but it is there and works although few people use it.

And again, after everything being managed by CMake, the updates will be easier and transition will be smooth.

sjmackenzie commented 10 years ago

@david-ragazzi

Its best to split out CMakeFiles over every directory and not one monolith file :-)

It's easier update a single file like CMake file than tranverse an entire repo looking for build files spread over it.

david-ragazzi commented 10 years ago

Its best to split out CMakeFiles over every directory and not one monolith file :-)

@sjmackenzie

Maybe along the time we could to do it, but for now it's better leaving the things as simple as possible..

rhyolight commented 10 years ago

So the decision is to put off this directory restructuring until we have CMake integrated into nupic then?

sjmackenzie commented 10 years ago

You're doing the implementation! You decide! :-)

On Tue, Feb 11, 2014 at 10:12 PM, Matthew Taylor notifications@github.comwrote:

So the decision is to put off this directory restructuring until we have CMake integrated into nupic then?

— Reply to this email directly or view it on GitHubhttps://github.com/numenta/nupic/issues/624#issuecomment-34807440 .

rhyolight commented 10 years ago

Alright, depends on #516.

david-ragazzi commented 10 years ago

Ok, I'll focus at nupic repo, but I ask a special favor. When I pull CMakeLists.txt, Readme.md, and Travis.yaml, please give priority this demand. I already noted that someone changed build system again (TestEverything app changes the way to include the tests)..

My PRs remain for weeks in stack which makes the other developers change the build system and so the CMake file get problems. Once CMake is running ok, we should force the community to use it. How? Updating Readme.md and deleting or renaming build.sh (so developers won't try use old build system and again crash the new build system).

So as nupic.core extraction is priority, also give priority to CMake at nupic, don't accept PRs until CMake PRs are pulled. Otherwise, this vicious cycle will never ends.

rhyolight commented 10 years ago

@david-ragazzi I understand, and I've taken the liberty to post a plan review on the nupic-hackers ML. Please respond there with any input. I don't want you doing any more rework either.

rhyolight commented 10 years ago

@david-ragazzi Although CMake is now in place for nupic, I would like to put off this directory restructuring and focus on nupic.core. I think it's more important at this point to put work towards getting nupic.core building itself before we worry about nupic.

What do you think about that?

david-ragazzi commented 10 years ago

Hi Matt,

Before we implement new structure, we have to assure that nupic.core is compiling well. Why? Because this would avoid we change CMake file at nupic repo.

So my suggestion:

rhyolight commented 10 years ago

@david-ragazzi Perfect.

subutai commented 10 years ago

@david-ragazzi Sounds like a great plan. One question: what about the environment variables and env.sh? Should we clean that up first or after the above tasks?

david-ragazzi commented 10 years ago

Sounds like a great plan. One question: what about the environment variables and env.sh? Should we clean that up first or after the above tasks?

Good question. As we will can pass binaries location by command line to CMake and even for Python tests, it won't have room for environment variables (fortunately)..

david-ragazzi commented 10 years ago

Solved by: https://github.com/numenta/nupic/pull/965

rhyolight commented 10 years ago

I want to have a party now that this issue is finally closed! :confetti_ball: :fireworks:

david-ragazzi commented 10 years ago

Party!!

http://madagascar.tribute.ca/wp-content/uploads/2012/04/madagascar2_3.jpg