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

Refactor Numba, part I #5197

Merged
merged 170 commits into from Feb 6, 2020
Merged

Refactor Numba, part I #5197

merged 170 commits into from Feb 6, 2020

Conversation

stuartarchibald
Copy link
Contributor

@stuartarchibald stuartarchibald commented Feb 4, 2020

The basic idea in this PR is to start organising the Numba code base, this will be a multi-PR effort.

This PR:

  • Adds new submodules:
    • core - for core Numba compiler code e.g. compilation pipeline, type inference
    • np - for NumPy re-implementations e.g. np.linalg
    • misc - for miscellaneous re-implementations and other features/utilities e.g. CFFI support
    • parfors - for the Parallel Accelerator technology
    • stencils - for the @stencil decorator implementations
    • cpython for CPython implementation and Python language features e.g. tuples.
    • experimental for experimental features e.g. jitclass

It then contains largely (99%+) file-at-once moves of each file in the Numba code base to somewhere appropriate. No refactoring is performed other than that necessary to fix imports or fix circular dependencies (apart from <1%). The numba.tests submodule remains unchanged as do most of the C-extensions. The <1% refactoring is to permit re-exporting the extension API as numba.extension and to fix up a few other issues.

@stuartarchibald stuartarchibald marked this pull request as ready for review February 5, 2020 20:09
@stuartarchibald stuartarchibald added the Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm label Feb 5, 2020
@sklam
Copy link
Member

sklam commented Feb 5, 2020

smoketesting at numba_smoketest_cpu_11

@sklam sklam added BuildFarm Passed For PRs that have been through the buildfarm and passed 5 - Ready to merge Review and testing done, is ready to merge and removed Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm 3 - Ready for Review labels Feb 6, 2020
@eric-wieser
Copy link
Contributor

eric-wieser commented Feb 7, 2020

This change breaks both the sparse and clifford numba code (links are to CI runs). To name a few issues:

  • The disappearance of numba.numpy_support.from_dtype
  • The disappearance of numba.numpy_support.as_dtype
  • The import from numba.targets.imputils import impl_ret_borrowed, lower_constant no longer working

@stuartarchibald
Copy link
Contributor Author

This change breaks both the sparse and clifford numba code (links are to CI runs). To name a few issues:

* The disappearance of `numba.numpy_support.from_dtype`

* The disappearance of `numba.numpy_support.as_dtype`

* The import `from numba.targets.imputils import impl_ret_borrowed, lower_constant` no longer working

Thanks for reporting these, we anticipated that there would be some knock-on churn is code bases consuming Numba internals and can only apologise, but is it unfortunately necessary. The Numba code base was becoming unsustainable and increasingly hard to manage, house-keeping work was needed https://groups.google.com/a/continuum.io/forum/#!topic/numba-users/CY7oXibUwXA. Part of this will be to define a public set of APIs as we identified users doing exactly as the above projects have done, using bits of Numba internals to make things work, and this is not sustainable for anyone.

Of the things listed above, the good news is that none of the things above have disappeared, they've merely moved and will likely be exported via the public API.

@eric-wieser
Copy link
Contributor

Not a complaint, just though I'd record the downstream impact here. I assume we'll end up with some checking of distutils.version.LooseVersion(numba.__version__) to get around this once 0.49 is released.

@eric-wieser
Copy link
Contributor

Although I'd perhaps encourage you to add back numba.numpy_support to re-export the old functions, and emit a deprecation warning upon import - I imagine that file is used by the most downstream projects.

dicta added a commit to dicta/cusignal that referenced this pull request Apr 8, 2020
numba/numba#5197 refactors many of Numba's
submodules. Mirror the required import changes in cusignal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge Review and testing done, is ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants