Bestdose (DO NOT MERGE) by Siel · Pull Request #292 · LAPKB/Pmetrics

Siel · 2026-02-11T19:27:02Z

No description provided.

…ow should be managed by Pmetrics

…zation

Copilot

Pull request overview

This PR introduces a new “BestDose” optimization workflow exposed to R, backed by new Rust bindings and R6 wrapper classes, and updates the Rust build/template plumbing to accept an explicit template/cache path.

Changes:

Add Rust BestDose executor + new extendr exports (bestdose, bestdose_prepare, bestdose_optimize) and corresponding R wrappers/R6 classes.
Update Rust model compilation and dummy compilation APIs to accept a template_path and bump pmcore dependency.
Add documentation/example assets demonstrating BestDose usage.

Reviewed changes

Copilot reviewed 27 out of 31 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/rust/src/lib.rs	Adds BestDose exports; changes `compile_model`/`dummy_compile` signatures to accept `template_path`.
src/rust/src/executor.rs	Adjusts `fit` trait bounds and mutability for updated algorithm API.
src/rust/src/bestdose_executor.rs	New Rust implementation bridging pmcore BestDose results/handles into R objects.
src/rust/Cargo.toml	Bumps `pmcore` version and adds `libloading`.
Cargo.lock	Locks updated Rust dependency graph for the new `pmcore`/`libloading` versions.
R/extendr-wrappers.R	Regenerated extendr wrappers reflecting new/changed Rust exports.
R/PMbuild.R	Updates `dummy_compile()` call to pass a user dir path.
R/PM_result.R	Adds `PM_result$bestdose()` convenience method returning `PM_bestdose`.
R/PM_bestdose.R	New R6 classes `PM_bestdose` and `PM_bestdose_problem` + helpers.
NAMESPACE	Exports `PM_bestdose`, `PM_bestdose_problem`, and `bestdose`.
man/compile_model.Rd	Updates docs for new `compile_model(..., template_path, ...)` signature.
man/dummy_compile.Rd	Updates docs for new `dummy_compile(template_path)` signature.
man/interp.Rd	Minor example syntax cleanup.
man/PM_result.Rd	Documents new `PM_result$bestdose()` method.
man/PM_model.Rd	Example formatting/assignment operator cleanup.
inst/.gitignore	Adds ignore for `template/` under `inst/`.
inst/Examples/src/bestdose_target.csv	New BestDose target example dataset.
inst/Examples/src/bestdose_prior.csv	New BestDose prior example dataset.
inst/Examples/src/bestdose_past.csv	New BestDose past-data example dataset.
inst/Examples/Runs/bestdose_result.rds	Adds a saved BestDose output artifact for examples.
inst/Examples/Rscript/bestdose_simple_test.R	Adds an example script for preparing/optimizing BestDose problems.
Examples/src/simTemp.csv	Adds example dataset asset.
Examples/src/ptaex1.csv	Adds example dataset asset.
Examples/src/ex_full.csv	Adds example dataset asset.
Examples/src/ex.csv	Adds example dataset asset.
Examples/src/bad.csv	Adds “bad” dataset example asset.
Examples/Runs/.gitkeep	Keeps example runs directory in git.
Examples/Rscript/examples.R	Adds a large end-to-end examples script.
Archived/PM_model_old.R	Updates archived code to pass `template_path` to `compile_model`.
.gitignore	Stops ignoring `Examples`, and adds `man/` to ignore list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T19:37:43Z

src/rust/src/bestdose_executor.rs

