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

Numba #25

Merged
merged 7 commits into from
Mar 21, 2021
Merged

Numba #25

merged 7 commits into from
Mar 21, 2021

Conversation

yoelcortes
Copy link
Collaborator

@yoelcortes yoelcortes commented Feb 26, 2021

Hi Caleb,

This pull request introduces a chemicals.numba.mark_jit_unsafe decorator to mark functions unsafe to JIT compile. This decorator is then used to prevent jit compiling lookup functions and more (e.g. Tc, omega). The introduction of this decorator mainly helps for development purposes (no need to go back to the numba module to check which functions are blacklisted; or having to add blacklisted function names to the numba module).

Additionally, if you have Python 3.7 or greater, you can now import directly from numba via lazy loading:

>>> from chemicals.numba import Antoine, EQ105 # This works

Let me know what you think,
Thanks!

@yoelcortes yoelcortes self-assigned this Feb 26, 2021
@yoelcortes yoelcortes added the enhancement New feature or request label Feb 26, 2021
@yoelcortes
Copy link
Collaborator Author

I just made a correction in case the user's python version does not support lazy loading.

@yoelcortes yoelcortes marked this pull request as draft February 26, 2021 23:34
@CalebBell
Copy link
Owner

Hi Yoel,
I was aware of this liability/technical debt when I coded it, but never revisited the issue. That's one of the virtues of collaboration!
In short, I like it. It was a genuinely happy experience to see that the CI had ran and passed everything too! The cost of the function calls to add function names to a list dynamically is about ~50 us on my machine, which I think is a price worth paying.

There are two more places a similar approach could be taken:

  • Functions that can't be cached by numba right now
  • Functions that require source-code transformations to be numba-compatible

For the first, I am hopeful numba 0.53 will allow caching functions that accept functions as arguments. I am incredibly excited for this. It also adds substantial speed benefits to them. However, some functions such as those that call scipy.special methods still can't be cached. There are also some issues with this code, one of which I have reported to numba: numba/numba#6772

I am doing testing with numba 0.53 now, so if it's OK with you I'll leave this as a PR and merge it in with all other needed changes!

Sincerely,
Caleb

@yoelcortes
Copy link
Collaborator Author

Awesome! I just added an optional "cache" keyword argument to the chemicals.mark_jit_unsafe decorator to denote a function as unsafe to cache (as opposed to completely blacklisting it).

I didn't know that numba was close to implementing functions as arguments to jit compiled functions. That's a real improvement, thanks for letting me know.

Feel free to pull whenever you're ready,
Thanks!

…he future

Made uncacheable function separate for simplicity - may be able to take out depending on numba's 0.53 bugfix cycle
Removed mark_numba_incompatible from __all__ as not part of public API
Removed mark_numba_incompatible from __init__ because the auto-rename feature of PyCharm couldn't trace the import logic to allow it easy rename
@CalebBell CalebBell marked this pull request as ready for review March 21, 2021 21:48
@CalebBell CalebBell merged commit 7795790 into master Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants