Pythonic build: Switch from cmake to distutils extensions for nupic installation #1579

david-ragazzi · 2014-12-02T11:26:06Z

This PR aims to help we have Nupic "pip installable". For this it tries to leave the build/install process the more "pythonic" as possible using:
a) native support from Distutils/SetupTools to build all C++ extensions (#1573)
b) Python scripts to perform jobs that were being performed by C++ executables (#1576).

The motivation of B is because it's very tricky install all current cpp executables and libraries using A solution. Some of these libraries (like bindings) are inevitable to be build as extensions, however c++ extensions like htmtest and compare_nupic_core_version can be replaced to python scripts. Different than bindings, these 2 extensions are complicating the process as they should be installed into souce repository (bin folder, to be exact) and not into nupic package install folder like the other modules.

Once that all extensions now can be build in a natural way, we can remove CMakeLists.txt from NuPIC Pyhton. This makes that CMake becomes essential only to build the NuPIC Core library: as this is an optional job, we could bypass CMake/Git use during PIP process decreasing the number of build/install steps and dependencies.

I still am working on the pythonic version of HtmTest. Thus it's normal that some tests fail, because Travis still is referencing its c++ version.

TODO:

Figure out the ARCHFLAGS="-arch x86_64" requirement on darwin64. Update: There's not way, distutils take the flags which python was build. So we cannot set this variable inside setup.py.
Update supporting documentation (README, wikis) with any build changes. Update: README updated with ARCHFLAGS instructions.
Simplification of this PR by move bindings refactoring to a new PR. Solved by Bindings refactored #1610
Simplification of this PR by move Travis scripts, CMakeLists.txt, and README update (replacement of make commands to python instructions and custom_targets remotion) to a new PR. Solved by Pythonize the tests calls #1622
Simplification of this PR by move HtmTest to nupic.core on a new PR. Solved by Remove HtmTest and call it from nupic.core binaries #1676 and Move test_py_htm to nupic.core and refactor test nupic.core-legacy#282
python setup.py --help & python setup.py --help-commands should print help, not compile

david-ragazzi · 2014-12-02T11:42:36Z

CC: @rhyolight @oxtopus

david-ragazzi · 2014-12-02T15:28:39Z

Update:

I've found these Python tests which are very similar to HtmTest:

https://github.com/numenta/nupic/blob/master/tests/integration/py2/nupic/engine/network_testnode_interchangeability.py
https://github.com/numenta/nupic/blob/master/tests/integration/py2/nupic/engine/network_twonode_test.py

I'm serioulsy thinking in remove it as it sounds very redundant.

rhyolight · 2014-12-05T05:09:36Z

setup.py

+    if not os.path.exists(destDir):
+      os.makedirs(destDir)
+
+    response = urllib2.urlopen(url)


@Davidragazzi You have to handle all HTTP errors here. No matter what the error, print a warning and proceed with a local clone and build of nupic.core.

rhyolight · 2014-12-05T05:11:37Z

Here is the error I'm getting on darwin:

duplicate symbol __ZNK4YAML6Stream11GetNextByteEv in:
    /Users/mtaylor/nta/nupic/extensions/core/build/release/lib/libnupic_core.a(stream.cpp.o)
    /Users/mtaylor/nta/nupic/extensions/core/build/release/lib/libyaml-cpp.a(stream.cpp.o)
ld: 93 duplicate symbols for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
error: command 'c++' failed with exit status 1

Looks like nupic.core contains libyaml.

oxtopus · 2014-12-05T06:05:01Z

Yes, libyaml is bundled into libnupic_core.a: https://github.com/numenta/nupic.core/blob/master/src/CMakeLists.txt#L398

oxtopus · 2014-12-05T06:07:28Z

setup.py

+    commonObjects = [
+      nupicCoreReleaseDir + "/lib/" + self.getStaticLibFile("nupic_core"),
+      nupicCoreReleaseDir + "/lib/" + self.getStaticLibFile("gtest"),
+      nupicCoreReleaseDir + "/lib/" + self.getStaticLibFile("gtest"),


Duplicate line.

oxtopus · 2014-12-05T06:38:13Z

Please see david-ragazzi#41

That builds for me on my laptop with caveats. For some reason, by default "-arch x86_64 -arch i386" is passed to clang, which causes compiler errors:

../nupic.core/release/include/nupic/math/ArrayAlgo.hpp:5256:30: error: register %rdx is only available in 64-bit mode
                     "ja 0b\n\t"                   // loop
                             ^
<inline asm>:21:7: note: instantiated into assembly here
        popq %rdx
             ^~~~
fatal error: too many errors emitted, stopping now [-ferror-limit=]
14 warnings and 20 errors generated.
error: command '/usr/bin/clang' failed with exit status 1

As far as I can tell, the only way around it is to set the env var, which is not ideal.

rhyolight · 2014-12-05T07:20:12Z

Hey @david-ragazzi I'd like to work on this PR tomorrow (in about 8 hours). If you have local changes that are worth pushing, try to push them before then. If you're online, maybe we can work together.

david-ragazzi · 2014-12-05T10:32:35Z

@oxtopus said:

Yes, libyaml is bundled into libnupic_core.a: https://github.com/numenta/nupic.core/blob/master/src/CMakeLists.txt#L398

Ah ok.. This answered my question in david-ragazzi#41 (comment)... I'll merge your PR now.. By the way, we still need solve this ARCH env var and avoid this. I see now that you agree that this is not ideal. So we could put a #TODO as @rhyolight suggested until we solve the other issues.

david-ragazzi · 2014-12-05T10:37:25Z

Hey @david-ragazzi I'd like to work on this PR tomorrow (in about 8 hours). If you have local changes that are worth pushing, try to push them before then. If you're online, maybe we can work together.

It's ok for me however I'm not on my machine now, so I couldn't help pushing directly code until go home (11 am - Los Angeles time) but merging PRs into my branch and helping to solve issues until there. About this, could someone solve the merge conflicts on this PR and then send a new PR to I update my branch?

rhyolight · 2014-12-05T16:07:26Z

@david-ragazzi said:

could someone solve the merge conflicts on this PR and then send a new PR to I update my branch?

david-ragazzi#42

rhyolight · 2014-12-05T16:52:24Z

Current status for local darwin build: http://pastebin.com/NtDGg2Fi

Basically, this error over and over:

In file included from nupic/bindings/math_wrap.cpp:3179:
/Users/mtaylor/nta/nupic/extensions/core/build/release/include/nupic/math/ArrayAlgo.hpp:5250:49: error: register %rsi is only
      available in 64-bit mode
                     "movaps %%xmm3, 48(%%rdi)\n\t"

oxtopus · 2014-12-05T17:05:17Z

@rhyolight That is the issue that requires setting the ARCHFLAGS env variable as I described. It's not happy with the -arch i386 flag that seems to be generated automatically.

rhyolight · 2014-12-05T17:21:15Z

@oxtopus Got it. I updated this PR with a new TODO item so we don't forget it.

rhyolight · 2014-12-05T17:22:56Z

@david-ragazzi I'm going to start by adding HTTP error handling logic when trying to pull in nupic.core binaries.

rhyolight · 2014-12-05T17:27:41Z

setup.py

+
+
+  def skipPyCompile(self, file, cfile=None, dfile=None, doraise=False):
+    filesToSkip = ["UnimportableNode.py"]


@oxtopus Didn't you take care of this in a recent PR?

Yes, I did, and it no longer needs to be skipped.

david-ragazzi · 2014-12-05T17:38:44Z

Hi guys, as we are in a Python environment, could be possible set temporary ARCH in runtime using os.env?

rhyolight · 2014-12-05T17:39:00Z

setup.py

+    #
+    if sys.version_info < (2, 7):
+      print "FATAL_ERROR: Only these versions of Python are accepted: 2.7 or later"
+      sys.exit()


Instead of this printout and exit, I think I'd rather raise an Exception for things we define as "fatal". I'll include in a PR I'm working on.

rhyolight · 2014-12-05T18:36:45Z

@david-ragazzi david-ragazzi#43

rhyolight · 2015-01-13T20:31:30Z

@david-ragazzi david-ragazzi#51 pulls in the latest changes from master after I pushed 0.1. It includes a big README simplification, but your ARCHFLAGS instructions were moved into the wiki along with the other "build from source" instructions.

Merges in master with 0.1 README

david-ragazzi · 2015-01-13T20:56:28Z

@david-ragazzi david-ragazzi#51 pulls in the latest changes from master after I pushed 0.1. It includes a big README simplification, but your ARCHFLAGS instructions were moved into the wiki along with the other "build from source" instructions.

Ok, it's fine.

coveralls · 2015-01-13T21:38:40Z

Coverage remained the same when pulling 53de635 on david-ragazzi:build_extensions into d8436f1 on numenta:master.

rhyolight · 2015-01-14T02:45:08Z

@scottpurdy ping!

scottpurdy · 2015-01-16T00:03:54Z

.gitignore

-nupic/bindings/*.cxx
-nupic/bindings/proto/*.capnp
+nupic/bindings/
+~nupic/bindings/*.i


what is ~nupic? I'm not convinced that I want this hidden from me if it gets created

~ = not, so these lines mean ignoring all files in nupic/bindings/, except *.i files.

scottpurdy · 2015-01-16T00:14:46Z

A couple questions but I think this is good. @oxtopus was happy and I wanted to make sure that we are fitting all of our compilation steps into the "extension" framework which it looks this does cleanly.

@david-ragazzi - thanks for sticking with this one! Big change but will be very nice if we can keep this working for source or binary installations on different platforms.

scottpurdy · 2015-01-16T00:15:07Z

Oh and to be clear, big 👍

rhyolight · 2015-01-16T17:19:38Z

@david-ragazzi Please respond to Scott's comments and take a look at https://github.com/numenta/nupic/wiki/NuPIC's-Dependency-on-nupic.core... does this wiki need to be updated along with the merge of this PR?

david-ragazzi · 2015-01-18T22:21:01Z

Please respond to Scott's comments

Done.

take a look at https://github.com/numenta/nupic/wiki/NuPIC's-Dependency-on-nupic.core... does this wiki need to be updated along with the merge of this PR?

Yeah, we need update wiki with removal of CMake after this is merged.

coveralls · 2015-01-18T22:57:14Z

Coverage decreased (-0.02%) when pulling e66635c on david-ragazzi:build_extensions into d8436f1 on numenta:master.

Pythonic build: Switch from cmake to distutils extensions for nupic installation

rhyolight · 2015-01-19T17:05:16Z

I updated https://github.com/numenta/nupic/wiki/NuPIC's-Dependency-on-nupic.core

Fixes: numenta#1573 It's weird.. I simply don't undertand why this file still is in repository. I thought I had removed it through numenta#1579

…le, dir, etc.) as well as minor alignment issues

Cleanup re: numenta#1579. Fixup namespace conflicts with builtins (file, dir, ...

Pythonic build: Switch from cmake to distutils extensions for nupic installation

Fixes: numenta#1573 It's weird.. I simply don't undertand why this file still is in repository. I thought I had removed it through numenta#1579

david-ragazzi force-pushed the build_extensions branch 10 times, most recently from 311ad29 to 7d446d6 Compare December 4, 2014 23:38

rhyolight reviewed Dec 5, 2014
View reviewed changes

oxtopus reviewed Dec 5, 2014
View reviewed changes

david-ragazzi mentioned this pull request Dec 5, 2014

Python-only platform-independent build of NuPIC? #1302

Open

rhyolight reviewed Dec 5, 2014
View reviewed changes

Merge pull request #51 from rhyolight/build_extensions

53de635

Merges in master with 0.1 README

scottpurdy reviewed Jan 16, 2015
View reviewed changes

rhyolight assigned david-ragazzi Jan 16, 2015

Changes after Scott's feedback

e66635c

rhyolight added a commit that referenced this pull request Jan 19, 2015

Merge pull request #1579 from david-ragazzi/build_extensions

9f17246

Pythonic build: Switch from cmake to distutils extensions for nupic installation

rhyolight merged commit 9f17246 into numenta:master Jan 19, 2015

This was referenced Jan 19, 2015

Use Python scripts to perform jobs that were being performed by C++ executables #1576

Closed

Move from cmake to setuptools for building and preparation for distribution #1617

Closed

This was referenced Jan 19, 2015

Wheel creation fails in Travis-CI: compile error in clang and gcc #1762

Closed

Remove default setup.py command #1749

Closed

david-ragazzi pushed a commit to david-ragazzi/nupic that referenced this pull request Jan 22, 2015

Removal of CMakeLists.txt

67cdaa4

Fixes: numenta#1573 It's weird.. I simply don't undertand why this file still is in repository. I thought I had removed it through numenta#1579

david-ragazzi mentioned this pull request Jan 22, 2015

Removal of CMakeLists.txt #1771

Merged

david-ragazzi deleted the build_extensions branch February 21, 2015 17:58

dbl001 mentioned this pull request Jul 4, 2015

ImportError: No module named _math #2299

Open

mihail911 pushed a commit to mihail911/nupic that referenced this pull request Aug 5, 2015

Cleanup re: numenta#1579. Fixup namespace conflicts with builtins (fi…

22dd35a

…le, dir, etc.) as well as minor alignment issues

mihail911 pushed a commit to mihail911/nupic that referenced this pull request Aug 5, 2015

Merge pull request numenta#48 from oxtopus/build_extensions

c17e787

Cleanup re: numenta#1579. Fixup namespace conflicts with builtins (file, dir, ...

mihail911 pushed a commit to mihail911/nupic that referenced this pull request Aug 5, 2015

Merge pull request numenta#1579 from david-ragazzi/build_extensions

83de484

Pythonic build: Switch from cmake to distutils extensions for nupic installation

mihail911 pushed a commit to mihail911/nupic that referenced this pull request Aug 5, 2015

Removal of CMakeLists.txt

dc00bca

Fixes: numenta#1573 It's weird.. I simply don't undertand why this file still is in repository. I thought I had removed it through numenta#1579

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pythonic build: Switch from cmake to distutils extensions for nupic installation #1579

Pythonic build: Switch from cmake to distutils extensions for nupic installation #1579

david-ragazzi commented Dec 2, 2014

david-ragazzi commented Dec 2, 2014

david-ragazzi commented Dec 2, 2014

rhyolight Dec 5, 2014

rhyolight commented Dec 5, 2014

oxtopus commented Dec 5, 2014

oxtopus Dec 5, 2014

oxtopus commented Dec 5, 2014

rhyolight commented Dec 5, 2014

david-ragazzi commented Dec 5, 2014

david-ragazzi commented Dec 5, 2014

rhyolight commented Dec 5, 2014

rhyolight commented Dec 5, 2014

oxtopus commented Dec 5, 2014

rhyolight commented Dec 5, 2014

rhyolight commented Dec 5, 2014

rhyolight Dec 5, 2014

oxtopus Dec 5, 2014

david-ragazzi commented Dec 5, 2014

rhyolight Dec 5, 2014

rhyolight commented Dec 5, 2014

rhyolight commented Jan 13, 2015

david-ragazzi commented Jan 13, 2015

coveralls commented Jan 13, 2015

rhyolight commented Jan 14, 2015

scottpurdy Jan 16, 2015

david-ragazzi Jan 18, 2015

scottpurdy commented Jan 16, 2015

scottpurdy commented Jan 16, 2015

rhyolight commented Jan 16, 2015

david-ragazzi commented Jan 18, 2015

coveralls commented Jan 18, 2015

rhyolight commented Jan 19, 2015



		def skipPyCompile(self, file, cfile=None, dfile=None, doraise=False):
		filesToSkip = ["UnimportableNode.py"]

Pythonic build: Switch from cmake to distutils extensions for nupic installation #1579

Pythonic build: Switch from cmake to distutils extensions for nupic installation #1579

Conversation

david-ragazzi commented Dec 2, 2014

TODO:

david-ragazzi commented Dec 2, 2014

david-ragazzi commented Dec 2, 2014

rhyolight Dec 5, 2014

Choose a reason for hiding this comment

rhyolight commented Dec 5, 2014

oxtopus commented Dec 5, 2014

oxtopus Dec 5, 2014

Choose a reason for hiding this comment

oxtopus commented Dec 5, 2014

rhyolight commented Dec 5, 2014

david-ragazzi commented Dec 5, 2014

david-ragazzi commented Dec 5, 2014

rhyolight commented Dec 5, 2014

rhyolight commented Dec 5, 2014

oxtopus commented Dec 5, 2014

rhyolight commented Dec 5, 2014

rhyolight commented Dec 5, 2014

rhyolight Dec 5, 2014

Choose a reason for hiding this comment

oxtopus Dec 5, 2014

Choose a reason for hiding this comment

david-ragazzi commented Dec 5, 2014

rhyolight Dec 5, 2014

Choose a reason for hiding this comment

rhyolight commented Dec 5, 2014

rhyolight commented Jan 13, 2015

david-ragazzi commented Jan 13, 2015

coveralls commented Jan 13, 2015

rhyolight commented Jan 14, 2015

scottpurdy Jan 16, 2015

Choose a reason for hiding this comment

david-ragazzi Jan 18, 2015

Choose a reason for hiding this comment

scottpurdy commented Jan 16, 2015

scottpurdy commented Jan 16, 2015

rhyolight commented Jan 16, 2015

david-ragazzi commented Jan 18, 2015

coveralls commented Jan 18, 2015

rhyolight commented Jan 19, 2015