Package updates following upstream 3.1.0 changes#17
Conversation
|
I intend to merge this in a few hours. It restores the package to working with current bandicoot sources and OpenCL, plus CUDA where available (e.g. locally for me). |
There was a problem hiding this comment.
Pull request overview
This PR updates the RcppBandicoot package to align with upstream Bandicoot library version 3.1.0 changes. The updates include changes to the build system configuration, a new GPU initialization function, bug fixes in the kernel loading code, and changes to default backend priority.
- Added new
gpu_initialize()function to allow explicit GPU backend initialization - Changed default backend priority from OpenCL-first to CUDA-first in configure scripts
- Fixed bug in OpenCL kernel source loading where wrong variable was used
- Updated build configuration to use new
COOT_TARGET_OPENCL_VERSIONandCOOT_KERNEL_SOURCE_DIRmacros
Reviewed changes
Copilot reviewed 5 out of 663 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coot.cpp | New file implementing gpu_initialize() wrapper around coot::coot_init() |
| src/RcppExports.cpp | Auto-generated Rcpp bindings for new gpu_initialize() function |
| src/Makevars.in | Updated compiler flags with COOT_TARGET_OPENCL_VERSION and COOT_KERNEL_SOURCE_DIR, removed OpenMP from CXXFLAGS |
| src/Makevars | Generated build configuration with system-specific paths (should not be committed) |
| inst/include/bandicoot_bits/opencl/kernel_src.hpp | Fixed bug using wrong variable name when constructing kernel file path |
| configure.ac | Changed backend priority to prefer CUDA over OpenCL |
| configure | Generated configure script reflecting CUDA priority change |
| R/rcppbandicoot-package.R | Package initialization code commented out (previously set kernel paths) |
| R/RcppExports.R | Added R wrapper for gpu_initialize() function without documentation |
| DESCRIPTION | Added Dirk Eddelbuettel as author, reformatted Authors@R, removed C++14 from SystemRequirements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
One TODO on configuration left, but we now have 0 errors ✔ | 0 warnings ✔ | 1 note ✖where the NOTE is on size and hence immutable. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 663 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This PR 'somewhat manually' and via cherry-picking re-integrates the changes developed while upstream was going from 3.0.1 to 3.1.0. We are now in good shape.