+        Self {
+            id: id.to_string(),
+            time: pred.time(),
+            observed: pred.obs().unwrap_or(0.0),
+            pop_mean: pred.pop_mean(),


pred.obs().unwrap_or(0.0) will silently convert missing observations into 0.0, which is indistinguishable from a real 0 value and will skew downstream plots/metrics in R. Prefer carrying missingness through to R (e.g., store observed as Option<f64> / Nullable<f64> and let it become NA, or use f64::NAN if the downstream R code expects numeric).

Copilot · 2026-02-11T19:37:43Z

R/PM_bestdose.R

+            self$model_info <- model_info
+            self$settings <- settings
+
+            cli::cli_alert_success("BestDose problem prepared with %d support points", dim[1])


cli::cli_alert_success() uses glue-style interpolation, so the %d format string won’t be substituted and dim[1] will be ignored. Use glue placeholders (e.g., {dim[1]}) or build the message with sprintf() before passing it to cli_alert_success().

Suggested change

cli::cli_alert_success("BestDose problem prepared with %d support points", dim[1])

cli::cli_alert_success("BestDose problem prepared with {dim[1]} support points")

Copilot · 2026-02-11T19:37:43Z

NAMESPACE

+export(PM_bestdose)
+export(PM_bestdose_problem)


These new exports (PM_bestdose, PM_bestdose_problem) do not appear to have corresponding .Rd documentation files in man/, which will typically surface as undocumented-exported-object notes in R CMD check. Add roxygen docs for these classes and ensure the generated man/*.Rd files are committed/included in the package build.

Suggested change

export(PM_bestdose)

export(PM_bestdose_problem)

Copilot · 2026-02-11T19:37:44Z

R/PM_result.R

+    bestdose = function(target,
+                        past_data = NULL,
+                        dose_range = list(min = 0, max = 1000),
+                        bias_weight = 0.5,
+                        target_type = "concentration",
+                        time_offset = NULL,
+                        ...) {
+      # Use this result's data as past_data if not specified
+      if (is.null(past_data)) {
+        past_data <- self$data
+      }
+
+      PM_bestdose$new(
+        prior = self,
+        model = self$model,
+        past_data = past_data,
+        target = target,
+        dose_range = dose_range,
+        bias_weight = bias_weight,
+        target_type = target_type,
+        time_offset = time_offset,
+        ...
+      )
+    },


New public behavior (PM_result$bestdose() and the BestDose R6 wrappers) is added here, but there are currently no testthat tests exercising success/failure paths (input validation, handle lifecycle, and basic structure of returned results). Adding a small unit/integration test would help prevent regressions, especially around the Rust externalptr handle and error propagation as strings.

Copilot · 2026-02-11T19:37:44Z

src/rust/src/bestdose_executor.rs

+    let pred_rows: Vec<BestDosePredictionRow> = result
+        .predictions()
+        .predictions()
+        .iter()
+        .map(|p| BestDosePredictionRow::from_np_prediction(p, "subject_1"))
+        .collect();


The predictions dataframe is currently stamped with a hard-coded id value ("subject_1"). If target/past data contain a real subject identifier, this will mislabel outputs and can break joins/grouping in R. Consider extracting the subject ID from the underlying pmcore subject used in the problem and pass it through when building BestDosePredictionRow.

Copilot · 2026-02-11T19:37:44Z

src/rust/src/bestdose_executor.rs

+        let (library, (eq, meta)) =
+            unsafe { pmcore::prelude::pharmsol::exa::load::load::<ODE>(model_path) };
+
+        let settings = settings(params, meta.get_params(), "/tmp/bestdose")
+            .map_err(|e| format!("Failed to parse settings: {}", e))?;


The settings path is hard-coded to /tmp/bestdose, which is not portable (e.g., Windows) and may cause permission/collision issues. Prefer using std::env::temp_dir() (optionally namespaced per process) or accepting a path from R (e.g., tools::R_user_dir(package="Pmetrics")) consistent with the template cache path used elsewhere.

Copilot · 2026-02-11T19:37:44Z

R/PM_bestdose.R

+        print = function() {
+            cat("BestDose Optimization Results\n")
+            cat("==============================\n\n")
+            cat(sprintf("Optimal doses: [%.2f, %.2f] mg\n", self$get_doses()[1], self$get_doses()[2]))


print() assumes there are exactly 2 doses (self$get_doses()[1] and [2]). If the target template contains a different number of doses (or optimization returns variable-length dosing), this will error or drop values. Consider formatting doses generically (e.g., paste/format the full vector) and handling length-0/1 safely.

Suggested change

cat(sprintf("Optimal doses: [%.2f, %.2f] mg\n", self$get_doses()[1], self$get_doses()[2]))

doses <- self$get_doses()

if (is.null(doses) || length(doses) == 0L || all(is.na(doses))) {

dose_str <- "None"

} else {

dose_str <- paste(sprintf("%.2f", doses), collapse = ", ")

}

cat(sprintf("Optimal dose(s): [%s] mg\n", dose_str))

Copilot · 2026-02-11T19:37:45Z

NAMESPACE

 export(add_shapes)
 export(add_smooth)
 export(additive)
+export(bestdose)


bestdose is exported but there is no man/bestdose.Rd (or equivalent roxygen block) in the current tree. If this is intended to be a public API, please add user-facing documentation; otherwise, consider not exporting it and exposing only the higher-level PM_result$bestdose() / PM_bestdose* interfaces.

Suggested change

export(bestdose)

Copilot · 2026-02-11T19:37:45Z

R/PMbuild.R

  clear_build() # clean prior template/artifacts
  if (is_rustup_installed()) {
    cli::cli_text("Rust was detected in your system, Fetching dependencies and building base project.")
-    template <- dummy_compile()
-
+    template <- dummy_compile(tools::R_user_dir(package = "Pmetrics"))


PM_build() still calls clear_build(), but the Rust/extendr wrapper for clear_build was removed in this PR. This will cause PM_build() to error with “could not find function 'clear_build'”. Either re-introduce the clear_build extendr export (and wrapper) or replace this call with an R implementation that clears the build/template cache in the same location used by dummy_compile() / compile_model().

mhovd · 2026-02-12T12:25:45Z

Can we close this, @Siel ?

Siel added 6 commits November 5, 2025 15:05

feat: clean_build() removed, template_path() removed, template_path n…

f7a1ad1

…ow should be managed by Pmetrics

initial version

a586a23

working example

7304c03

working example

15793e4

missing files

bab4eb2

bestdose is subdivided in two steps, posterior calculation and optimi…

2236e6c

…zation

Copilot AI review requested due to automatic review settings February 11, 2026 19:27

Copilot started reviewing on behalf of Siel February 11, 2026 19:27 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bestdose (DO NOT MERGE)#292

Bestdose (DO NOT MERGE)#292
Siel wants to merge 6 commits intomainfrom
bestdose

Siel commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

mhovd commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	cli::cli_alert_success("BestDose problem prepared with %d support points", dim[1])
	cli::cli_alert_success("BestDose problem prepared with {dim[1]} support points")

-            cat(sprintf("Optimal doses: [%.2f, %.2f] mg\n", self$get_doses()[1], self$get_doses()[2]))
+            doses <- self$get_doses()
+            if (is.null(doses) || length(doses) == 0L || all(is.na(doses))) {
+                dose_str <- "None"
+            } else {
+                dose_str <- paste(sprintf("%.2f", doses), collapse = ", ")
+            }
+            cat(sprintf("Optimal dose(s): [%s] mg\n", dose_str))

Conversation

Siel commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

mhovd commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants