ENH: Move cython build to c++ #648

nonhermitian · 2017-02-15T07:29:19Z

I have managed to get everything moved over to c++. Should address #647. There are still a few issues to address, but the tests pass on unix based systems.

CFLAGS need to be set for visual studio builds.
zspmv src files need to be platform dependent (i.e. win/unix versions). This is mainly due to a few minor variations in how compiler hinting is done. No big deal. Also, when moving to openmp, visual studio uses v2.0, which is a bit dated.

ajgpitch

All looks great to me. I can hardly believe you managed to get this all implemented and working so quickly

ajgpitch · 2017-02-16T13:00:57Z

qutip/__init__.py

-pyximport.install(setup_args={'include_dirs': [numpy.get_include()]})
-del pyximport
+import qutip.cy.pyxbuilder as pbldr
+os.environ['CFLAGS'] = '-O1 -w -ffast-math'


Just a question: why do we have a reduced set of flags here compared to _compiler_flags in cy/setup.py?

Because we are just gluing things together here. Setting optimizations to high here increases runtime build time for no performance gains.

ajgpitch · 2017-02-16T13:05:33Z

qutip/cy/spmatfuncs.pyx

@@ -35,7 +35,7 @@ cimport numpy as np
 cimport cython
 cimport libc.math

-cdef extern from "src/zspmv.h" nogil:
+cdef extern from "src/zspmv.hpp" nogil:


Would we do a conditional import here based on compiler / platform

Yep. Maybe src/win_zspmv or something on win platforms.

ajgpitch · 2017-02-16T13:05:45Z

qutip/cy/setup.py

@@ -11,23 +11,24 @@
 exts = ['spmatfuncs', 'stochastic', 'sparse_utils', 'graph_utils', 'interpolate',
        'spmath', 'heom', 'math', 'spconvert', 'ptrace', 'testing']

-_compiler_flags = ['-w', '-ffast-math', '-O3', '-march=native', '-funroll-loops']
+_compiler_flags = ['-w', '-std=c++11', '-ffast-math', '-O3', '-march=native', '-funroll-loops']


Would we add platform / compiler specific flags in here?

Yes, the format for visual studio compiler args is different.

ajgpitch · 2017-02-16T13:15:38Z

There may be some who would wish continue using the mingw compiler on Windows, as it is a smaller download. I guess we could just say that we only support MS compilers on Windows.
Would the zspmv.cpp need to be platform or compiler specific?
Is it possible to choose flags / source based on which compiler is being used? I have seen in the cython generated C that there are compiler specific conditionals in the directives.

ajgpitch · 2017-02-16T13:17:44Z

I have built this branch and tested on my linux machine here. All tests pass.
I am assuming that you need to make another commit before it's ready for me to test on Windows?

ajgpitch · 2017-02-16T14:06:33Z

Meant to add: I get this warning in build_ext and in all str format td tests
cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++

nonhermitian · 2017-02-16T14:28:23Z

Everything should work using mingw. The dwfault Anaconda mingw is 4.7.1, that supports the c++11 features we need. Mingwpy is 4.9 and even supports openmp. So eveything should be fine. But I have not checked.

The command line warning you see is something that Cython is emitting, not QuTiP. Perhaps this is a bug on their end? I dont know how to remove the flags that Cython specifies. The dash -w should block all warnings though.

nonhermitian · 2017-02-16T14:29:11Z

And yes, will figure out the win stuff today at work.

nonhermitian · 2017-02-16T22:25:56Z

There is an issue with numpy.distutils on win. To get this done anytime soon, I think we need to move to setuptools.

ajgpitch · 2017-02-17T10:38:16Z

Do you mean the issue with not being able to find vcvarsall.bat? mentioned in:
https://github.com/cython/cython/wiki/CythonExtensionsOnWindows

In my test project I used the workaround they suggest:

try:
    from setuptools import setup
    from setuptools import Extension
except ImportError:
    from distutils.core import setup
    from distutils.extension import Extension

And this seems to work. Is it more complicated than that for us?

nonhermitian · 2017-02-17T19:04:33Z

Nope, it is something else. I have a workaround similar to what you did. Everything seems to ve working as the tests run fine. Awesome stuff.

nonhermitian · 2017-02-18T05:14:03Z

