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

First numba tests. #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

schodge
Copy link
Contributor

@schodge schodge commented Aug 5, 2014

Experiment in implementing simulateDSM in Numba to improve performance. Currently craters performance (37s on the test!), can't accept keyword arguments (just pass the values in the right place), plays badly with some type checking (disabled it), and I'm having trouble getting the second argument tuple specified correctly so I can specify all types instead of simply leaving it up to the library.

May work faster if I put individual JIT blocks in the function. Consider this a long term work in progress.

@ggventurini
Copy link
Owner

Hi Shayne,

I find this to be very nice as I believe numba may nicely circumvent all troubles with compilers and system configuration on Windows. I also really like the fact that there is no strict dependency on numba introduced in the package.

I must add I have little to no experience with numba, mostly what I know comes from the documentation and a couple of tutorials, no hands-on experience, so I am afraid I am of little help with this.

That said, I will still run the changes you suggest on my machine, maybe try a few changes and post my results here as soon as time allows.

I am willing to merge this in a numba-dedicated branch right now and then merge into master when we're confident about the implementation details.

Will write again soon.

Best,

GV

@schodge
Copy link
Contributor Author

schodge commented Aug 6, 2014

Separate branch sounds good.

http://jakevdp.github.io/blog/2013/06/15/numba-vs-cython-take-2/ is where I got the idea, it seems it could exceed Cython in speed. I'm not sure how (or if you'd need to) also interface it with BLAS.

You have more experience with Numba than me then - I'm finding the documentation a bit scattered. I may need to add a helper function to split out the arguments for the two types, and change the kwargs to normal arguments - though that'll mess things up if the first kwarg is missing - so I have definitive types. Mostly, though, I can't figure out why I can't use an array to catch the tuple.

As for the no hard dependency, I just copied your Cython code :)

@ggventurini
Copy link
Owner

I have merged this pull request in numba_tests. It runs alright -- except the performance problem you mention -- on my test setup.

@schodge
Copy link
Contributor Author

schodge commented Aug 14, 2014

EDIT: Tied to PR 43, because I didn't recommit to the same branch.

Added a test script. For reasons I don't fully understand, I had to go remove . from all imports, even trying to use python -m otherwise python would complain it was a relative import in a non-module. And this wasn't just the modules I was importing in numba_timing_tests.py, but anything they imported, hence the various changes.

Anyway, run outcome - first two numbers are numba runs in seconds, the second two are pure python runs. Not the way things are supposed to work!

c:\github\python-deltasigma\deltasigma (nt_2)
λ python numba_timing_tests.py
C:\Anaconda\lib\site-packages\deltasigma\_config.py:40: UserWarning: Numpy did not detect the BLAS library in the system
  warn("Numpy did not detect the BLAS library in the system")
C:\Anaconda\lib\site-packages\deltasigma\_config.py:55: UserWarning: Cannot find the path for 'cblas.h'. You may set it using the environment variable BLAS_H.
NOTE: You need to pass the path to the directories were the header files are, not the path to the files.
  warn("Cannot find the path for 'cblas.h'. You may set it using the environment variable "
20.0362619895
21.3940657678
0.866870953264
0.858791593329

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

Successfully merging this pull request may close these issues.

2 participants