Replace the deprecated @generated_jit by @njit #439
Conversation
Issues detected by flake8: 4 E275 missing whitespace after keyword 1 E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()` 6 F401 'random' imported but unused 5 F841 local variable 'biv2' is assigned to but never used
* scipy.e is deprecated, use numpy.e instead see scipy/scipy@6e207a1 * Migrate to the latest github actions * Replace the unsupported python 3.5 with 3.8 * Use python 3.11 instead of 3.8, and 3.12 instead of 3.9
Remove leftovers from python 2 compatibility Co-authored-by: Eric Wieser <[email protected]>
Done by avoid the numpy.result_type (not supported by numba). Instead do the array multiplication at-once, then use the result dtype. This requires some numpy array-indexing tricks.
The test_function_cache is rewritten to use @njit, which may not make much sense for particular test.
In various `__repr__()` implementations, replace 'list(ndarray)' with 'ndarray.tolist()', in order to avoid inclusion of type information (like 'np.int64(1)') by the latest numpy version. This also fixes test_clifford.TestPrettyRepr and test_transformations pytest-s, as they rely on hardcoded results from IPython.lib.pretty. Note: numpy>=2.0 is allowed by numba>=0.60.
|
Hi @arsenovic, I understand, this is not a good practice, so if you don't work that way, I'll just scrap the PR and rebase the branch on top of the current "master". |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #439 +/- ##
==========================================
- Coverage 71.35% 70.92% -0.44%
==========================================
Files 73 74 +1
Lines 9461 9420 -41
Branches 1313 1208 -105
==========================================
- Hits 6751 6681 -70
- Misses 2539 2564 +25
- Partials 171 175 +4 ☔ View full report in Codecov by Sentry. |
|
great. i dont know how much the jit'ing is worth in terms of complexity/debug/maintence vs speed. i think the most important thing is to lower the barrier to new additions and attract a new team. i appreciate you working on this. |
|
I'm glad I can help. EDIT: The jit'ing should seriously speed-up the code, esp. as multiplication is a key operation, that is used everywhere. |
The branch is based on the PR#438 "Fix GitHub workflow CI", instead of on the
master, just to run the CI tests. If it's too messy, I'll create another branch/PR.Some details about the changes
Seems like, the only reason for using the
@generated_git(instead of@njit) is thatnumpy.result_type()is not supported by numba.Here, the use of this function is avoid by rewriting the original
mv_mult.mult_inner()to:k_list/m_listin a single ndarray operationdtypeis selected by numpy as anoutputdtypeThis makes both
mv_mult()instances much more compact and faster in DISABLE_JIT mode. In_get_mult_function_runtime_sparse.mv_mult(), there is an attempt to mimic the original trick to avoid multiplication-by-zero.Additional comments
As an alternative, it is possible to use the
@overloadapproach, as recommended herehttps://numba.readthedocs.io/en/stable/reference/deprecation.html#replacement. This will make the code somehow similar to existing implementation, but it would become even more complicated.
The test in
test_function_cache.pyis just hacked to make it pass. I'm not sure about the purpose of this test.The GitHub workflow is still NOT expected to be fully successful, as it needs an extra fix for
numpy>=2.0.This extra issue is related to the printing of numeric scalars with their type information (like 'np.int64(1)').
It become quite messy, please excuse me for it.
Fixes #430