@ajgpitch, please try this latest update on Win. I had everything up and running at work today. The only thing I didn't have was a spmv source file that compiled under GCC, CLANG, and VS. I think the file I have up now should work, but as I don't have Win at home, I need a verification.

The only thing missing from the pull is the optional fortran extension. I will add that on Monday.

ajgpitch · 2017-02-20T22:33:44Z

Attempt to install qutip in python2.7

running build_ext
building 'qutip.cy.spmatfuncs' extension
creating build\temp.win-amd64-2.7
creating build\temp.win-amd64-2.7\Release
creating build\temp.win-amd64-2.7\Release\qutip
creating build\temp.win-amd64-2.7\Release\qutip\cy
creating build\temp.win-amd64-2.7\Release\qutip\cy\src
C:\Users\alex\AppData\Local\Programs\Common\Microsoft\Visual C++ for Python\9.0\VC\Bin\amd64\cl.exe /c /nologo /Ox /MD /W3 /GS- /DNDEBUG -IC:\Anaconda2\envs\qtpy2\lib\site-packages\numpy\core\include -IC:\Anaconda2\envs\qtpy2\lib\site-packages\numpy\core\include -IC:\Anaconda2\envs\qtpy2\include -IC:\Anaconda2\envs\qtpy2\PC /Tpqutip/cy/spmatfuncs.cpp /Fobuild\temp.win-amd64-2.7\Release\qutip/cy/spmatfuncs.obj -w -std=c++11 -O3 -march=native -funroll-loops
cl : Command line warning D9025 : overriding '/W3' with '/w'
cl : Command line warning D9002 : ignoring unknown option '-std=c++11'
cl : Command line warning D9002 : ignoring unknown option '-O3'
cl : Command line warning D9002 : ignoring unknown option '-march=native'
cl : Command line warning D9002 : ignoring unknown option '-funroll-loops'
spmatfuncs.cpp
c:\anaconda2\envs\qtpy2\lib\site-packages\numpy\core\include\numpy\npy_1_7_deprecated_api.h(12) : Warning Msg: Using deprecated NumPy API, disable it by #defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
C:\Users\alex\AppData\Local\Programs\Common\Microsoft\Visual C++ for Python\9.0\VC\Include\xlocale(342) : warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc
c:\users\alex\documents\github\qutip\qutip\cy\src/zspmv.hpp(3) : error C2146: syntax error : missing ',' before identifier 'data'
qutip/cy/spmatfuncs.cpp(2503) : error C2660: 'zspmvpy' : function does not take 7 arguments
qutip/cy/spmatfuncs.cpp(2688) : error C2660: 'zspmvpy' : function does not take 7 arguments
qutip/cy/spmatfuncs.cpp(2923) : error C2660: 'zspmvpy' : function does not take 7 arguments
qutip/cy/spmatfuncs.cpp(4801) : error C2668: 'sqrt' : ambiguous call to overloaded function
        C:\Users\alex\AppData\Local\Programs\Common\Microsoft\Visual C++ for Python\9.0\VC\Include\math.h(581): could be 'long double sqrt(long double)'
        C:\Users\alex\AppData\Local\Programs\Common\Microsoft\Visual C++ for Python\9.0\VC\Include\math.h(533): or       'float sqrt(float)'
        C:\Users\alex\AppData\Local\Programs\Common\Microsoft\Visual C++ for Python\9.0\VC\Include\math.h(128): or       'double sqrt(double)'
        while trying to match the argument list '(int)'
error: command 'C:\\Users\\alex\\AppData\\Local\\Programs\\Common\\Microsoft\\Visual C++ for Python\\9.0\\VC\\Bin\\amd64\\cl.exe' failed with exit status 2

ajgpitch · 2017-02-20T22:42:57Z

Attempt in Python3.5

running build_ext
building 'qutip.cy.spmatfuncs' extension
creating build\temp.win-amd64-3.5
creating build\temp.win-amd64-3.5\Release
creating build\temp.win-amd64-3.5\Release\qutip
creating build\temp.win-amd64-3.5\Release\qutip\cy
creating build\temp.win-amd64-3.5\Release\qutip\cy\src
C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\x86_amd64\cl.exe /c /nologo /Ox /W3 /GL /DNDEBUG /MD -IC:\Anaconda2\envs\qtpy3\lib\site-packages\numpy\core\include -IC:\Anaconda2\envs\qtpy3\lib\site-packages\numpy\core\include -IC:\Anaconda2\envs\qtpy3\include -IC:\Anaconda2\envs\qtpy3\include "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE" "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\ATLMFC\INCLUDE" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\shared" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\um" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\winrt" "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE" "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\ATLMFC\INCLUDE" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\shared" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\um" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\winrt" /EHsc /Tpqutip/cy/spmatfuncs.cpp /Fobuild\temp.win-amd64-3.5\Release\qutip/cy/spmatfuncs.obj /w /Ox
cl : Command line warning D9025 : overriding '/W3' with '/w'
spmatfuncs.cpp
c:\anaconda2\envs\qtpy3\lib\site-packages\numpy\core\include\numpy\npy_1_7_deprecated_api.h(12) : Warning Msg: Using deprecated NumPy API, disable it by #defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
c:\users\alex\documents\github\qutip\qutip\cy\src/zspmv.hpp(3): error C2146: syntax error: missing ')' before identifier 'data'
c:\users\alex\documents\github\qutip\qutip\cy\src/zspmv.hpp(3): error C3646: 'data': unknown override specifier
c:\users\alex\documents\github\qutip\qutip\cy\src/zspmv.hpp(3): error C2062: type 'int' unexpected
c:\users\alex\documents\github\qutip\qutip\cy\src/zspmv.hpp(7): error C2059: syntax error: ')'
qutip/cy/spmatfuncs.cpp(2503): error C2660: 'zspmvpy': function does not take 7 arguments
qutip/cy/spmatfuncs.cpp(2688): error C2660: 'zspmvpy': function does not take 7 arguments
qutip/cy/spmatfuncs.cpp(2923): error C2660: 'zspmvpy': function does not take 7 arguments
error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\BIN\\x86_amd64\\cl.exe' failed with exit status 2

nonhermitian · 2017-02-21T05:19:30Z

Py2.7 on Windows needs to always use the GCC. The VS version corresponding to Py27 does not support the functions that we need. The Py3.5 errors mean that my #define did not work. I have put up a different version, and will check it tomorrow at work.

ajgpitch · 2017-02-21T11:48:19Z

Okay, I am much happier with the idea that our recommended Python version for qutip on MS Wondows is >=3.5 than the other way round. I would be quite happy to consider dropping support for Python 2 altogether at some point.

nonhermitian · 2017-02-22T02:06:13Z

Ok, we should be set now. Tested working on GCC, CLANG, mingwpy, and VS 2015.

ajgpitch · 2017-02-22T16:39:45Z

I have just finished testing with Vs 2015 as well (win10, Python 3.5). All passed.
I feel this is a momentous occasion. Great work!
I think this is worthy of a 4.1 release. Very keen to get working conda package for Windows out there

nonhermitian · 2017-02-23T05:00:10Z

Merging this now.

nonhermitian added 30 commits September 12, 2016 14:48

Merge remote-tracking branch 'qutip/master'

21c31c3

Merge remote-tracking branch 'qutip/master'

8bcdade

Merge remote-tracking branch 'qutip/master'

c2122cd

Merge remote-tracking branch 'qutip/master'

3b33f25

Merge remote-tracking branch 'qutip/master'

521c4cc

Merge remote-tracking branch 'qutip/master'

a4b0049

Merge remote-tracking branch 'qutip/master'

90800d6

Merge remote-tracking branch 'qutip/master'

8ed5b6a

Merge remote-tracking branch 'qutip/master'

4bb5fdb

Merge remote-tracking branch 'qutip/master'

06db6e5

Merge remote-tracking branch 'qutip/master'

81b554b

Merge remote-tracking branch 'qutip/master'

0235814

Merge remote-tracking branch 'qutip/master'

b972e80

Merge remote-tracking branch 'qutip/master'

00e1808

Merge remote-tracking branch 'qutip/master'

2048be0

Merge remote-tracking branch 'qutip/master'

d6f0405

Merge remote-tracking branch 'qutip/master'

587ee2e

Merge remote-tracking branch 'qutip/master'

498537d

Merge remote-tracking branch 'qutip/master'

3658c1a

