Skip to content

Python: Add support for handling numpy vectors and JIT compilation with Numba (Closes #34)#57

Merged
dmey merged 18 commits intomasterfrom
dmey/numba-support
Apr 11, 2020
Merged

Python: Add support for handling numpy vectors and JIT compilation with Numba (Closes #34)#57
dmey merged 18 commits intomasterfrom
dmey/numba-support

Conversation

@dmey
Copy link
Contributor

@dmey dmey commented Apr 8, 2020

This PR adds the following improvements:

  1. Support for handling numpy vectors as inputs/outputs and greater run time performance using Numba: when Numba is found, all functions are compiled using the Numba just in time (JIT) compiler. Note that there is a small overhead the first time the code is run due to the JIT.

  2. CI: jobs are now split into 1-2: Python (with/without Numba); 3: C#; 4: R

  3. README: content and style.

@dmey dmey added this to the 2.4.0 milestone Apr 8, 2020
@dmey dmey requested a review from didierthevenard April 8, 2020 09:47
@dmey dmey self-assigned this Apr 8, 2020
@dmey dmey linked an issue Apr 8, 2020 that may be closed by this pull request
@C-H-Simpson
Copy link

An unfortunate effect of wrapping in this way is that the docstrings are hidden by the wrapper, so e.g. help(psy.GetRelHumFromVapPres) gives "Help on DUFunc in module numba.npyufunc.dufunc object...". There may be a way of fix this, I don't know.

@dmey
Copy link
Contributor Author

dmey commented Apr 8, 2020

An unfortunate effect of wrapping in this way is that the docstrings are hidden by the wrapper, so e.g. help(psy.GetRelHumFromVapPres) gives "Help on DUFunc in module numba.npyufunc.dufunc object...". There may be a way of fix this, I don't know.

That's right -- it looks like this will be a limitation so will should probably ask people to just consult the online API -- not great but given the other options this seems reasonable.

@dmey
Copy link
Contributor Author

dmey commented Apr 8, 2020

@C-H-Simpson how did you handle return type for the Calc functions (e.g. CalcPsychrometricsFromRelHum)? I may need to change those for Numba...

@C-H-Simpson
Copy link

C-H-Simpson commented Apr 9, 2020

@dmey The Calc... functions can't be straightforwardly vectorized because they return a tuple.
They should be excluded from the list of functions to wrap with vectorized. As the functions it calls to do the calculations are vectorized, it will return a tuple of vectors.
I have tested this and it works.
I don't think there would be much of a speed-up from wrapping the Calc... functions in numba anyway, as they don't actually do any calculations themselves.

So I would recommend just not vectorizing them. Or rather, they are already vectorized because they are calling vectorized functions.

@dmey
Copy link
Contributor Author

dmey commented Apr 9, 2020

@C-H-Simpson thanks -- this makes perfect sense! I will just need to check a few more things/tests so I will need a bit longer before we can release a new version -- hopefully will not be too long though!

@dmey dmey changed the title Python: Add support for handling vectorised operations with Numpy + Add JIT compile support with Numba (Closes #34) Python: Add support for handling vectorised operations and JIT compilation with Numba (Closes #34) Apr 9, 2020
@dmey dmey modified the milestones: 2.4.0, 2.5.0 Apr 9, 2020
@dmey
Copy link
Contributor Author

dmey commented Apr 9, 2020

@didierthevenard this PR is now ready to be merged in -- all tests pass. Please review when you have a moment and let me know for any issue.

@dmey dmey changed the title Python: Add support for handling vectorised operations and JIT compilation with Numba (Closes #34) Python: Add support for handling numpy vectors and JIT compilation with Numba (Closes #34) Apr 10, 2020
Copy link
Contributor

@didierthevenard didierthevenard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments added to the proposed changes. No issue with the code itself, it's mostly that new code that has been added (around the use of numba and vectorization) needs to have explaining comments.

@dmey
Copy link
Contributor Author

dmey commented Apr 11, 2020

A few comments added to the proposed changes. No issue with the code itself, it's mostly that new code that has been added (around the use of numba and vectorization) needs to have explaining comments.

👍. Please see now.

@dmey dmey requested a review from didierthevenard April 11, 2020 07:25
Copy link
Contributor

@didierthevenard didierthevenard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the added comments. I fixed one typo and added a general comment about the last section in the code (the entire chunk that deals with Numba) - feel free to change if you can think of a better wording. Also wondering about the usefulness of the last line in the code.

@dmey
Copy link
Contributor Author

dmey commented Apr 11, 2020

Thanks for the added comments. I fixed one typo and added a general comment about the last section in the code (the entire chunk that deals with Numba) - feel free to change if you can think of a better wording. Also wondering about the usefulness of the last line in the code.

Thanks -- looks good. Unused var removed in af64406.

@dmey dmey merged commit 1500caa into master Apr 11, 2020
@dmey dmey deleted the dmey/numba-support branch April 11, 2020 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Add support for handling vectorised operations in Python

3 participants