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.

Sign up for GitHub

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

Jump to bottom

Remove all linpack_lite code and replace with LAPACK routines #3916

Closed
wants to merge 12 commits into from
Closed

Remove all linpack_lite code and replace with LAPACK routines #3916

wants to merge 12 commits into from

Conversation

chatcannon
Copy link
Contributor

This was suggested in #3834 when I tried to fix some compiler warnings in the linpack_lite code.

To reduce the amount of work I had to do I also removed a lot of odepack code which is not used anywhere in SciPy.

@chatcannon
Removed some unused code from ODEPACK
b0daf1c 
This will reduce the burden of eliminating compiler warnings and
replacing LINPACK calls with LAPACK
@chatcannon
Replaced DGBFA and DGEFA with DGBTRF and DGETRF
79eb9ec
@chatcannon
Replaced DGESL with DGETRS
7ba6896
@chatcannon
Replaced DGBSL with DGBTRS
59ee177
@chatcannon
Replaced ZGEFA and ZGESL with ZGETRF and ZGETRS
98f3f96
@chatcannon
Replaced ZGBFA and ZGBSL with ZGBTRF and ZGBTRS
3bc191c
@chatcannon
Replaced linpack DGTSL with lapack DGTSV
777cbe3
@chatcannon
Rationalised the numpy.distutils stuff in integrate setup.py
1d98144 
Assuming that lapack_opt will include all BLAS libraries and headers
If this is not the case then the blas_opt info will have to be
added to the libraries, include directories etc.
@chatcannon
Fixed a typo in _odepackmodule.c
f76057c
@chatcannon
Attempted to update the bento stuff for linpack_lite -> LAPACK
c7b1ad7
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling c7b1ad7 on chatcannon:replace_linpack into 96b2461 on scipy:master.

@argriffing
Copy link
Contributor

travis is having problems getting cython again

Downloading/unpacking Cython
  Could not find any downloads that satisfy the requirement Cython
Cleaning up...
No distributions at all found for Cython
Storing debug log for failure in /home/travis/.pip/pip.log
The command "pip install --find-links $WHEELHOUSE --no-index Cython numpy$NUMPYSPEC" failed and exited with 1 during .
Your build has been stopped.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling c7b1ad7 on chatcannon:replace_linpack into 96b2461 on scipy:master.

@WarrenWeckesser
Copy link
Member

Closed and reopened to rerun tests.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling c7b1ad7 on chatcannon:replace_linpack into 96b2461 on scipy:master.

@ev-br
Copy link
Member

ev-br commented Aug 31, 2014 

$ python runtests.py -s integrate
Building, see build.log...
Build OK
Running unit tests for scipy.integrate
NumPy version 1.9.0.dev-fe3410f
NumPy is installed in /home/br/virtualenvs/scipy-dev/local/lib/python2.7/site-packages/numpy
SciPy version 0.15.0.dev-c7b1ad7
SciPy is installed in /home/br/installs/scipy/build/testenv/lib/python2.7/site-packages/scipy
Python version 2.7.3 (default, Sep 26 2013, 20:03:06) [GCC 4.6.3]
nose version 1.3.4
....................................FK......................................
======================================================================
FAIL: test_integrate.test_odeint_banded_jacobian
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/br/virtualenvs/scipy-dev/local/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/br/installs/scipy/build/testenv/lib/python2.7/site-packages/scipy/integrate/tests/test_integrate.py", line 589, in test_odeint_banded_jacobian
    assert_array_equal(info0['nje'], info1['nje'])
  File "/home/br/virtualenvs/scipy-dev/local/lib/python2.7/site-packages/numpy/testing/utils.py", line 734, in assert_array_equal
    verbose=verbose, header='Arrays are not equal')
  File "/home/br/virtualenvs/scipy-dev/local/lib/python2.7/site-packages/numpy/testing/utils.py", line 660, in assert_array_compare
    raise AssertionError(msg)
AssertionError: 
Arrays are not equal

(mismatch 20.0%)
 x: array([12, 13, 14, 14, 15], dtype=int32)
 y: array([12, 13, 14, 14, 19], dtype=int32)

----------------------------------------------------------------------
Ran 76 tests in 3.193s

@pv pv added the needs-work label Aug 31, 2014
@WarrenWeckesser
Copy link
Member

@ev-br: I'll take a look at the odeint test failure.

@WarrenWeckesser
Copy link
Member

I removed the failing assertion in #3940, for the reasons given in the commit message. Once that is merged, try testing integrate with this PR again.

@ev-br
Copy link
Member

ev-br commented Aug 31, 2014

seems to be fixing it indeed locally --- I'll wait until TravisCI goes green just to be on the safe side though.

@ev-br ev-br added this to the 0.15.0 milestone Sep 2, 2014
@ev-br
Copy link
Member

ev-br commented Sep 5, 2014

Spoke too soon, it seems. Rebased on master, which includes gh-3940, test_banded_ode_solvers.py and test_integrate.py segfault somewhere in the vode solvers.
Here's the output: https://gist.github.com/ev-br/6104929edbc4c4121876

git bisect indicates 05981b6dbc9e60b64a46935561082cc67ced4ad0 as a first bad commit.

@chatcannon
Copy link
Contributor Author

I got an error that looked similar at some point but it went away when I deleted the build directory and rebuilt from scratch. Can you check if that also happens for you?

@chatcannon
Copy link
Contributor Author

Scratch that -- I've just done a full rebuild and I get the error too.

@chatcannon
Copy link
Contributor Author

N.B. The Travis 'error' was that the full test suite with coverage took over 50 minutes.

@WarrenWeckesser
Copy link
Member

Close and reopen to rerun tests.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 47c7cfa on chatcannon:replace_linpack into a37c7f1 on scipy:master.

@ev-br
Copy link
Member

ev-br commented Sep 5, 2014

Great work @chatcannon! Works for me locally now.

What system are you on, which BLAS/LAPACK? TravisCI is a 32-bit Ubuntu AFAIK, I can only test it on a 64-bit ubuntu with stock ATLAS at the moment, it might be worth it to have a data point from at least one other system.

One minor quibble, hopefully the last one: can you get rid of that merge commit, df131d8? The git history is really much cleaner if you rebase on master --- git fetch upstream, git rebase upstream/master, then reaply 47c7cfa.

@ev-br ev-br removed the needs-work label Sep 5, 2014
@chatcannon chatcannon mentioned this pull request Sep 6, 2014
Closed
@chatcannon
Copy link
Contributor Author

I wasn't sure how to change the commit history of a github remote branch so I created a new branch with a new pull request instead

@chatcannon
Copy link
Contributor Author

On my laptop I'm running Debian testing with default numpy and atlas.
On my desktop, Funtoo (a Gentoo derivative) with atlas-3.10.1-r2 from the gentoo-science overlay and a numpy install tweaked to re-enable the ATLAS specific code. (The default gentoo numpy disables this in a half-hearted attempt to make it possible to swap BLAS implementations without recompiling, I simply removed these patches.)

@chatcannon chatcannon closed this Sep 6, 2014
@chatcannon
Copy link
Contributor Author

Closing this pull request; use #3971 instead

ev-br added a commit that referenced this pull request Sep 9, 2014
@ev-br
Merge branch 'pr/3971'
4b0073f 
review in #3971 and
#3916
@chatcannon chatcannon deleted the replace_linpack branch September 9, 2014 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement A new feature or improvement scipy.integrate
Projects
None yet
Milestone
0.15.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@chatcannon @coveralls @argriffing @WarrenWeckesser @ev-br @pv @rgommers