Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot compile with Cython 0.17 (using clang on Mac OS 10.8) but ok with Cython 0.16 #102

Closed
rwest opened this issue Sep 13, 2012 · 8 comments

Comments

@rwest
Copy link
Member

rwest commented Sep 13, 2012

We couldn't get RMG-Py to compile on a new Mac running MacOS X 10.8 with clang as the c compiler (as used by numpy, and a homebrew-installed python), but it was working fine on an older mac with the same operating system version, compiler etc. Turns out I hadn't updated Cython on the older computer and it was still on Cython 0.16. Once we downgraded the new computer from Cython 0.17 to Cython 0.16, it worked fine.
Is this a known problem with cython?
Is it unique to clang and/or MacOS?
Should we update the requirements.txt file to specify cython==0.16 ?

Anyway, for anyone running into this problem in the mean time, try:

$ pip uninstall cython
$ pip install cython==0.16
$ make clean
$ make
@jwallen
Copy link
Contributor

jwallen commented Sep 13, 2012

Yes, I ran into the same issue on my Linux laptop when I upgraded to Cython 0.17 on Monday. Cython 0.16 continues to work. It looked like the Cython-to-C translator was choking on new-style cdef classes (i.e. derived from object). I decided to put off dealing with it until my big refactoring branch is further along. At some point we may have to contact the Cython devs.

@DetlevCM
Copy link

