Opened 6 years ago
Closed 6 years ago
#4571 closed defect (fixed)
[with patch, positive review] merge sage's numpy.pxd with the cython numpy.pxd
Reported by: | jason | Owned by: | mabshoff |
---|---|---|---|
Priority: | major | Milestone: | sage-4.1.1 |
Component: | numerical | Keywords: | |
Cc: | robertwb, dagss | Merged in: | Sage 4.1.1.alpha1 |
Authors: | Robert Bradshaw, Dag Sverre Seljebotn | Reviewers: | Robert Bradshaw, Dag Sverre Seljebotn, Minh Van Nguyen |
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
[15:53] <robertwb> yep, we should merge them
Attachments (2)
Change History (19)
comment:1 Changed 6 years ago by robertwb
comment:2 Changed 6 years ago by robertwb
- Summary changed from merge sage's numpy.pxd with the cython numpy.pxd to [with patch, needs work] merge sage's numpy.pxd with the cython numpy.pxd
Changed 6 years ago by robertwb
comment:3 Changed 6 years ago by robertwb
Preliminary patch, need a new Cython spkg for it to work all the way.
Changed 6 years ago by dagss
comment:4 follow-up: ↓ 11 Changed 6 years ago by dagss
- Cc dagss added
Here are some more fixes. With this, and the latest state of the
repo, all the relevant tests seem to pass. I do get two failures (due to Cython upgrade? something else?), see test.log in
/home/dagss/sage-4.0.2-sage.math.washington.edu-x86_64-Linux
Once this works let's tag http://hg.cython.org/cython as 0.11.2.1 for #6438?
comment:5 Changed 6 years ago by dagss
Note (re: discussion on Cython ML) that shape in numpy.pxd is still a field, long story (well, it is performance critical), and I shouldn't change it.
comment:6 Changed 6 years ago by dagss
Another quick comment: With this, full Cython/NumPy? functionality is available from the notebook and works well.
comment:7 Changed 6 years ago by robertwb
Those errors seem completely unrelated, I'll see if I get them too. Other than that, it looks really good.
I've used Cython + NumPy? from the notebook before, as long as you weren't in sage.ext then "cimport numpy" worked as expected (though there was a point where it didn't).
comment:8 Changed 6 years ago by robertwb
Hmm... I also got
The following tests failed: sage -t -long devel/sage-main/sage/rings/polynomial/polynomial_template.pxi # 1 doctests failed sage -t -long devel/sage-main/sage/interfaces/sage0.py # 1 doctests failed
comment:9 Changed 6 years ago by robertwb
- Component changed from build to numerical
- Summary changed from [with patch, needs work] merge sage's numpy.pxd with the cython numpy.pxd to [with patch, positive review] merge sage's numpy.pxd with the cython numpy.pxd
Looks good. The single doctest failure was minor, fixed in #6438.
comment:10 Changed 6 years ago by mvngu
- Reviewers set to Robert Bradshaw, Dag Sverre Seljebotn
comment:11 in reply to: ↑ 4 Changed 6 years ago by mvngu
Replying to dagss:
Here are some more fixes. With this, and the latest state of the
<SNIP>
The patch 4571-more-fixes.patch doesn't contain a commit message and your username is not on the patch. Your username should follow the format:
Full Name <email@somewhere.com>
The username makes it easy to identify the patch, when it is committed, as your contribution.
comment:12 Changed 6 years ago by mvngu
- Summary changed from [with patch, positive review] merge sage's numpy.pxd with the cython numpy.pxd to [with patch, needs work] merge sage's numpy.pxd with the cython numpy.pxd
With both patches applied in the following order:
- 4571-numpy-pxd.patch
- 4571-more-fixes.patch
I see the following build failure:
[mvngu@sage sage-4.1.1.alpha0]$ ./sage -br main ---------------------------------------------------------- sage: Building and installing modified Sage library files. Installing c_lib scons: `install' is up to date. Updating Cython code.... Building modified file sage/finance/time_series.pyx. Building modified file sage/matrix/change_ring.pyx. Building modified file sage/matrix/matrix_complex_double_dense.pyx. Building modified file sage/matrix/matrix_double_dense.pyx. Building modified file sage/matrix/matrix_real_double_dense.pyx. Building modified file sage/modules/vector_complex_double_dense.pyx. Building modified file sage/modules/vector_double_dense.pyx. Building modified file sage/modules/vector_real_double_dense.pyx. Building sage/rings/polynomial/real_roots.pyx because it depends on sage/modules/vector_double_dense.pxd. Building sage/stats/hmm/chmm.pyx because it depends on sage/matrix/matrix_double_dense.pxd. Building sage/stats/hmm/hmm.pyx because it depends on sage/matrix/matrix_double_dense.pxd. python `which cython` --embed-positions --incref-local-binop -I/scratch/mvngu/release/sage-4.1.1.alpha0/devel/sage-main -o sage/finance/time_series.c sage/finance/time_series.pyx warning: /scratch/mvngu/release/sage-4.1.1.alpha0/devel/sage-main/sage/finance/time_series.pyx:1722:24: cdef variable 'j' declared after it is used Error converting Pyrex file to C: ------------------------------------------------------------ ... [20.0000, -3.0000, 4.5000, -2.0000] """ cnumpy.import_array() #This must be called before using the numpy C/api or you will get segfault cdef cnumpy.npy_intp dims[1] dims[0] = self._length cdef cnumpy.ndarray n = cnumpy.PyArray_SimpleNewFromData(1, dims, cnumpy.NPY_DOUBLE, self._values) ^ ------------------------------------------------------------ /scratch/mvngu/release/sage-4.1.1.alpha0/devel/sage-main/sage/finance/time_series.pyx:1862:72: Cannot convert 'numpy.npy_intp [1]' to Python object Error converting Pyrex file to C: ------------------------------------------------------------ ... [20.0000, -3.0000, 4.5000, -2.0000] """ cnumpy.import_array() #This must be called before using the numpy C/api or you will get segfault cdef cnumpy.npy_intp dims[1] dims[0] = self._length cdef cnumpy.ndarray n = cnumpy.PyArray_SimpleNewFromData(1, dims, cnumpy.NPY_DOUBLE, self._values) ^ ------------------------------------------------------------ /scratch/mvngu/release/sage-4.1.1.alpha0/devel/sage-main/sage/finance/time_series.pyx:1862:97: Cannot convert 'double *' to Python object Error running command, failed with status 256. sage: There was an error installing modified sage library code.
comment:13 follow-up: ↓ 14 Changed 6 years ago by robertwb
Looks like it'll have to be rebased, worked fine on vanilla 4.1. Where's the .tar on sage.math?
comment:14 in reply to: ↑ 13 Changed 6 years ago by mvngu
Replying to robertwb:
Looks like it'll have to be rebased, worked fine on vanilla 4.1. Where's the .tar on sage.math?
Source and binary versions are up at
http://sage.math.washington.edu/home/mvngu/release/sage-4.1.1.alpha0.tar
http://sage.math.washington.edu/home/mvngu/release/sage-4.1.1.alpha0-sage.math.washignton.edu-x86_64-Linux.tar.gz
comment:15 Changed 6 years ago by robertwb
comment:16 Changed 6 years ago by robertwb
- Summary changed from [with patch, needs work] merge sage's numpy.pxd with the cython numpy.pxd to [with patch, needs review] merge sage's numpy.pxd with the cython numpy.pxd
comment:17 Changed 6 years ago by mvngu
- Merged in set to Sage 4.1.1.alpha1
- Resolution set to fixed
- Reviewers changed from Robert Bradshaw, Dag Sverre Seljebotn to Robert Bradshaw, Dag Sverre Seljebotn, Minh Van Nguyen
- Status changed from new to closed
- Summary changed from [with patch, needs review] merge sage's numpy.pxd with the cython numpy.pxd to [with patch, positive review] merge sage's numpy.pxd with the cython numpy.pxd
dagss: The patch 4571-more-fixes.patch doesn't contain your username. I'm committing it in your name. Merged in this order
- the SPKG at #6438
- 4571-numpy-pxd.patch
- 4571-more-fixes.patch
See http://trac.cython.org/cython_trac/ticket/339