Sage: Ticket #11614: Make Cython libcpp usable
https://trac.sagemath.org/ticket/11614
<p>
Cython's libcpp allows use of C++ container classes (aka STL), but using it from Sage is still challenging. A simple <code>from libcpp.vector cimport vector</code> (<a class="ext-link" href="http://docs.cython.org/src/userguide/wrapping_CPlusPlus.html#standard-library"><span class="icon"></span>http://docs.cython.org/src/userguide/wrapping_CPlusPlus.html#standard-library</a>) does not work in a Sage extension class because <code>setup.py</code> will not find some of the dependencies.
</p>
<p>
This ticket aims to fix this and add an example to the Sage libary documenting its use.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/11614
Trac 1.1.6vbraunWed, 20 Jul 2011 13:06:31 GMTstatus changed; cc, author set
https://trac.sagemath.org/ticket/11614#comment:1
https://trac.sagemath.org/ticket/11614#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>cc</strong>
<em>jdemeyer</em> added
</li>
<li><strong>author</strong>
set to <em>Volker Braun</em>
</li>
</ul>
TicketleifWed, 20 Jul 2011 18:34:43 GMTcc changed
https://trac.sagemath.org/ticket/11614#comment:2
https://trac.sagemath.org/ticket/11614#comment:2
<ul>
<li><strong>cc</strong>
<em>leif</em> added
</li>
</ul>
TicketjdemeyerTue, 27 Sep 2011 08:34:48 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/11614#comment:3
https://trac.sagemath.org/ticket/11614#comment:3
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>reviewer</strong>
set to <em>Jeroen Demeyer</em>
</li>
</ul>
<p>
My most important question about this is:
what is the purpose of this code? It looks like it is meant only as an example, since it has basically no features. I'm not sure whether we want to put code into Sage which is <em>only</em> meant as example. There is supposed to be <code>examples/</code> for this, but I realize that this is totally unmaintained (and therefore scheduled for removal).
</p>
<p>
Then some random thoughts:
</p>
<p>
1) Why
</p>
<pre class="wiki">cdef vector[int] *data
</pre><p>
and not
</p>
<pre class="wiki">cdef vector[int] data
</pre><p>
Considering that a <code>std::vector<int></code> is essentially a pointer to an array, there is no need to declare a pointer to a pointer.
</p>
<p>
2) You should make the copyright statement consistent with the template at <a class="ext-link" href="http://sagemath.org/doc/developer/conventions.html#headings-of-sage-library-code-files"><span class="icon"></span>http://sagemath.org/doc/developer/conventions.html#headings-of-sage-library-code-files</a>
</p>
<p>
3) You should add an example of <code>__getitem__</code> failing.
</p>
TicketvbraunTue, 27 Sep 2011 17:15:07 GMTstatus changed
https://trac.sagemath.org/ticket/11614#comment:4
https://trac.sagemath.org/ticket/11614#comment:4
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
As for why, right now Cython allows us to use stl classes but our <code>setup.py</code> is broken. This ticket fixes it and adds a testcase for future reference. I was very surprised that this issue was not reported before. My guess is that nobody figured out how to use it, which is why I want to have an example in the Sage library at least until it STL containers are used more often so that there are examples elsewhere.
</p>
<p>
1) Thats not allowed by Cython:
</p>
<pre class="wiki">Error compiling Cython file:
------------------------------------------------------------
...
sage: from sage.tests.stl_vector import stl_int_vector
sage: v = stl_int_vector()
"""
cdef vector[int] data
^
------------------------------------------------------------
sage/tests/stl_vector.pyx:40:21: C++ classes not allowed as members of an extension type, use a pointer or reference instead
</pre><p>
2,3) Done
</p>
Ticketjohn_perrySat, 14 Jan 2012 03:12:01 GMT
https://trac.sagemath.org/ticket/11614#comment:5
https://trac.sagemath.org/ticket/11614#comment:5
<p>
I can't get this to work on alpha0 or alpha4; I get, for example,
</p>
<pre class="wiki">Error converting Pyrex file to C:
------------------------------------------------------------
...
sage: TestSuite(v)
Test suite for vector<int>:
data[0] = 123
data[1] = 456
"""
self.data = new vector[int]()
^
------------------------------------------------------------
/Users/user/.sage/temp/Users_MacBook_Pro.local/5523/spyx/_Users_user__sage_temp_Users_MacBook_Pro_local_5523_tmp_0_spyx/_Users_user__sage_temp_Users_MacBook_Pro_local_5523_tmp_0_spyx_0.pyx:62:24: Operation only allowed in c++
</pre><p>
I find that odd, considering it's supposed to be C++. Is there a minimum alpha revision for which this works?
</p>
TicketvbraunSat, 14 Jan 2012 04:21:16 GMT
https://trac.sagemath.org/ticket/11614#comment:6
https://trac.sagemath.org/ticket/11614#comment:6
<p>
Sorry, at one point I dropped the entry in <code>module_list.py</code> so the C extension module did not get compiled. The updated patch fixes this and gives a minimal example of how to use <code>std::string</code>. It works fine with sage-4.8alpha6. The alpha0 might be too old (not sure), but alpha4 should be fine though I haven't tested it.
</p>
TicketvbraunSat, 14 Jan 2012 04:23:47 GMT
https://trac.sagemath.org/ticket/11614#comment:7
https://trac.sagemath.org/ticket/11614#comment:7
<p>
PS: I just checked and it seems we upgrade to cython-0.15.1 in sage4.8alpha2 so anything past that should work just as alpha6.
</p>
Ticketjohn_perrySat, 14 Jan 2012 05:23:50 GMT
https://trac.sagemath.org/ticket/11614#comment:8
https://trac.sagemath.org/ticket/11614#comment:8
<p>
I'm getting the same doctest errors, plus more, what with the addition to the tests.pyx file of <code>string</code> examples. This is on alpha4.
</p>
<p>
I'm not likely to update to the latest alpha over the weekend; if it still needs a review next week, I'll download a fresh copy of the latest alpha then, and see if it will work.
</p>
TicketvbraunTue, 17 Jan 2012 05:06:13 GMT
https://trac.sagemath.org/ticket/11614#comment:9
https://trac.sagemath.org/ticket/11614#comment:9
<p>
I've tried out the patch on bsd.math on top of sage-4.8.alpha3 and it works for me. Did you forget to rebuild the Sage library (<code>sage -b</code>)? Please post the whole output.
</p>
Ticketjohn_perryWed, 18 Jan 2012 15:04:40 GMT
https://trac.sagemath.org/ticket/11614#comment:10
https://trac.sagemath.org/ticket/11614#comment:10
<blockquote class="citation">
<p>
Did you forget to rebuild the Sage library (sage -b)?
</p>
</blockquote>
<p>
C'mon, I'm not <em>that</em> dumb! <code>X-D</code> But just to be sure, I tried building & testing again, and I encountered the same errors.
</p>
<p>
I do think the following output from building is pertinent (slightly edited for readability):
</p>
<pre class="wiki">setup.py:693: UserWarning: could not find dependency <string> included in
/Applications/sage-4.8.alpha4/local/lib/python/site-packages/Cython/Includes/libcpp/string.pxd.
I will assume it is a system C/C++ header.
warnings.warn(msg+' I will assume it is a system C/C++ header.')
setup.py:693: UserWarning: could not find dependency <vector> included in
/Applications/sage-4.8.alpha4/local/lib/python/site-packages/Cython/Includes/libcpp/vector.pxd.
I will assume it is a system C/C++ header.
warnings.warn(msg+' I will assume it is a system C/C++ header.')
</pre><p>
The file referenced is definitely there, and includes a declaration of C headers. If I inspect the file, I see a declaration of a <code>cppclass</code> from <code><string></code> that should cover it. Yet, when I run the doctests, I get boatloads of errors of this sort:
</p>
<pre class="wiki">Detected SAGE64 flag
Building Sage on OS X in 64-bit mode
sage -t "sage/tests/stl_vector.pyx"
Traceback (most recent call last):
File "/Users/user/.sage//tmp/stl_vector.py", line 18, in <module>
cython(open('sage/tests/stl_vector.pyx').read())
File "cython_c.pyx", line 32, in sage.misc.cython_c.cython (sage/misc/cython_c.c:645)
File "/Applications/Sage-4.6.2-OSX-64bit-10.6.app/Contents/Resources/sage/local
/lib/python/site-packages/sage/server/support.py", line 469, in cython_import_all
create_local_c_file=create_local_c_file)
File "/Applications/Sage-4.6.2-OSX-64bit-10.6.app/Contents/Resources/sage/local
/lib/python/site-packages/sage/server/support.py", line 446, in cython_import
create_local_c_file=create_local_c_file)
File "/Applications/Sage-4.6.2-OSX-64bit-10.6.app/Contents/Resources/sage/local
/lib/python/site-packages/sage/misc/cython.py", line 367, in cython
raise RuntimeError, "Error converting %s to C:\n%s\n%s"%(filename, log, err)
RuntimeError: Error converting /Users/user/.sage//temp/Users_MacBook_Pro.local/
9281//tmp_0.spyx to C:
Error converting Pyrex file to C:
------------------------------------------------------------
...
from sage.structure.sage_object cimport SageObject
from sage.rings.integer cimport Integer
from sage.libs.gmp.mpz cimport mpz_add_ui
from libcpp.vector cimport vector
from libcpp.string cimport string
^
------------------------------------------------------------
/Users/user/.sage/temp/Users_MacBook_Pro.local/9281/spyx
/_Users_user__sage_temp_Users_MacBook_Pro_local_9281_tmp_0_spyx
/_Users_user__sage_temp_Users_MacBook_Pro_local_9281_tmp_0_spyx_0.pyx:37:0:
'libcpp.string.pxd' not found
Error converting Pyrex file to C:
------------------------------------------------------------
...
from sage.structure.sage_object cimport SageObject
from sage.rings.integer cimport Integer
from sage.libs.gmp.mpz cimport mpz_add_ui
from libcpp.vector cimport vector
from libcpp.string cimport string
^
------------------------------------------------------------
/Users/user/.sage/temp/Users_MacBook_Pro.local/9281/spyx
/_Users_user__sage_temp_Users_MacBook_Pro_local_9281_tmp_0_spyx
/_Users_user__sage_temp_Users_MacBook_Pro_local_9281_tmp_0_spyx_0.pyx:37:0:
'string.pxd' not found
Error converting Pyrex file to C:
------------------------------------------------------------
...
from sage.structure.sage_object cimport SageObject
from sage.rings.integer cimport Integer
from sage.libs.gmp.mpz cimport mpz_add_ui
from libcpp.vector cimport vector
from libcpp.string cimport string
^
------------------------------------------------------------
/Users/user/.sage/temp/Users_MacBook_Pro.local/9281/spyx
/_Users_user__sage_temp_Users_MacBook_Pro_local_9281_tmp_0_spyx
/_Users_user__sage_temp_Users_MacBook_Pro_local_9281_tmp_0_spyx_0.pyx:37:27: Name
'string' not declared in module 'libcpp.string'
Error converting Pyrex file to C:
------------------------------------------------------------
...
sage: from sage.tests.stl_vector import stl_int_vector
sage: v = stl_int_vector()
"""
cdef vector[int] *data
cdef string *name
^
------------------------------------------------------------
/Users/user/.sage/temp/Users_MacBook_Pro.local/9281/spyx
/_Users_user__sage_temp_Users_MacBook_Pro_local_9281_tmp_0_spyx
/_Users_user__sage_temp_Users_MacBook_Pro_local_9281_tmp_0_spyx_0.pyx:50:9: 'string'
is not a type identifier
Error converting Pyrex file to C:
------------------------------------------------------------
...
Test suite for A vector of integers
vector<int>:
data[0] = 123
data[1] = 456
"""
self.data = new vector[int]()
^
------------------------------------------------------------
/Users/user/.sage/temp/Users_MacBook_Pro.local/9281/spyx
/_Users_user__sage_temp_Users_MacBook_Pro_local_9281_tmp_0_spyx
/_Users_user__sage_temp_Users_MacBook_Pro_local_9281_tmp_0_spyx_0.pyx:66:24: Operation
only allowed in c++
</pre><p>
and more just like it.
</p>
<p>
I suppose it's possible that this installation of sage is simply pooched, though that would seem very, very weird; other patches go in without a problem. It's a <a class="missing wiki">MacBook?</a> Pro running 10.6.8. On my Ubuntu box running sage 4.8.rc0, the installation passes.
</p>
<p>
I will run the full battery of doctests on the Ubuntu box, and also try a fresh install of 4.8.rc0 on the <a class="missing wiki">MacBook?</a> Pro. If it all works out, I'll give it a positive review.
</p>
TicketvbraunWed, 18 Jan 2012 16:32:50 GMT
https://trac.sagemath.org/ticket/11614#comment:11
https://trac.sagemath.org/ticket/11614#comment:11
<pre class="wiki">File "/Applications/Sage-4.6.2-OSX-64bit-10.6.app/Contents/Resources/sage/local
/lib/python/site-packages/sage/misc/cython.py", line 367, in cython
</pre><p>
Note the path, you are running cython from sage-4.6.2. I'm pretty sure that this ancient version does not have C++ support. It seems like you use the wrong sage (perhaps wrong <code>PATH</code>?) to run the doctests.
</p>
Ticketjohn_perryWed, 18 Jan 2012 16:45:02 GMT
https://trac.sagemath.org/ticket/11614#comment:12
https://trac.sagemath.org/ticket/11614#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11614#comment:11" title="Comment 11">vbraun</a>:
</p>
<blockquote class="citation">
<p>
Note the path, you are running cython from sage-4.6.2. I'm pretty sure that this ancient version does not have C++ support. It seems like you use the wrong sage (perhaps wrong <code>PATH</code>?) to run the doctests.
</p>
</blockquote>
<p>
That's really weird. I am in the correct path, and type
</p>
<pre class="wiki">
../../sage -b
</pre><p>
So I don't know why it's doing that. I'll look at it later.
</p>
Ticketjohn_perryWed, 18 Jan 2012 16:51:07 GMT
https://trac.sagemath.org/ticket/11614#comment:13
https://trac.sagemath.org/ticket/11614#comment:13
<p>
In any case, it works on 4.8.rc0 on the MacBook. Now I'm just waiting for Ubuntu's tests to finish.
</p>
Ticketjohn_perryWed, 18 Jan 2012 16:55:39 GMT
https://trac.sagemath.org/ticket/11614#comment:14
https://trac.sagemath.org/ticket/11614#comment:14
<p>
Sorry, I have to correct myself: that '''was''' 4.8.alpha4. I really have no idea why it didn't work before. Perhaps I typed `sage` instead of `../../sage`...? (as you say, the path.) Oh, well.
</p>
Ticketjohn_perryWed, 18 Jan 2012 20:46:30 GMTstatus, reviewer changed
https://trac.sagemath.org/ticket/11614#comment:15
https://trac.sagemath.org/ticket/11614#comment:15
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
changed from <em>Jeroen Demeyer</em> to <em>Jeroen Demeyer, John Perry</em>
</li>
</ul>
<p>
All tests passed! While I'm not an expert in Cython, what I see of the code makes sense, too. (& thanks for illustrating the use of `string`.) I'm giving it a positive review, but there are two points I want to ask about (& which perhaps should prevent positive review, but I'm not sure):
</p>
<ol><li>You should probably change this in the comments:
</li></ol><pre class="wiki">
Compare whith !``other!``.
</pre><blockquote>
<p>
That should be "with" not "whith". :-)
</p>
</blockquote>
<ol start="2"><li>The format opening the file is not quite in conformance with the format Jeroen requested. There is no list of authors, and the copyright notice is not quite the same. It's close enough that I'm okay with it, & I've seen other files that don't follow this pattern. Nevertheless, I should point it out.
</li></ol>
TicketvbraunThu, 19 Jan 2012 05:25:18 GMT
https://trac.sagemath.org/ticket/11614#comment:16
https://trac.sagemath.org/ticket/11614#comment:16
<p>
The updated patch fixes the typo and extends the header to match the template.
</p>
TicketjdemeyerThu, 19 Jan 2012 09:54:17 GMTmilestone changed
https://trac.sagemath.org/ticket/11614#comment:17
https://trac.sagemath.org/ticket/11614#comment:17
<ul>
<li><strong>milestone</strong>
changed from <em>sage-4.8</em> to <em>sage-5.0</em>
</li>
</ul>
TicketjdemeyerMon, 23 Jan 2012 14:13:18 GMTattachment set
https://trac.sagemath.org/ticket/11614
https://trac.sagemath.org/ticket/11614
<ul>
<li><strong>attachment</strong>
set to <em>trac_11614_stl_vectors.patch</em>
</li>
</ul>
<p>
Rebased to sage-5.0.beta1
</p>
TicketjdemeyerSun, 29 Jan 2012 11:17:39 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/11614#comment:18
https://trac.sagemath.org/ticket/11614#comment:18
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-5.0.beta2</em>
</li>
</ul>
Ticket