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.55k forks source link

CMake fails when $NTA and $NUPIC are both empty #755

Closed utensil closed 10 years ago

utensil commented 10 years ago

In README.md:

After CMake generation, two useful environment variables will be created:

  • $NUPIC, which is the same as $REPOSITORY
  • $NTA, which references $HOME/nta/eng (the directory with all executables and libraries generated from build process). If this variable is already set, the $REPOSITORY/release will not be created, and $NTA will be used as the release directory.

This is an usability improvement compared to the old "set $NUPIC and $NTA yourself or the build fails" approach, providing sane defaults in the spirit of 2014 Goals For NuPIC: Easy install, easy build.

But when $NTA and $NUPIC are both empty, cmake $REPOSITORY complains:

CMake Error at CMakeLists.txt:181 (message):
  $NTA environment variable cannot be the same as the $NUPIC environment
  variable!

-- Configuring incomplete, errors occurred!

In CMakeLists.txt:181, it reads:

#
# Cannot have same $NTA and $NuPIC
#
if("$ENV{NTA}" STREQUAL "$ENV{NUPIC}")
  message(FATAL_ERROR "\$NTA environment variable cannot be the same as the \$NUPIC environment variable!")
endif()

This logic fixes #659 , but is in contradiction to with the statements in README.md, which implemented around CMakeLists.txt:436:

set_environment_variable(NUPIC "${PROJECT_SOURCE_DIR}" OFF)
...
if("$ENV{NTA}" STREQUAL "")
  set_environment_variable(NTA "${CMAKE_INSTALL_PREFIX}" OFF)

Thus, as long as they are not both empty, defaults would work.

And I noticed in .travis.yml, $NUPIC and $NTA are declare in env.global, and I guess most nupic developers already have them configured, that's why it's not spotted.

utensil commented 10 years ago

There's one way to fix this issue, as I tried in #756 : we can confirms $NTA != $NUPIC after applying defaults to them.

Reasoning:

  1. fixes #755
  2. prevents anything breaking $NTA != $NUPIC ( #659 ) in the process of setting these environment variables.
rhyolight commented 10 years ago

@utensil Good catch! Thanks for your contribution and trying to follow our process. You've been added as a contributor now, so your PRs should pass.

rhyolight commented 10 years ago

@david-ragazzi This makes sense to me. Do you have an opinion?

david-ragazzi commented 10 years ago

@david-ragazzi This makes sense to me. Do you have an opinion?

Yes, a good observation by @utensil . I also had same problems when these variables were empty.. So the change proposed is need.. Although that in new version of CMake file (https://github.com/numenta/nupic/pull/751), CMake configures every time $NTA and $NUPIC using default values ($REPOSITORY/build/release and $REPOSITORY folders, which can be changed by command line). This makes that these variables never will be empty..

@utensil Please, keep on eye on https://github.com/numenta/nupic/pull/751 , and let us know if this error remains even in the version of CMake file.. Your help is appreciate..

utensil commented 10 years ago

@david-ragazzi Glad to see #751 further promote the forming of nupic.core, it's really excellent work. First time I encounter nupic, excited by its beauty, it took me quite a while to figure out what's going on in all directories, and I just felt puzzled and a little frustrated. And now, a more clear structure is showing a brighter future for the community to get involved.

I'll try #751 soon.

david-ragazzi commented 10 years ago

First time I encounter nupic, excited by its beauty, it took me quite a while to figure out what's going on in all directories, and I just felt puzzled and a little frustrated. And now, a more clear structure is showing a brighter future for the community to get involved.

@utensil Uow.. Read this is really motivating! Because this was what I have in mind, i.e. decreasing the learning curve by put the things in a structure easy to follow and so more people get involved.. :-)

rhyolight commented 10 years ago

:metal:

david-ragazzi commented 10 years ago

http://www.youtube.com/watch?v=AHb4gs1hwck :japanese_ogre:

:metal:

rhyolight commented 10 years ago

I love that one.


Matt Taylor OS Community Flag-Bearer Numenta

On Wed, Apr 2, 2014 at 10:15 AM, David Ragazzi notifications@github.comwrote:

http://www.youtube.com/watch?v=AHb4gs1hwck [image: :japanese_ogre:]

[image: :metal:]

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

david-ragazzi commented 10 years ago

I love that one.

:smile:

rhyolight commented 10 years ago

@utensil Is this bug still valid?

utensil commented 10 years ago

Though there's still a lot of discussions related to this issue, this issue is no longer describing an issue of nupic@HEAD, so I'll close it.

The discussion continues at hackers' ML, and some of my opinions are expressed at https://github.com/numenta/nupic/issues/790#issuecomment-40035027 and https://github.com/numenta/nupic/pull/801#issuecomment-40172540 .