-
Notifications
You must be signed in to change notification settings - Fork 53
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
Corrfunc 3.0 release discussion #328
Comments
I see a packaging and build-system overhaul as the top priority for improving the user experience in Corrfunc 3. I have a student working on a prototype right now that uses meson, meson-python, and pybind11. The basic setup seems to work, and we're looking at runtime SIMD dispatch now. I would love to be able to build and distribute binaries for Corrfunc 3! When the code is ready to share, I'll link it here. |
That sounds great! Any reason to go with pybind11 rather than cffi? Is that to future-proof in case we switch to C++ code? |
Yeah, future-proofing was part of it. Pybind11 is also a better developer experience; it tends to emit informative error messages instead of segfaults! It's also NumPy-aware, which makes it easy to enforce conditions on the memory order and shape of arrays. |
@lgarrison Would nanobind be a drop-in replacement for pybind11? I really like how @dfm used templating within simple functions with C++ and nanobind - see here. |
I think it would be great to use nanobind! Meson isn't an officially supported build system for nanobind so I decided to stick with pybind11 for the summer project, but I don't think it would be very hard to figure nanobind out. At the API level, it would probably be nearly a drop-in replacement since our usage is simple. |
Excellent! What do you think about adding Typing into the function parameters? Looking at astropy code, type hints do seem to remove a lot of redundant code from the function body (into the function declaration ... ) |
I'm all for type hints! However, type hints don't have any effect at runtime, so it wouldn't let us remove code from the function body. They're just used by type checkers, language servers, and other static analysis tools. I do think we could improve/change the Python API, including type hints. E.g. some positional arguments could probably be keyword arguments, and the return type should probably be a columnar container like a pandas df or an Astropy table (or maybe just a dict/dataclass of arrays). It's something we can iterate on once we get the harder (build system) stuff working. |
@lgarrison To guide what needs to/could happen for Corrfunc 3.0, here is my list:
Essential (?)
Remove python2 support completely
from __future__
type constructs from python codeAdd modern packaging with
pyproject.toml
/meson
/ whatever-else-we-should-be-usingSolve the multiple OpenMP runtime library issue
Possibly (?)
Add
numbins
optimisation that only uses the number of bins necessary (as determined by the min. and max. distance possible between two cell-pairs)nbins-1
)simd reduction
clause, or simply a+=
. This could be in conjunction with a bit-setting routine that sets bits and shifts to count up to 64 pairs before callingpopcnt
. However, I don't see how to do that without a LOT OF code duplication.Change the OpenMP parallelization to go over cell-pairs (improves cache utilisation, reduces memory requirement -> we can increase the max-bin-ref factors)
generate_cell_pairs
function that returns the potential neighbouring cell-pairs for any given primary cellMay be (?)
Add ARM64 kernels
-march=armv8a
(or-mcpu=apple-m1
) to CFLAGSRename package to
corrfunc
and release conda wheelstarget_clones
to all functionslinux + x86_64
)import Corrfunc
still works (how?)This is also open for community discussion. If anyone has opinions on what should go in, please do add a comment.
The text was updated successfully, but these errors were encountered: