CalebBell / chemicals

chemicals: Chemical database of Chemical Engineering Design Library (ChEDL)
MIT License
186 stars 36 forks source link

Numba #25

Closed yoelcortes closed 3 years ago

yoelcortes commented 3 years ago

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 commented 3 years ago

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

CalebBell commented 3 years ago

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:

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: https://github.com/numba/numba/issues/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 commented 3 years ago

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!