Merge remote-tracking branch 'qutip/master'

8e50dc3

Merge remote-tracking branch 'qutip/master'

4e8d506

Merge remote-tracking branch 'qutip/master'

59f039e

Merge remote-tracking branch 'qutip/master'

626931e

Merge remote-tracking branch 'qutip/master'

f8b1e30

Merge remote-tracking branch 'qutip/master'

b51eeed

Merge remote-tracking branch 'qutip/master'

20a80b2

Merge remote-tracking branch 'qutip/master' into spmv_updates

6e7f9f8

added openmp

a068c65

Merge remote-tracking branch 'qutip/master' into spmv_updates

4a32d5f

move to cpp in cython

e900a06

nonhermitian self-assigned this Feb 15, 2017

nonhermitian requested a review from ajgpitch February 15, 2017 07:29

ajgpitch approved these changes Feb 16, 2017

View reviewed changes

nonhermitian added 2 commits February 17, 2017 22:07

updated

bf8d0e3

Merge remote-tracking branch 'qutip/master' into spmv_updates

b580abb

should work better

debdad8

nonhermitian added 2 commits February 21, 2017 18:52

updates

f3403a8

minor change in passing

510d633

This was referenced Feb 22, 2017

Fortran solver does not work using Anaconda #439

Closed

'z' format not supported by mingw32 compilers #597

Closed

ajgpitch mentioned this pull request Feb 22, 2017

Are we ready for v4.1? #649

Closed

nonhermitian changed the title ~~ENH(WIP): Move cython build to c++~~ ENH: Move cython build to c++ Feb 23, 2017

nonhermitian merged commit 82e2095 into qutip:master Feb 23, 2017

nonhermitian mentioned this pull request Feb 23, 2017

ENH : Last 41 fixes #650

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Move cython build to c++ #648

ENH: Move cython build to c++ #648

nonhermitian commented Feb 15, 2017

ajgpitch left a comment

ajgpitch Feb 16, 2017

nonhermitian Feb 16, 2017

ajgpitch Feb 16, 2017

nonhermitian Feb 16, 2017

ajgpitch Feb 16, 2017

nonhermitian Feb 16, 2017

ajgpitch commented Feb 16, 2017

ajgpitch commented Feb 16, 2017

ajgpitch commented Feb 16, 2017

nonhermitian commented Feb 16, 2017

nonhermitian commented Feb 16, 2017

nonhermitian commented Feb 16, 2017

ajgpitch commented Feb 17, 2017

nonhermitian commented Feb 17, 2017

nonhermitian commented Feb 18, 2017

ajgpitch commented Feb 20, 2017

ajgpitch commented Feb 20, 2017

nonhermitian commented Feb 21, 2017

ajgpitch commented Feb 21, 2017

nonhermitian commented Feb 22, 2017

ajgpitch commented Feb 22, 2017

nonhermitian commented Feb 23, 2017

ENH: Move cython build to c++ #648

ENH: Move cython build to c++ #648

Conversation

nonhermitian commented Feb 15, 2017

ajgpitch left a comment

Choose a reason for hiding this comment

ajgpitch Feb 16, 2017

Choose a reason for hiding this comment

nonhermitian Feb 16, 2017

Choose a reason for hiding this comment

ajgpitch Feb 16, 2017

Choose a reason for hiding this comment

nonhermitian Feb 16, 2017

Choose a reason for hiding this comment

ajgpitch Feb 16, 2017

Choose a reason for hiding this comment

nonhermitian Feb 16, 2017

Choose a reason for hiding this comment

ajgpitch commented Feb 16, 2017

ajgpitch commented Feb 16, 2017

ajgpitch commented Feb 16, 2017

nonhermitian commented Feb 16, 2017

nonhermitian commented Feb 16, 2017

nonhermitian commented Feb 16, 2017

ajgpitch commented Feb 17, 2017

nonhermitian commented Feb 17, 2017

nonhermitian commented Feb 18, 2017

ajgpitch commented Feb 20, 2017

ajgpitch commented Feb 20, 2017

nonhermitian commented Feb 21, 2017

ajgpitch commented Feb 21, 2017

nonhermitian commented Feb 22, 2017

ajgpitch commented Feb 22, 2017

nonhermitian commented Feb 23, 2017