The same is true for Ubuntu (and explains why I couldn't get it to compile)... I was trying yo compile RMG-Py and couldn't, only obtaining a a cython error...

This is the "reply" when compiling on Ubuntu:
(Note: Cython claims it is running correctly if you run the tests, but it wasn't compiled with the debug option)

detlevcm@detlevcm-virtual-machine:$ cd ~
detlevcm@detlevcm-virtual-machine:
$ cd RMG-Py; make
python setup.py build_ext main --build-lib . --build-temp build --pyrex-c-in-temp
running build_ext
skipping 'build/pyrex/rmgpy/molecule/atomtype.c' Cython extension (up-to-date)
skipping 'build/pyrex/rmgpy/molecule/element.c' Cython extension (up-to-date)
cythoning rmgpy/molecule/graph.py to build/pyrex/rmgpy/molecule/graph.c

Error compiling Cython file:

...
import cython
import logging

################################################################################

class Vertex(object):

^

rmgpy/molecule/graph.py:43:0: Compiler crash in AnalyseDeclarationsTransform

ModuleNode.body = StatListNode(graph.py:38:0)
StatListNode.stats[2] = StatListNode(graph.py:43:0)
StatListNode.stats[0] = CClassDefNode(graph.py:43:0,
class_name = u'Vertex',
doc = u"\n A base class for vertices in a graph. Contains several connectivity values\n useful for accelerating isomorphism searches, as proposed by\n Morgan (1965) <http://dx.doi.org/10.1021/c160017a018>_.\n\n =================== =============== ========================================\n Attribute Type Description\n =================== =============== ========================================\n connectivity1 int The number of nearest neighbors\n connectivity2 int The sum of the neighbors' connectivity1 values\n connectivity3 int The sum of the neighbors' connectivity2 values\n sortingLabel int An integer label used to sort the vertices\n =================== =============== ========================================\n \n ",
visibility = 'private')

Compiler crash traceback from this point on:
File "Visitor.py", line 178, in Cython.Compiler.Visitor.TreeVisitor._visitchild (/home/detlevcm/Desktop/Cython-0.17/Cython/Compiler/Visitor.c:3939)
File "/usr/local/lib/python2.7/dist-packages/Cython/Compiler/ParseTreeTransforms.py", line 1438, in visit_CClassDefNode
self.visit(property)
File "Visitor.py", line 157, in Cython.Compiler.Visitor.TreeVisitor.visit (/home/detlevcm/Desktop/Cython-0.17/Cython/Compiler/Visitor.c:3583)
File "Visitor.py", line 158, in Cython.Compiler.Visitor.TreeVisitor.visit (/home/detlevcm/Desktop/Cython-0.17/Cython/Compiler/Visitor.c:3535)
File "Visitor.py", line 167, in Cython.Compiler.Visitor.TreeVisitor._visit (/home/detlevcm/Desktop/Cython-0.17/Cython/Compiler/Visitor.c:3735)
File "Visitor.py", line 288, in Cython.Compiler.Visitor.CythonTransform.visit_Node (/home/detlevcm/Desktop/Cython-0.17/Cython/Compiler/Visitor.c:5785)
File "Visitor.py", line 240, in Cython.Compiler.Visitor.VisitorTransform.visitchildren (/home/detlevcm/Desktop/Cython-0.17/Cython/Compiler/Visitor.c:4843)
File "Visitor.py", line 214, in Cython.Compiler.Visitor.TreeVisitor._visitchildren (/home/detlevcm/Desktop/Cython-0.17/Cython/Compiler/Visitor.c:4620)
AssertionError: Cannot insert list here: body in <Cython.Compiler.Nodes.PropertyNode object at 0x8ecf1ec>
building 'rmgpy.molecule.graph' extension
gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fPIC -I. -I/usr/local/lib/python2.7/dist-packages/numpy/core/include -I/usr/include/python2.7 -c build/pyrex/rmgpy/molecule/graph.c -o build/build/pyrex/rmgpy/molecule/graph.o
build/pyrex/rmgpy/molecule/graph.c:1:2: error: #error Do not use this file, it is the result of a failed Cython compilation.
error: command 'gcc' failed with exit status 1
make: *** [main] Error 1
detlevcm@detlevcm-virtual-machine:~/RMG-Py$

@jwallen
Copy link
Contributor

jwallen commented Sep 17, 2012

Yes, that is the same error I am seeing. According to this post on cython-users the issue is with the "binding" option, which as of 0.17 is turned on by default for pure Python files. Apparently using cython -Xbinding=False should allow the build to run as before. I will test this later.

@jwallen
Copy link
Contributor

jwallen commented Nov 5, 2012

I tested things with the recently-released Cython 0.17.1, but the issue still persists. The problem is with the readonly part of the declaration of the edges dict on the Vertex class, though I don't know enough about the Cython parser to interpret the error message. I'll probably change the declaration from readonly to public since that makes the issue go away. (I had made edges read-only for a reason, as it is meant to be a cache and not to be directly edited by the user.) Ultimately the Cython devs should address this upstream, and the link I gave above indicates that someone has at least told them about the issue.

@jwallen
Copy link
Contributor

jwallen commented Nov 9, 2012

FYI I still can't get a complete compilation of RMG-Py with Cython 0.17.1, even after making the above change. The new issue involves the C code generated from the rmgpy.quantity module. The new error message suggests malformed C code is being generated by Cython:

build/pyrex/rmgpy/quantity.c:10731:64: error: expected expression before ‘static’
build/pyrex/rmgpy/quantity.c:11547:68: error: expected expression before ‘static’

I think it might be related to this issue, which suggests something wrong with numpy + pure Python mode. We would have to make rmgpy.quantity into a .pyx file in order to work around this, which is probably okay but not the greatest solution.

Also, if you've recently upgraded to numpy 1.7, you'll probably start seeing this warning during the compilation (on Cython 0.16 as well as 0.17):

warning: #warning "Using deprecated NumPy API, disable it by #defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION"

This doesn't seem to be a problem yet, but it appears for every module that has a cimport numpy line. Again, this seems to be something the Cython devs need to address upstream.

@jwallen
Copy link
Contributor

jwallen commented Apr 26, 2013

Good news: Cython 0.19 was released last week, and it appears to have fixed the issue involving the malformed quantity.c. If I change all of the Cython attributes that are marked readonly to public, then I can build and run RMG-Py with Cython 0.19. Although readonly would be cleaner, I'm willing to make that change if it means we can be running the latest Cython version again.

@rwest
Copy link
Member Author

rwest commented Apr 26, 2013

Excellent. I support this change.

@jwallen
Copy link
Contributor

jwallen commented May 3, 2013

I have made the necessary changes in e64ec1b. After you pull this change you can safely upgrade to Cython 0.19.

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

No branches or pull requests

3 participants