Conversation
- Add `graph_to_r_function()` to lower supported `Graph` computations to plain base R - Add `graph_to_quickr_function()` wrapper that emits `declare(type(...))` and compiles via `quickr::quick()` - Export new helpers; add roxygen docs and DESCRIPTION/Collate updates (incl. quickr in Suggests) - Add tests for R conversion and quickr parity vs PJRT across core ops/reductions/select - Add anvil.Rproj and fix minor Rd whitespace
- Replace large switch() in .emit_prim() with a registry of per-primitive lowerers and add a preflight check that errors with a list of unsupported primitives. - Add mul_broadcast_axis lowering and an op-level fusion pass that rewrites broadcast_in_dim + mul into a loop-based broadcasted multiply for higher-rank tensors. - Add base emitters for any/all reductions when include_declare = FALSE. - Test improvements: move eval_graph_pjrt() into tests/testthat/helper-eval-graph.R, add graph_to_r_function parity coverage vs PJRT, add broadcast-mul fusion coverage for both R and quickr backends, and extend graph_to_r_function primitive support tests (max/min/transpose/ reshape/convert) plus an explicit error for unsupported higher-order primitives.
- Add if primitive lowerer to graph_to_r_function(), including scalar predicate validation and support for scalar/tensor outputs (including nested if). - Refactor graph lowering to share counters and extract helpers for constant inlining and op extraction (.inline_constants_for_graph(), .ops_from_graph()), reused by if branches. - Extend broadcast-mul fusion coverage (axis=1 and broadcast-as-LHS) and ensure fusion doesn’t trigger when the broadcasted value is reused. - Expand R/quickr parity tests against PJRT for if (scalar/tensor outputs, input-dependent branches, nested if, and fusion inside branches) and update the “unsupported higher-order primitives” test to while.
…tion tests - Ignore local mnist.rds in git/R builds (.gitignore, .Rbuildignore) - Fix graph_to_r_function() dot_general lowering to respect contracting_dims/batching_dims (R/ graph-to-r.R) - Add missing backward rules for exp, log, maximum/minimum, and reduce_max (tie-splitting) (R/ rules-backward.R) - Add MNIST MLP training script + opt-in training test using mnist.rds (inst/extra-tests/train- mnist-mlp.R, tests/testthat/test-mnist-training.R, tests/testthat/helper.R) - Add quickr/PJRT integration tests for larger “real use” graphs + extra dot_general coverage (tests/testthat/test-graph-to-quickr-integration.R, tests/testthat/test-graph-to-r.R, tests/ testthat/test-primitives-backward.R)
- Load MNIST from Sys.getenv("ANVIL_MNIST_RDS", "mnist.rds") (no path searching) (tests/testthat/
helper.R, inst/extra-tests/train-mnist-mlp.R)
- Update MNIST training defaults to full dataset sizes (train_n=60000, test_n=10000) and set the
training test default ANVIL_MNIST_TRAIN_N=60000 (inst/extra-tests/train-mnist-mlp.R, tests/
testthat/test-mnist-training.R)
- Add greta-like probabilistic model integration tests (log-joint / gradient) validated via
quickr vs PJRT (tests/testthat/test-greta-like-models.R)
- Add R/quickr.R helpers for optional {quickr} integration and eager compilation
- Refactor graph_to_quickr_function() to use assert_quickr_installed() and quickr_eager_compile()
- Make graph_to_r_function() default include_declare = TRUE, with declare(type(...)) treated as a
no-op in plain R
- Rename dtype mapping helper to .dtype_to_r_ctor and clean up related codegen paths
- Avoid relying on %||% in backward pass required-env lookup
- Wire new file into Collate and add a small optional-dependency test
- Reorder Collate so quickr/conversion helpers load before dependent code - Add as_r_function() and as_quickr_function() convenience wrappers - Drop the constants argument from graph_to_r_function() and graph_to_quickr_function(); constants are always inlined so only graph inputs become function args - Update docs (.Rd) and remove tests that covered the removed “constants as args” mode - Export new helpers in NAMESPACE
- Compile the leaf-argument function with quickr and, when graph@in_tree is nested, return an outer R wrapper that accepts the original top-level inputs, flattens them, and forwards to the compiled inner function. - Make eval_graph_pjrt() flatten ... so PJRT evaluation matches nested input calling conventions. - Add a PJRT-vs-quickr parity test covering nested params input (MLP loss-style).
- Allow graph_to_quickr_function() to handle non-leaf outputs by packing to a flat vector for quickr and unflattening to out_tree - Add graph_to_r_function(pack_output=) plus quickr-safe lowering for sign() and atan2(), and implement while graph lowering - Tighten eval_graph_pjrt() arg checking and preserve nested output structures - Extend tests for sign/atan2, packed list outputs, and while
# Conflicts: # DESCRIPTION
# Conflicts: # DESCRIPTION # R/reexports.R # man/platform.Rd # man/reexports.Rd
- Extend lowering coverage (reshape, reductions incl. max/min/prod, boolean reduce_any/reduce_all, plus additional primitives/rules) - Improve generated-function wrappers (constants/static args, packing/unpacking structured outputs, and edge cases like empty dims/slices) - Add/reshape test suite to compare quickr vs PJRT across primitives and integration workloads; update docs/pkgdown + dependency metadata
- Qualify testthat helpers (skip_if_not_installed/expect_*) for object_usage_linter - Avoid cross-helper binding lint for eval_graph_pjrt via get(...) - Silence object_length lint for helper name without breaking line-length rules
Rename long helper functions to satisfy lintr object_length_linter after formatting, and update call sites.
|
Hi @sebffischer, thanks again for the careful review, and sorry for the long delay here. I’ve updated the PR to address the main points you raised, and I also merged the latest changes from I’m also trying to keep this PR tightly scoped (avoid touching unrelated files / large cross-cutting refactors). In that spirit, I kept the quickr lowering rules together in one place for now; if you’d prefer rules colocated with primitives, I’m happy to do that as a follow-up PR after this lands. Two integration tests/examples that exercise end-to-end use cases and show how this might fit into a real workflow:
One additional note: working on this PR also drove some feature development in Longer-term, I’d like to move towards making Replies to your numbered questions:
A few other highlights (non-exhaustive):
Things I’d like your take on:
Suggested review entry points: |
|
@t-kalinowski I am on vacation for the next 3 weeks, so it will take some time for me to respond. |
|
Also a TODO for me:
|
3 participants
Why
PJRT is great for accelerator-backed execution, but today there are two CPU-centric use cases where a
{quickr}backend is especially compelling:Graphto a concrete piece of R code is the natural seam for future ahead-of-time compilation and/or caching of compiled artifacts.For pure CPU graphs,
{quickr}should also be a strong performance baseline (and may beat PJRT-on-CPU by avoiding PJRT program/execute overhead).In the longer term, I anticipate we'll add GPU support to quickr (likely via OpenACC, or possibly via CUDA directly), at which time
quickrmight provide a simple interface to writing custom, fast,anvilkernels.What’s included
graph_to_quickr_function()anvil::Graphto a plain R function and eagerly compiles it withquickr::quick(){quickr}{quickr}is optional (Suggests); feature/tests skip gracefully when it isn’t installed{quickr}execution vs PJRT executionMinimal example (reprex)
Integration tests
loss + gradmatches PJRT outputs, then a few SGD steps reduce the loss:https://github.com/t-kalinowski/anvil/blob/quickr/tests/testthat/test-graph-to-quickr-integration.R#L1
log_prob + gradmatches PJRT, and parameter updates remain identical over multiple gradient-ascent steps:https://github.com/t-kalinowski/anvil/blob/quickr/tests/testthat/test-graph-to-quickr-integration.R#L70
Testing
devtools::test()Notes / limitations
constant,add,sub,mul,divide,negate,broadcast_in_dim,dot_general,transpose,reshape,sum.transposeis currently rank-2 only;sumsupports rank 0–2 reductions and full reductions for rank > 2.