Add R bindings - #84
Open
s3alfisc wants to merge 30 commits into
Open
Conversation
crates/within-r stopped building when it was removed from the root workspace members (it is neither excluded via workspace.exclude nor detached with its own [workspace] table), and nothing in CI or the build wiring references it. The dev-mode build of the embedded crate (withinr/src/rust with the crates-io patch) covers the same ground, so delete the shim instead of resurrecting it.
Nothing compiled the R bridge in CI, so core Rust API changes could break it silently. Add a job that rustfmt-checks the bridge crate, installs the package in dev mode (patched against the workspace 'within' crate), and runs the manual R test suite. Formats the bridge source so the new fmt gate passes.
Every extendr entry point was split into a throwing wrapper plus a Result-returning inner function, repeating each signature twice. Returning Result directly is not an option: extendr's default Result conversion unwraps (panics), and the generated wrapper reports only a generic 'User function panicked' message, losing the error text. So keep one function per binding and wrap the fallible body in a ?-friendly closure funnelled through or_throw.
Pure code motion: mirror the module layout of crates/within-py so a change to the Python glue points at the same-named file in the R glue. lib.rs is registration-only, composed from per-module extendr_module! blocks; api holds the solve entry points and the solver handle, config the R->native config parsing and the preconditioner handle, convert the shared coercion helpers, results the native->list conversions.
The $(STATLIB) make target has no prerequisites, and C_clean did not remove the staticlib, so any existing rust/target/release/libwithinr.a was linked as-is: R CMD INSTALL silently shipped stale Rust code after bridge changes. Remove the staticlib in C_clean (as rextendr's template does) so cargo always runs; cargo's own cache keeps rebuilds cheap.
The solve entry points flattened LsmrOptions into (tol, maxiter, local_size) scalars, so adding one option meant touching the R constructor, four R call sites, four bridge signatures, and two validation layers. Pass the classed R list through as a single Robj and resolve it in config::parse_lsmr_options, mirroring resolve_lsmr_config in within-py; argument order now matches the PyO3 signatures (categories, y, options, weights, preconditioner). Validation now lives once, in the bridge: the R constructors reduce to class tagging, like the pyclass constructors. Rust doc comments on the internal entry points are demoted to plain comments so the regenerated wrapper file does not roxygen-export them.
The R side carried metadata Python does not expose: a build-time field threaded through dedicated SolverHandle/PreconditionerHandle wrapper structs, a $variant field, an lsmr_options alias, and CamelCase spellings of the shortcut strings. Drop all of it: the handles collapse to the bare native types, the print method now matches the Python __repr__ (Preconditioner(<variant>, n=...)), and the within_preconditioner unwrap moves out of R into parse_preconditioner, mirroring resolve_precond_input.
R matrices are column-major, so every RHS column is already a contiguous slice of the matrix data; the batch paths nevertheless copied each column into a Vec<Vec<f64>>. Borrow them directly (the Python glue needs Cow columns only because numpy input can be strided; R input never is). One-shot weights are borrowed too; the persistent solver keeps its owned copy since it stores weights across solves, matching the borrow/own split in within-py.
Attaching the package masked base::solve(): calling solve(A) on a square numeric matrix would coerce it to integer categories instead of inverting it. Python does not have this problem because within.solve is namespaced. Prefix the two one-shot entry points; the persistent solver methods (solver$solve, solver$solve_batch) are unaffected.
The 15 MB vendor.tar.xz re-landed in git history on every dependency bump. It is a packaging-time artifact: generate it with rextendr::vendor_crates() when cutting a release (the Makevars error hint said vendor_pkgs, which is the undocumented alias — align it with the README on vendor_crates). Also drop the hardcoded crate version from the DESCRIPTION prose and note the Cargo.lock regeneration step in the release flow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #24.
Adds R bindings follow choices of the python binding's API.